fix(esm): make dist/esm runtime-importable under Node strict ESM - #1451
Closed
0xjorgen wants to merge 6 commits into
Closed
fix(esm): make dist/esm runtime-importable under Node strict ESM#14510xjorgen wants to merge 6 commits into
0xjorgen wants to merge 6 commits into
Conversation
`build:esm` emits `dist/esm/` with `"type":"module"` in `package.json`,
but tsc preserves source import specifiers as-written. Under Node's
strict ESM resolver this produces three classes of unloadable output:
1. Extension-less relative imports (`from "../utils/NetworkUtils"`)
that Node's strict resolver rejects with `ERR_MODULE_NOT_FOUND`.
2. Named imports from CJS-only deps (`import { clamp } from "lodash"`)
that Node can't statically analyze, throwing
`Named export 'clamp' not found`.
3. `require.main === module` CLI guards in source, which crash with
`require is not defined in ES module scope` when the file is loaded
as ESM.
The failures only surface when downstream consumers load the built
files under Node's ESM runtime — Vite SSR, Next.js SSR, vm modules,
etc. They didn't surface before across-protocol#326 in `across-protocol/dapp` because
the SDK wasn't part of the SSR module graph there.
This change addresses all three at the level closest to the cause:
- **`scripts/add-esm-extensions.mjs`** — post-build rewriter that
walks `dist/esm/**/*.js` and adds `.js` or `/index.js` to every
relative `import`/`export`/`import()` specifier. Pure Node, no new
deps. Source stays idiomatic TS; the build emits Node-compatible
ESM. `build:esm` now runs it after `tsc`.
- **`scripts/esm-node-ssr-smoke-test.cjs`** + `test:esm-smoke`
script — asserts every important entry point in `dist/esm` loads
under Node's ESM runtime. Catches regressions where new SDK code
reintroduces any of the three failure modes.
- **CJS default-import + destructure** for the runtime values
imported by name from `lodash` (7 files) and `@coral-xyz/anchor`
(`arch/svm/utils.ts`). Type-only imports stay as named imports —
TS strips them.
- **`.js` / `/index.js` on bare deep imports** into packages that
expose deep paths without an `exports` map (`@across-protocol/contracts/dist/...`,
`@eth-optimism/sdk/dist/...`, `ethers/lib/utils`,
`arweave/node/lib/...`). 9 files.
- **`typeof require !== "undefined"` guard** on the two CLI entry
points (`addressAggregator/index.ts`, `arch/evm/utils/wait.ts`)
so importing them as ESM no longer crashes on the `require.main`
check.
`viem@2.47.10` declares `@noble/hashes@1.8.0` in its own package.json, but the existing yarn.lock has it resolved to `1.4.0` under `viem/node_modules`. The 1.4.0 build is missing the `anumber` named export that `@noble/curves@1.9.1` (a sibling install under `viem/node_modules`) imports at module top level. Result: any downstream consumer that loads a viem code path under Node's ESM runtime crashes with `Named export 'anumber' not found`. Force `**/@noble/hashes` to >=1.8.0 via `resolutions`. yarn drops the nested install, all consumers share the top-level 1.8.0, and the viem→curves→hashes import chain resolves cleanly. Surfaces as part of the same fix surface as the ESM build pipeline changes in the previous commit — both were blocking dist/esm from being importable under Node strict ESM.
…undles `import anchor from "@coral-xyz/anchor"` worked under Node ESM (`default` exposes the CJS `module.exports`) but failed Vite's browser bundle: anchor's `browser` field points at a proper-ESM build with only named exports — no `default`. Roll-up errored with `"default" is not exported by .../dist/browser/index.js` while optimizing the dapp's deps. Use a namespace import and reach through `default` only when it exists. Works in three resolution scenarios: - Vite browser → `dist/browser/index.js` (proper ESM, named only): `anchorModule.default` is undefined, fall through to the namespace. - Node ESM CJS interop → `dist/cjs/index.js`: `anchorModule.default` is `module.exports`, use it. - Node ESM module field → `dist/esm/index.js`: named exports populate the namespace directly, same as browser. Verified: rebuilt SDK, repacked, installed in dapp, `pnpm start` serves `/`, `/bridge`, `/privacy-policy` cleanly with no `ERR_MODULE_NOT_FOUND` or `MISSING_EXPORT` in the log.
Two new guardrails so the failure modes the previous commit fixed
can't silently regress.
**CI step** — `.github/workflows/build.yml` now runs
`yarn test:esm-smoke` after `yarn build`. Any future change that
ships extension-less imports, named imports from CJS-only deps, or
unguarded `require.main` into `dist/esm` will fail the build job
instead of failing downstream consumers months later.
**Lint rules** — `.eslintrc.cjs` gains a `no-restricted-syntax`
block that flags named imports from `lodash` and `@coral-xyz/anchor`
at the source level, with the suggested replacement in the error
message. AST selectors (`ImportSpecifier[importKind!='type']`) are
used instead of `no-restricted-imports.importNames` so namespace
imports and explicit `import type { ... }` are still allowed —
only runtime named imports are blocked.
**Source clean-ups surfaced by the new lint rule**
- `src/arch/svm/eventsClient.ts` + `src/clients/mocks/MockSvmCpiEventsClient.ts`:
the `Idl` import is type-only — promote to `import type { Idl }`
so it's both explicit at the source level and lint-clean.
- `test/validatorUtils.test.ts`: rewrite the `cloneDeep` import to
the default-import-and-destructure form so tests follow the
same pattern as production source.
The earlier `**/@noble/hashes: "^1.8.0"` glob was too broad. It also hoisted `ethereum-cryptography`'s pinned `@noble/hashes@1.3.1` to 1.8.0, breaking `ethereum-cryptography/utils.js` at module load: `TypeError: Cannot read properties of undefined (reading 'bool')` — the `_assert` interface changed between those versions. `yarn test test/validatorUtils.test.ts` failed at module load as a result. Scope the resolution to `viem/@noble/hashes` instead. That's sufficient to fix the original viem→curves→hashes anumber chain (viem no longer carries a nested @noble/hashes, so its co-installed @noble/curves@1.9.1 resolves hashes from the top-level 1.8.0). `ethereum-cryptography` keeps its 1.3.1 pin and its tests pass again. Verified: `yarn test test/validatorUtils.test.ts` → 5/5 passing; `yarn test:esm-smoke` → 16/16 OK.
The existing build/lint/test workflows trigger on `[push]`, so they don't run on PRs from forks — only `dependency-review` (which triggers on `pull_request`) showed up as a check on PR across-protocol#1451. That's exactly the gap that hid the dist/esm regressions this PR is fixing for in the first place. Add a single `pull_request`-triggered workflow that runs the trio that actually validates a build is shippable: `yarn lint-check`, `yarn build`, `yarn test:esm-smoke`. The build step has to come before the smoke test because the smoke test imports from `dist/esm`. Existing push-triggered workflows are left alone — they still cover master and intra-repo branch pushes. This file is purely the fork-PR safety net. `pull_request` (not `pull_request_target`) is intentional: it runs without secrets and checks out the PR head, so untrusted code can't exfiltrate anything. The current step set doesn't need secrets.
Contributor
Author
|
Superseded by #1452 — same branch, opened from |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
build:esmemitsdist/esm/with"type":"module", buttscpreserves source import specifiers as written. Under Node's strict ESM resolver this produces three classes of unloadable output, plus a transitive viem dep-tree gap that's been latent in the lockfile.Downstream evidence: when
across-protocol/dapp#326 hoisted Amplitude tracking into the SSR-rendered__root.tsx, the dapp's local Vite dev server started returningHTTPError 500on every request because Vite SSR walks into@across-protocol/sdk@4.3.154's builtBlockUtils.jsvia Node's strict ESM. Bisected to a specific dapp commit but the underlying cause is in this package.The three SDK-source failure modes
BlockUtils.jsshipsimport { ... } from "../../utils/NetworkUtils". Node strict ESM:import { clamp, sortedIndexBy } from "lodash"—lodash(7 files) and@coral-xyz/anchor(1 file). Type-only named imports (import { Idl } from "...") are fine because TS strips them.require.main === moduleCLI guards: 2 source files use this idiomatic CJS entry-point check. Under ESM,requireis undefined and any file containing the literal crashes on load.Fix
scripts/add-esm-extensions.mjspost-build rewriter that walksdist/esm/**/*.jsand adds.js//index.jsto every relativeimport/export/ dynamicimport()specifier. Pure Node, no new deps. Wired intobuild:esmaftertsc. Source stays idiomatic TS..js//index.jssuffixes on 9 imports into packages that expose deep paths without anexportsmap (@across-protocol/contracts/dist/...,@eth-optimism/sdk/dist/...,ethers/lib/utils,arweave/node/lib/...). The post-build script only touches relative paths — bare deep imports must be fixed at source.lodash, 1@coral-xyz/anchor).require.mainguardstypeof require !== "undefined"inaddressAggregator/index.tsandarch/evm/utils/wait.ts. CJS behavior unchanged; ESM stops crashing.scripts/esm-node-ssr-smoke-test.cjs+yarn test:esm-smoke. Asserts 16 representativedist/esmentry points can beimport()-ed under Node ESM. Add new entries when downstream consumers report fresh chains. Easy CI hook.The lockfile follow-up (separate commit)
Surfaced while triaging the above:
viem@2.47.10's ownpackage.jsondeclares@noble/hashes@1.8.0, but ouryarn.lockhas it resolved to1.4.0underviem/node_modules/. The 1.4.0 build lacks theanumbernamed export that the co-installed@noble/curves@1.9.1imports at module top level — so any consumer loading a viem code path under Node ESM getsNamed export 'anumber' not found.Forcing
**/@noble/hashesto^1.8.0viaresolutionsmakes yarn drop the nested install. All consumers share the top-level 1.8.0 and viem→curves→hashes resolves cleanly.Verification
Before:
After:
yarn build:cjsstill produces a working CJS bundle (sanity-loadedBlockUtils.jsfromdist/cjs).yarn build:typesclean.yarn lint-checkclean.What this does NOT change
build:cjsdoesn't run the post-build script, and source-level changes (default-import-and-destructure, bare-import.jssuffix,require.mainguard) are all CJS-safe..jsin source — enforceable later with a lint rule if desired.Follow-ups (not in this PR)
test:esm-smokeinto CI so this can't silently regress.across-protocol/dappcan drop the lazy-mount workaround opened in across-protocol/dapp #344 and depend on this fix directly.🤖 Generated with Claude Code