fix: resolve re-exported values from their defining module#263
Open
BridgeAR wants to merge 2 commits into
Open
fix: resolve re-exported values from their defining module#263BridgeAR wants to merge 2 commits into
BridgeAR wants to merge 2 commits into
Conversation
BridgeAR
force-pushed
the
BridgeAR/2026-06-30-star-reexport-identity
branch
4 times, most recently
from
f1fd80f to
fd57f77
Compare
BridgeAR
requested review from
bengl,
jsumners-nr and
timfish
and removed request for
jsumners-nr
timfish
previously approved these changes
Jun 30, 2026
A name reached through two `export *` re-exports that both bottom out at the same defining module is one binding, not an ambiguous collision, so it must stay exported (ECMAScript ResolveExport; tc39/ecma262#3715). processModule excluded it because addSetter treated any name arriving from two `*` re-exports as ambiguous, regardless of where it was defined. Track the defining module per name and exclude only when two `*` re-exports name it from different modules, matching the spec and Node's own resolution. The dedup bookkeeping is allocated lazily on the first `export *`, so a module without one allocates nothing beyond the setter map it already built — keeping the common case free on the startup-time wrap path. This restores the export name; wiring the value through the wrapper is a separate change. Fixes: nodejs#171
BridgeAR
force-pushed
the
BridgeAR/2026-06-30-star-reexport-identity
branch
from
fd57f77 to
619e04f
Compare
A binding re-exported into one module through more than one `export *` chain that all bottom out at the same defining module stays exported (ECMAScript ResolveExport; tc39/ecma262#3715), but it read back undefined: the wrapper took every name off the aggregating module's namespace, and because iitm wraps each chain as a distinct module Node sees that aggregate re-export as ambiguous and exposes nothing. Read such a name from the module that defines it instead, which always holds the value, by importing that module's namespace into the wrapper under its own alias. The alias and its import are minted only for a name that actually survives the same-origin collision; every other export, including the common non-colliding `export *`, keeps reading from the wrapped `namespace` and adds no import. A module with no such collision allocates no registry, so the startup-time wrap path stays free in the usual case. Fixes: nodejs#171
jsumners-nr
previously approved these changes
Jul 1, 2026
BridgeAR
force-pushed
the
BridgeAR/2026-06-30-star-reexport-identity
branch
from
619e04f to
e0dc7af
Compare
timfish
approved these changes
Jul 1, 2026
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
3 participants
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.
A name that reaches a module through two export * re-exports which both bottom out at the same defining module is one binding, not an ambiguous collision, so it must stay exported. processModule dropped it because addSetter treated any name arriving from two * re-exports as ambiguous regardless of where it was defined. Per ECMAScript ResolveExport, ambiguity requires the name to resolve to different modules (or different binding names); the same binding reached through more than one chain resolves to one module and stays exported (tc39/ecma262#3715).
This tracks the defining module per name and excludes only when two * re-exports name it from different modules — restoring the export name. The genuinely-ambiguous case (export * from './a' + export * from './b' where both declare their own foo) still drops foo, as the existing duplicate-exports.mjs test asserts.
A binding re-exported into one module through more than one
export *chain came backundefinedeven once it was no longer dropped as ambiguous. The wrapper read every name off the aggregating module's namespace, and Node treats the two chains as distinct wrapper modules, so the aggregate namespace leaves the name ambiguous and exposesundefined. The defining module always holds the value, so the wrapper now imports one namespace per defining module and points each re-exported binding's setter at the module that declares it.Fixes: #171