fix(build): preserve ESM import specifiers in published package#584
fix(build): preserve ESM import specifiers in published package#584amartidandqdq wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
WalkthroughThis PR fixes ESM module resolution failures in packaged environments (issue Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a post-build script (
Confidence Score: 1/5Not safe to merge — the TypeScript compiler will fail before the post-build ESM fixer ever runs. Three imports added to src/plugin.ts contains all three broken imports and needs the three missing source files (or exports) to be added before the build can succeed. Important Files Changed
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
src/plugin.ts:42
**Missing module: `./plugin/rate-limit-parser`**
`parseRateLimitResetMs` and the new 2-argument `parseRateLimitReason(bodyText, status)` are imported from `./plugin/rate-limit-parser`, but `src/plugin/rate-limit-parser.ts` does not exist in the repository. The TypeScript build will fail with a "cannot find module" error the moment `tsc` is invoked, which means the post-build ESM-fixer script added in the same PR never even runs.
### Issue 2 of 4
src/plugin.ts:50
**Missing module: `./plugin/usage-tracker`**
`getTotalUsage` is imported from `./plugin/usage-tracker`, but `src/plugin/usage-tracker.ts` does not exist anywhere in the repository. This is a second compile-time "cannot find module" error that will break the build independently of the missing `rate-limit-parser`.
### Issue 3 of 4
src/plugin.ts:14
**Missing export: `promptContinue` from `./plugin/cli`**
`promptContinue` is added to the named import from `./plugin/cli`, but the function is not exported (nor defined) in `src/plugin/cli.ts`. The only exports there are `promptProjectId`, `promptAddAnotherAccount`, `promptLoginMode`, and re-exports from `./ui/auth-menu`. TypeScript will raise a compile error for this import, and the eleven call-sites added throughout the plugin will fail at runtime if this is worked around.
### Issue 4 of 4
script/fix-esm-imports.mjs:9-17
**Redundant regex patterns cause all `from "..."` specifiers to be visited twice**
Pattern 1 already matches every `from "./..."` statement — including all named imports, type imports, and re-exports that patterns 4–7 target. Because each pattern is applied to the mutated `output` in sequence, the affected specifiers will be matched and passed through `resolveSpecifier` a second time. The second pass is harmless only because `hasExtension` short-circuits once `.js` has already been appended, but removing patterns 4–7 (or restricting pattern 1 to non-`from` forms) would make the intent clearer and halve the work on large `dist` trees.
Reviews (1): Last reviewed commit: "fix(build): preserve ESM import specifie..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -40,11 +40,13 @@ import { EmptyResponseError } from "./plugin/errors"; | |||
| import { AntigravityTokenRefreshError, refreshAccessToken } from "./plugin/token"; | |||
| import { startOAuthListener, type OAuthListener } from "./plugin/server"; | |||
| import { clearAccounts, loadAccounts, saveAccounts, saveAccountsReplace } from "./plugin/storage"; | |||
There was a problem hiding this comment.
./plugin/rate-limit-parser
parseRateLimitResetMs and the new 2-argument parseRateLimitReason(bodyText, status) are imported from ./plugin/rate-limit-parser, but src/plugin/rate-limit-parser.ts does not exist in the repository. The TypeScript build will fail with a "cannot find module" error the moment tsc is invoked, which means the post-build ESM-fixer script added in the same PR never even runs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 42
Comment:
**Missing module: `./plugin/rate-limit-parser`**
`parseRateLimitResetMs` and the new 2-argument `parseRateLimitReason(bodyText, status)` are imported from `./plugin/rate-limit-parser`, but `src/plugin/rate-limit-parser.ts` does not exist in the repository. The TypeScript build will fail with a "cannot find module" error the moment `tsc` is invoked, which means the post-build ESM-fixer script added in the same PR never even runs.
How can I resolve this? If you propose a fix, please make it concise.| import { createSessionRecoveryHook, getRecoverySuccessToast } from "./plugin/recovery"; | ||
| import { checkAccountsQuota } from "./plugin/quota"; | ||
| import { getTotalUsage } from "./plugin/usage-tracker"; | ||
| import { initDiskSignatureCache } from "./plugin/cache"; |
There was a problem hiding this comment.
./plugin/usage-tracker
getTotalUsage is imported from ./plugin/usage-tracker, but src/plugin/usage-tracker.ts does not exist anywhere in the repository. This is a second compile-time "cannot find module" error that will break the build independently of the missing rate-limit-parser.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 50
Comment:
**Missing module: `./plugin/usage-tracker`**
`getTotalUsage` is imported from `./plugin/usage-tracker`, but `src/plugin/usage-tracker.ts` does not exist anywhere in the repository. This is a second compile-time "cannot find module" error that will break the build independently of the missing `rate-limit-parser`.
How can I resolve this? If you propose a fix, please make it concise.| import type { AntigravityTokenExchangeResult } from "./antigravity/oauth"; | ||
| import { accessTokenExpired, isOAuthAuth, parseRefreshParts, formatRefreshParts } from "./plugin/auth"; | ||
| import { promptAddAnotherAccount, promptLoginMode, promptProjectId } from "./plugin/cli"; | ||
| import { promptAddAnotherAccount, promptLoginMode, promptProjectId, promptContinue } from "./plugin/cli"; |
There was a problem hiding this comment.
promptContinue from ./plugin/cli
promptContinue is added to the named import from ./plugin/cli, but the function is not exported (nor defined) in src/plugin/cli.ts. The only exports there are promptProjectId, promptAddAnotherAccount, promptLoginMode, and re-exports from ./ui/auth-menu. TypeScript will raise a compile error for this import, and the eleven call-sites added throughout the plugin will fail at runtime if this is worked around.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin.ts
Line: 14
Comment:
**Missing export: `promptContinue` from `./plugin/cli`**
`promptContinue` is added to the named import from `./plugin/cli`, but the function is not exported (nor defined) in `src/plugin/cli.ts`. The only exports there are `promptProjectId`, `promptAddAnotherAccount`, `promptLoginMode`, and re-exports from `./ui/auth-menu`. TypeScript will raise a compile error for this import, and the eleven call-sites added throughout the plugin will fail at runtime if this is worked around.
How can I resolve this? If you propose a fix, please make it concise.| const importPatterns = [ | ||
| /(from\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| /(import\s*\(\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| /(\bimport\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| /(export\s*\{[^}]*\}\s*from\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| /(export\s*(?:type\s*)?\*\s*from\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| /(import\s+type\s*\{[^}]*\}\s*from\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| /(import\s*\{\s*type\s+[^}]*\}\s*from\s*["'])(\.{1,2}\/[^"']+)(["'])/g, | ||
| ] |
There was a problem hiding this comment.
from "..." specifiers to be visited twice
Pattern 1 already matches every from "./..." statement — including all named imports, type imports, and re-exports that patterns 4–7 target. Because each pattern is applied to the mutated output in sequence, the affected specifiers will be matched and passed through resolveSpecifier a second time. The second pass is harmless only because hasExtension short-circuits once .js has already been appended, but removing patterns 4–7 (or restricting pattern 1 to non-from forms) would make the intent clearer and halve the work on large dist trees.
Prompt To Fix With AI
This is a comment left during a code review.
Path: script/fix-esm-imports.mjs
Line: 9-17
Comment:
**Redundant regex patterns cause all `from "..."` specifiers to be visited twice**
Pattern 1 already matches every `from "./..."` statement — including all named imports, type imports, and re-exports that patterns 4–7 target. Because each pattern is applied to the mutated `output` in sequence, the affected specifiers will be matched and passed through `resolveSpecifier` a second time. The second pass is harmless only because `hasExtension` short-circuits once `.js` has already been appended, but removing patterns 4–7 (or restricting pattern 1 to non-`from` forms) would make the intent clearer and halve the work on large `dist` trees.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f75a8a258
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import type { AntigravityTokenExchangeResult } from "./antigravity/oauth"; | ||
| import { accessTokenExpired, isOAuthAuth, parseRefreshParts, formatRefreshParts } from "./plugin/auth"; | ||
| import { promptAddAnotherAccount, promptLoginMode, promptProjectId } from "./plugin/cli"; | ||
| import { promptAddAnotherAccount, promptLoginMode, promptProjectId, promptContinue } from "./plugin/cli"; |
There was a problem hiding this comment.
This import adds promptContinue, but src/plugin/cli.ts does not export that symbol (it only exports the existing prompt helpers and isTTY). Any build or runtime ESM load of the plugin will fail on this missing export before the new menu paths can run, so either add the helper to cli.ts or remove these calls.
Useful? React with 👍 / 👎.
| import { clearAccounts, loadAccounts, saveAccounts, saveAccountsReplace } from "./plugin/storage"; | ||
| import { AccountManager, type ModelFamily, parseRateLimitReason, calculateBackoffMs, computeSoftQuotaCacheTtlMs } from "./plugin/accounts"; | ||
| import { AccountManager, type ModelFamily, calculateBackoffMs, computeSoftQuotaCacheTtlMs } from "./plugin/accounts"; | ||
| import { parseRateLimitResetMs, parseRateLimitReason } from "./plugin/rate-limit-parser"; |
There was a problem hiding this comment.
This changes the parser import to ./plugin/rate-limit-parser, but that file is not present in the tree (I checked the repo file list with find/rg). As a result the package cannot compile or load once src/plugin.ts is included; either commit the new parser module or keep importing the existing parser from ./plugin/accounts.
Useful? React with 👍 / 👎.
| import { loadConfig, initRuntimeConfig, type AntigravityConfig } from "./plugin/config"; | ||
| import { createSessionRecoveryHook, getRecoverySuccessToast } from "./plugin/recovery"; | ||
| import { checkAccountsQuota } from "./plugin/quota"; | ||
| import { getTotalUsage } from "./plugin/usage-tracker"; |
There was a problem hiding this comment.
The new Usage menu branch imports ./plugin/usage-tracker, but no matching source file is committed in src/plugin or elsewhere in the repo. This unresolved module breaks the build/load path for the plugin even if the menu action is never selected, so the usage tracker implementation needs to be added or the import and branch removed.
Useful? React with 👍 / 👎.
| @@ -1,5 +1,5 @@ | |||
| import { exec } from "node:child_process"; | |||
| import { tool } from "@opencode-ai/plugin"; | |||
| import { tool } from "@opencode-ai/plugin/tool"; | |||
There was a problem hiding this comment.
OpenCode’s plugin/custom-tool API exposes tool from @opencode-ai/plugin, while this change switches to a @opencode-ai/plugin/tool subpath without updating the dependency or showing that this subpath is exported by the installed package. In installations that only have the declared @opencode-ai/plugin package entrypoint, this unresolved import prevents the plugin module from evaluating before any hooks run, so keep the root import unless the dependency/export map is updated accordingly.
Useful? React with 👍 / 👎.
| const entryStat = await stat(entryPath) | ||
| if (entryStat.isDirectory()) { | ||
| files.push(...await listJsFiles(entryPath)) | ||
| } else if (entry.endsWith(".js") || entry.endsWith(".mjs")) { |
There was a problem hiding this comment.
The post-build fixer only walks .js and .mjs outputs, but the package publishes dist/index.d.ts via the types field and those declarations keep extensionless relative exports like ./src/plugin. Consumers using NodeNext/Node16 TypeScript resolution can still fail on the published types even though the runtime .js files were fixed, so the same rewrite needs to cover emitted .d.ts files too.
Useful? React with 👍 / 👎.
| continue; | ||
| } | ||
|
|
||
| if (menuResult.mode === "usage") { |
There was a problem hiding this comment.
This new usage branch is never reachable: promptLoginMode/showAuthMenu only return add, fresh, manage, check, verify, verify-all, or cancel actions, and there is no menu item or fallback input that produces mode: "usage". After the missing module is added, users still cannot open the advertised session token usage view from opencode auth login unless the menu result types and prompts are updated to return this mode.
Useful? React with 👍 / 👎.
1 participant
Fixes #564, #568. Based on PR #565 by @pandaria75. Post-build script rewrites bare relative ESM specifiers to explicit .js or /index.js paths. Also fixes import of tool from @opencode-ai/plugin/tool.