fix: replace dynamic require with static import for node sdk SDK_VERSION#1285
fix: replace dynamic require with static import for node sdk SDK_VERSION#1285motopods wants to merge 1 commit intoiii-hq:mainfrom
Conversation
…SION (#2) * refactor: use static import for SDK_VERSION from package.json in iii.ts Co-authored-by: motopods <58200641+motopods@users.noreply.github.com>
|
@Copilot is attempting to deploy a commit to the motia Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughRefactors the SDK version import from dynamic CommonJS require to static ES module import, replacing a multi-line require statement with a single import declaration while maintaining identical functionality. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/packages/node/iii/src/iii.ts`:
- Line 61: The current import statement uses a named export for package.json
(import { version as SDK_VERSION } from '../package.json'), which is
non-standard and can break under Node JSON modules; replace it with a default
import (e.g., import pkg from '../package.json') and then read the version
property (const SDK_VERSION = pkg.version) so SDK_VERSION is derived from the
default-exported JSON object; update any references to SDK_VERSION accordingly
in iii.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c107df9-7a22-4d51-b860-643bcc7d1dbb
📒 Files selected for processing (1)
sdk/packages/node/iii/src/iii.ts
|
|
||
| const require = createRequire(import.meta.url) | ||
| const { version: SDK_VERSION } = require('../package.json') | ||
| import { version as SDK_VERSION } from '../package.json' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Do Node.js JSON modules expose named exports or only a default export, and does TypeScript resolveJsonModule model JSON imports that way as well? Also, does Rolldown/Rollup JSON handling synthesize named exports by default?
💡 Result:
- Node.js JSON modules (ESM): default export only (the parsed JSON). No named exports. (nodejs.org)
- TypeScript
resolveJsonModule: models JSON imports as default-only as well (e.g.import cfg from "./x.json"). In--module nodenext, TS explicitly says it will not generate named exports for JSON; you must access via the default (ornamespace.default). (typescriptlang.org) - Rollup JSON handling: Rollup itself needs a plugin;
@rollup/plugin-jsonsynthesizes named exports by default (namedExports: true), in addition to the default export. (raw.githubusercontent.com) - Rolldown JSON handling: Rolldown has built-in JSON module support (“module type”
json), but it does not document any default synthesis of named exports the way@rollup/plugin-jsondoes; if you want named exports, you generally need Rollup-style JSON transformation. (A real-world report also shows Rolldown warning when something is imported as if it were an export that doesn’t exist in a.jsonmodule.) (rolldown.rs)
Sources (numbered)
[1] Node.js ESM docs (JSON import assertions; JSON exposes only default). (nodejs.org)
[2] TypeScript TSConfig resolveJsonModule. (typescriptlang.org)
[3] TypeScript 5.7 release notes (JSON imports in NodeNext; default-only, no named exports). (typescriptlang.org)
[4] @rollup/plugin-json README (default namedExports: true). (raw.githubusercontent.com)
[5] Rolldown “Module Types” docs + example report/warning. (rolldown.rs)
Citations:
- 1: https://nodejs.org/download/release/v19.0.0/docs/api/esm.html
- 2: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-7.html
- 3: https://raw.githubusercontent.com/rollup/plugins/master/packages/json/README.md
- 4: https://rolldown.rs/guide/in-depth/module-types?utm_source=openai
- 5: https://nodejs.org/download/release/v19.0.0/docs/api/esm.html
- 6: https://www.typescriptlang.org/tsconfig/resolveJsonModule.html
- 7: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-7.html
- 8: https://raw.githubusercontent.com/rollup/plugins/master/packages/json/README.md
- 9: https://rolldown.rs/guide/in-depth/module-types?utm_source=openai
Use a default import from package.json instead of named exports.
Node.js JSON modules expose only a default export—no named exports. TypeScript's resolveJsonModule with --module nodenext explicitly does not generate named exports either. Named exports from JSON are synthesized only by specific bundlers like Rollup's @rollup/plugin-json, making import { version as SDK_VERSION } from '../package.json' non-standard and fragile across different build configurations, including Rolldown. A default import is portable and follows the standard.
♻️ Proposed change
-import { version as SDK_VERSION } from '../package.json'
+import packageJson from '../package.json'
+
+const SDK_VERSION = packageJson.version📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { version as SDK_VERSION } from '../package.json' | |
| import packageJson from '../package.json' | |
| const SDK_VERSION = packageJson.version |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/packages/node/iii/src/iii.ts` at line 61, The current import statement
uses a named export for package.json (import { version as SDK_VERSION } from
'../package.json'), which is non-standard and can break under Node JSON modules;
replace it with a default import (e.g., import pkg from '../package.json') and
then read the version property (const SDK_VERSION = pkg.version) so SDK_VERSION
is derived from the default-exported JSON object; update any references to
SDK_VERSION accordingly in iii.ts.
iii.tswas loading the package version at runtime viacreateRequire, which is unnecessary givenresolveJsonModule: trueis already set intsconfig.json.Changes
src/iii.ts: Swap dynamiccreateRequire-based version load for a static ES import:Fix: #1261
Summary by CodeRabbit