Skip to content

fix(esm): make the published ESM build loadable under Node's native loader#1475

Open
droplet-rl wants to merge 1 commit into
masterfrom
droplet/esm-node-loadable
Open

fix(esm): make the published ESM build loadable under Node's native loader#1475
droplet-rl wants to merge 1 commit into
masterfrom
droplet/esm-node-loadable

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Problem

The published ESM build does not load under Node's native ESM loader. Any consumer that loads the package under real Node ESM — Vite dev SSR (TanStack Start / Next), Next RSC, ts-node/ESM, or plain node --input-type=module — crashes immediately. Bundler consumers (webpack / rollup / vite build) tolerate the malformed output, which is why this went unnoticed.

This is the root cause of the across-protocol/dapp regression where pnpm start (vite dev) 500s on every route:

ERR_MODULE_NOT_FOUND: Cannot find module '.../@across-protocol/sdk/dist/esm/src/utils/NetworkUtils'
  imported from '.../@across-protocol/sdk/dist/esm/src/arch/evm/BlockUtils.js'

Root cause

The ESM build is tsc --module es2015 + a stamped {"type":"module"}. tsc neither adds extensions to relative imports nor normalizes CJS interop, so the emitted ESM is not spec-valid for Node.

Fix — four classes, no public API or output-layout change

The dist/{esm,cjs,types}/... file layout is unchanged, so the ./dist/esm/* / ./dist/cjs/* deep-path exports consumers rely on still resolve. (Audited the V4 prod consumers: relayer & indexer use only the package root + named subpath exports; quote-api has a few deep dist/ imports — all preserved.)

  1. Extensionless relative imports → add a tsc-alias --resolve-full-paths post-step to build:esm. Emitted relative imports get .js / /index.js (606 specifiers fixed at build time, zero source churn).
  2. Extensionless deep imports into exports-less CJS deps → explicit .js//index.js for @across-protocol/contracts/dist/*, @eth-optimism/sdk/dist/*, ethers/lib/utils, arweave/node/lib/* (tsc-alias only rewrites relative imports, not bare-package ones).
  3. CJS named-import interoplodash and @coral-xyz/anchor don't expose Node-ESM-detectable named exports. Switched to default/namespace import with an interop fallback that is dual CJS/ESM safe (verified both builds).
  4. CJS-only require.main === module CLI guards throw in ESM scope → gated behind typeof require !== "undefined" (inert in ESM/browser, unchanged in Node CJS).

Verification

  • yarn dev (cjs / esm / types) builds clean; eslint + prettier clean on all changed files.
  • ESM and CJS index both import clean under node (17 exports each).
  • The across-protocol/dapp SDK import set (everything vendor/across/shared pulls — arch/evm/BlockUtils, utils/{Address,Network,Token,Event,Spoke,...}Utils, constants, interfaces/SpokePool, …) loads under Node native ESM: 13/13 modules, 0 failures (was 4 failures). This is exactly the dev-SSR externalization path that was failing.
  • dapp pnpm start before/after (overlaying this build): module-load errors 4 → 0.

Follow-ups (separate, optional)

  • CI: a dev-SSR smoke test (or a node --input-type=module -e 'import(".../dist/esm/src/index.js")' check) would catch ESM-load regressions that the bundler-based tests miss.
  • Longer term: moving the ESM build to a bundler (tsup/rollup preserveModules) would handle interop wholesale; this PR is the minimal, layout-preserving fix that unblocks consumers now.
  • Consumers deep-importing @across-protocol/sdk/dist/esm/src/... (e.g. the dapp's vendor/across/shared) could move to the package root — hygiene, not required by this change.

🤖 Generated with Claude Code

…oader

The ESM build (`tsc --module es2015` + a stamped `{"type":"module"}`) emitted
output that Node's native ESM loader rejects, so any consumer loading the
package under real Node ESM (Vite dev SSR, Next RSC, ts-node/ESM, plain `node`)
crashes. Bundler consumers (webpack/rollup/vite build) tolerate it, which is
why it went unnoticed.

Four classes of fix — no public API or output-layout change, so the
`./dist/esm/*` / `./dist/cjs/*` deep paths consumers rely on still resolve:

- Relative imports were extensionless. Add a `tsc-alias --resolve-full-paths`
  post-step to `build:esm`; emitted relative imports get `.js` / `/index.js`
  (606 specifiers, zero source churn).
- Deep imports into exports-less CJS deps were extensionless: add explicit
  `.js`/`/index.js` for `@across-protocol/contracts/dist/*`,
  `@eth-optimism/sdk/dist/*`, `ethers/lib/utils`, `arweave/node/lib/*`.
- CJS named-import interop: `lodash` and `@coral-xyz/anchor` don't expose
  Node-ESM-detectable named exports; switch to default/namespace import with
  an interop fallback (dual CJS/ESM safe).
- `require.main === module` CLI guards throw in ESM scope; gate behind
  `typeof require !== "undefined"`.

Verified: `yarn dev` (cjs/esm/types) clean; ESM and CJS `index` both import
clean under `node`; the across-protocol/dapp SDK import set loads under Node
native ESM; eslint + prettier clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gsteenkamp89 gsteenkamp89 marked this pull request as ready for review June 30, 2026 11:38
@gsteenkamp89

Copy link
Copy Markdown
Contributor

@codex review

@droplet-rl

Copy link
Copy Markdown
Contributor Author

Review — fix(esm): make the published ESM build loadable under Node's native loader

I checked the branch out, ran a full yarn install (which runs the build), and exercised the actual dist/ output under Node 22's native ESM loader. The approach is sound and the four fix-classes are correct. Verified independently:

  • Buildcjs / esm / types all compile clean.
  • tsc-alias --resolve-full-paths — relative imports in the emitted ESM get .js (and /index.js for dir imports), e.g. dist/esm/src/arch/evm/BlockUtils.js../../utils/NetworkUtils.js. No extensionless deep bare-package imports remain (grep is clean).
  • Native ESM loaddist/esm/src/index.js imports clean (17 exports); the full dapp / interop-sensitive set loads (BlockUtils evm+svm, svm/utils, SpokeUtils, eventsClient, ArweaveClient, providers, MerkleDistributor, mockedClient, PoolRebalanceUtils, DeploymentUtils, …).
  • Interop is real, not just loadablelodash.clamp/sortedIndexBy/random and anchor.BN/BorshEventCoder resolve to actual functions in ESM via the default ?? ns fallback. The type-only import { Idl } left in eventsClient.ts / MockSvmCpiEventsClient.ts is fine — verbatimModuleSyntax is unset, so tsc elides it.
  • No CJS regressiondist/cjs/src/index.js still loads clean (17 exports); the require.main guards are correctly inert in ESM.

One finding (not a blocker, not caused by this PR)

relayFeeCalculator/chain-queries/baseQuery.js throws Cannot access 'QueryBase' before initialization when imported directly/first under native ESM. Root cause is a pre-existing circular import — baseQuery.ts:25 does import { arch } from "../..", pulling the package root barrel into a module the barrel itself re-exports (root → relayFeeCalculator → chain-queries → customGasToken does extends QueryBase before baseQuery finishes). Same applies to svmQuery.ts:36 and tvmQuery.ts:1.

Two reasons it's not a blocker for this PR:

  1. It's not introduced here — line 25 is untouched, and the current CJS build fails identically (Class extends value undefined) when baseQuery is required standalone. ESM now just matches CJS behavior.
  2. Via the barrels / package index (the realistic consumer path) both builds load fine.

Worth a follow-up since the PR headline is "loadable under native loader" and this is a standalone-deep-import counterexample. Low-risk narrow fix that breaks the cycle in both builds:

// baseQuery.ts / svmQuery.ts / tvmQuery.ts
- import { arch } from "../..";
+ import * as arch from "../../arch";

(src/arch/index.ts already exposes evm/svm/tvm, so arch.evm.getAuxiliaryNativeTokenCost(...) at baseQuery.ts:176 is unchanged.) Happy to push this if you'd like it in-scope; otherwise it's a clean separate change.

Verdict: LGTM — approve. The fix does what it claims; the one caveat above is pre-existing and orthogonal.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: ce81200dfb

ℹ️ 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".

@droplet-rl

Copy link
Copy Markdown
Contributor Author

Thanks Codex — that matches my manual pass (built the branch and exercised the actual dist/ ESM output under Node 22; everything in the consumer surface loads clean, no CJS regression).

To consolidate for whoever merges: the only open item is the non-blocking one from my earlier reviewbaseQuery.js/svmQuery.js/tvmQuery.js fail when deep-imported standalone due to a pre-existing root-barrel cycle (import { arch } from "../.."). It predates this PR (current CJS build fails identically) and is fine via the barrels/index, so it's safe to merge as-is. Optional one-line fix if you want it in-scope: import * as arch from "../../arch" in those three files — say the word and I'll push it.

No changes needed otherwise. LGTM.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

@gsteenkamp89 gsteenkamp89 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@droplet-rl For my understanding, why are we adding file extensions to some imports but not all?

@droplet-rl

Copy link
Copy Markdown
Contributor Author

@gsteenkamp89 Good question — it looks selective in the diff, but the underlying rule is uniform: Node's ESM loader needs an explicit file extension exactly when a specifier resolves to a literal file path it can't otherwise complete. That splits imports into three buckets:

1. Relative imports (./, ../) — all need extensions, but you won't see them in this diff.
They're rewritten at build time by the new tsc-alias --resolve-full-paths step (fix-class #1), not hand-edited. So src/ stays extensionless (zero churn) and the emitted dist/esm gets .js / /index.js on every one of them (~600 specifiers). That's why the source diff shows no extension changes on relative imports.

2. Bare-package imports to a package root or its declared subpath exports — no extension, intentionally.
ethers, lodash, winston, @solana/kit, @across-protocol/contracts (root), etc. Node resolves these through the dependency's own package.json (exports/main/module). Adding .js here would break them, so they're correctly left alone.

3. Bare-package imports that reach a literal file inside a dependency's tree — these get .js, by hand.
ethers/lib/utils, @across-protocol/contracts/dist/..., @eth-optimism/sdk/dist/..., arweave/node/lib/.... These point at an actual file in a dep that has no exports map to remap the subpath, so Node's ESM loader needs the exact filename. And critically, tsc-alias only rewrites relative specifiers — not bare-package ones — so these are the only extension edits that show up in the source diff (fix-class #2).

So: every relative import → extension (auto, at build); deep file-paths into exports-less deps → extension (manual, in source); package-root / named-subpath imports → no extension (resolved via the dep's package.json). The diff just makes (3) visible while (1) happens silently in the build.

@droplet-rl

Copy link
Copy Markdown
Contributor Author

🔎 View trace

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.

2 participants