Skip to content

fix(sdk): add .js extensions to all ESM imports for Node.js compatibility#623

Open
ihsraham wants to merge 3 commits intomainfrom
fix/sdk-esm-imports
Open

fix(sdk): add .js extensions to all ESM imports for Node.js compatibility#623
ihsraham wants to merge 3 commits intomainfrom
fix/sdk-esm-imports

Conversation

@ihsraham
Copy link
Collaborator

@ihsraham ihsraham commented Mar 16, 2026

Summary

Both @yellow-org/sdk and @yellow-org/sdk-compat declared "type": "module" but used extensionless relative imports in source (e.g. from './client'). TypeScript emits these as-is, and Node.js strict ESM resolution rejects them. This forced consumers to use tsx, postinstall patches, or --experimental-specifier-resolution=node.

Changes

  • Add .js extensions to all relative imports in sdk/ts/src/ (24 files) and sdk/ts-compat/src/ (7 files)
  • Switch module and moduleResolution from ESNext/bundler to Node16 in both tsconfig.json files
  • Fix Decimal import: default import -> named import for Node16 compatibility

Verification

  • Both packages build clean with tsc
  • node -e "import('./dist/index.js')" succeeds in both packages without any flags
  • Bundler consumers (Vite, webpack) are unaffected since they handle .js extensions natively

Summary by CodeRabbit

  • Chores
    • Refactor: Updated module imports across the SDK to use explicit .js extensions for ESM compatibility.
    • Refactor: Adjusted TypeScript config to Node16 module resolution.
    • Refactor: Replaced several default imports with named imports for improved interoperability.
    • Note: Public APIs and runtime behavior remain unchanged.

…lity

Both @yellow-org/sdk and @yellow-org/sdk-compat declared "type": "module"
but used extensionless relative imports (e.g. from './client') in source.
TypeScript emits these as-is, and Node.js strict ESM resolution rejects
them. This forced consumers to use tsx, postinstall patches, or
--experimental-specifier-resolution=node.

Changes:
- Add .js extensions to all relative imports in sdk/ts/src (24 files)
- Add .js extensions to all relative imports in sdk/ts-compat/src (7 files)
- Switch moduleResolution from "bundler" to "Node16" in both tsconfig.json
  files so TypeScript enforces correct ESM paths at compile time
- Switch module from "ESNext" to "Node16" to match
- Fix Decimal import: default import -> named import for Node16 compat

Verified: both packages build clean with tsc, dist output has .js
extensions, and `node -e "import('./dist/index.js')"` succeeds without
any flags or workarounds.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b8d13d9-2500-42ad-a2fe-aa976c794a53

📥 Commits

Reviewing files that changed from the base of the PR and between 004ed30 and 0c048a0.

📒 Files selected for processing (1)
  • sdk/ts/src/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • sdk/ts/src/utils.ts

📝 Walkthrough

Walkthrough

This PR updates TypeScript/JS module specifiers across the SDK to use explicit .js extensions, changes several decimal.js default imports to named { Decimal } imports, and updates TypeScript compiler options to Node16 module/resolution settings.

Changes

Cohort / File(s) Summary
TypeScript config
sdk/ts-compat/tsconfig.json, sdk/ts/tsconfig.json
Change compiler module and moduleResolution from ESNext/bundler to Node16.
ts-compat core & barrels
sdk/ts-compat/src/app-signing.ts, sdk/ts-compat/src/auth.ts, sdk/ts-compat/src/client.ts, sdk/ts-compat/src/events.ts, sdk/ts-compat/src/rpc.ts, sdk/ts-compat/src/signers.ts, sdk/ts-compat/src/index.ts
All relative imports/re-exports updated to include .js extensions; minor import shape change in client.ts (Decimal import adjusted).
SDK top-level barrels
sdk/ts/src/index.ts, sdk/ts/src/core/index.ts, sdk/ts/src/rpc/index.ts, sdk/ts/src/blockchain/index.ts, sdk/ts/src/app/index.ts
Public re-exports updated to reference .js module files (index paths adjusted).
Core modules
sdk/ts/src/core/*, sdk/ts/src/asset_store.ts, sdk/ts/src/utils.ts
Local imports switched to .js extensions; multiple files change Decimal import from default to named; no API signature changes.
Client & SDK surface
sdk/ts/src/client.ts, sdk/ts-compat/src/client.ts
Numerous local import paths updated to .js; Decimal import style adjusted; public class/method signatures unchanged.
Blockchain / EVM
sdk/ts/src/blockchain/... (evm/client.ts, evm/erc20.ts, evm/index.ts, evm/locking_client.ts, evm/utils.ts)
Imports and re-exports updated to .js; Decimal import style adjusted; no functional changes.
RPC stack
sdk/ts/src/rpc/* (api.ts, client.ts, dialer.ts, error.ts, message.ts, types.ts)
All relative imports and re-exports switched to .js extensions; logic unchanged.
App modules
sdk/ts/src/app/packing.ts, sdk/ts/src/app/types.ts
Imports/exports updated to .js; Decimal import changed to named import; no API changes.
Misc small updates
sdk/ts/src/state*, sdk/ts/src/core/state_packer.ts, other scattered files
Consistent conversion of local specifiers to .js and Decimal import style updates; no behavior changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

ready

Suggested reviewers

  • alessio
  • nksazonov
  • philanton
  • dimast-x

Poem

🐰 Small hops from .ts to .js I go,
I tidy imports in a gentle row,
Decimal now named with a cheerful cheer,
Node16 whispers "resolve me here",
I nibble changes — build time soon to show. 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: adding .js extensions to ESM imports for Node.js compatibility, which is the primary focus across all 31 modified files and both tsconfig.json updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sdk-esm-imports
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

ihsraham and others added 2 commits March 16, 2026 16:48
This commit cleans up the utils.ts file by removing the unused import of RPCApp, streamlining the code and improving readability.
Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants