fix: resolve ESM-only exports conditions when packaging (#281)#283
Open
robertsLando wants to merge 1 commit into
Open
fix: resolve ESM-only exports conditions when packaging (#281)#283robertsLando wants to merge 1 commit into
robertsLando wants to merge 1 commit into
Conversation
Packaged ESM apps crashed at startup when importing a dependency whose package.json "exports" map is valid for `import` but not resolvable through CommonJS `require()`. pkg transforms ESM to CJS and renames .mjs files to .js in the snapshot, but left the "exports" field untouched. Node's CJS resolver always prefers "exports" over "main", so at runtime it resolved the original, unconverted conditions and failed with: - ERR_PACKAGE_PATH_NOT_EXPORTED for import-only packages, and - MODULE_NOT_FOUND for targets pointing at the renamed .mjs files. The previous synthetic-"main" workaround could never take effect because Node ignores "main" whenever "exports" is present. Rewrite the "exports" field in the snapshotted package.json to mirror the ESM->CJS transformation pkg already applies: rewrite every .mjs target to .js, and add a synthetic `require` condition (mirroring `import`/`default`) to any conditions object that lacks a CJS-resolvable condition. Add unit tests for normalizeExportsForCJS and an e2e regression test (test-54-esm-exports-conditions) covering both failure cases.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
==========================================
+ Coverage 86.36% 87.12% +0.76%
==========================================
Files 22 22
Lines 7297 7389 +92
Branches 1047 1076 +29
==========================================
+ Hits 6302 6438 +136
+ Misses 988 943 -45
- Partials 7 8 +1
🚀 New features to boost your workflow:
|
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.
Closes #281
Problem
A packaged ESM app crashes at startup when it imports a dependency whose
exportsmap is valid for ESMimportbut not resolvable through CommonJSrequire(). Plain Node runs fine; the packaged binary builds but crashes at runtime.Two cases (from the repro):
{ "exports": { ".": { "import": "./index.mjs" } } }→ERR_PACKAGE_PATH_NOT_EXPORTED.mjstargets →MODULE_NOT_FOUND .../index.mjsRoot cause
pkg transforms ESM→CJS at build time and renames each dependency's
.mjsfiles to.jsin the snapshot, but left the dependency'spackage.jsonexportsfield untouched. Since Node's CJS resolver always prefersexportsovermain, at runtimerequire('<dep>')resolves against the original, unconvertedexportsmap and fails. The pre-existing "syntheticmain" workaround could never help, because Node ignoresmainwheneverexportsis present.Fix
normalizeExportsForCJS()inlib/esm-transformer.ts(recursive): rewrites every.mjsstring target to.js, and for any conditions object lacking a CJS-resolvable condition (require/node/node-addons/default) injects a syntheticrequiremirroring theimport/defaulttarget.lib/walker.ts: rewrites the snapshot'sexportsfield vianormalizeExportsForCJSin the existing package.json patch block (alongside synthetic-main andtyperewrites). Gated on!seaMode, matching the surrounding ESM-transform gating.Tests
test/unit/esm-transformer.test.ts— 7 unit cases fornormalizeExportsForCJS(string, import-only,.mjs-target, default fallback, no-op when CJS condition present, nested/subpath recursion, arrays).test/test-54-esm-exports-conditions/— e2e regression mirroring the repro (esm-onlyimport-only package +req-mjs.mjs-target package). Fails without the fix (exit 1), passes with it (exit 0).Verification
tsc --noEmit, prettier, eslint all clean; existing ESM e2e tests (test-50/51/52/53) still pass.🤖 Generated with Claude Code