Skip to content

fix: replace dynamic require with static import for node sdk SDK_VERSION #1285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
motopods wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −4
Open

fix: replace dynamic require with static import for node sdk SDK_VERSION #1285

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions sdk/packages/node/iii/src/iii.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { context, trace } from '@opentelemetry/api'
import { createRequire } from 'node:module'
import * as os from 'node:os'
import { type Data, WebSocket } from 'ws'
import { ChannelReader, ChannelWriter } from './channels'
Expand Down Expand Up @@ -59,9 +58,7 @@ import type {
FunctionRef,
} from './types'
import { isChannelRef } from './utils'

const require = createRequire(import.meta.url)
const { version: SDK_VERSION } = require('../package.json')
import { version as SDK_VERSION } from '../package.json'
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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 (or namespace.default). (typescriptlang.org)
  • Rollup JSON handling: Rollup itself needs a plugin; @rollup/plugin-json synthesizes 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-json does; 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 .json module.) (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:

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.

Suggested change
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.


function getOsInfo(): string {
return `${os.platform()} ${os.release()} (${os.arch()})`
Expand Down