feat: report detached re-exports of imported bindings without a local name#222
feat: report detached re-exports of imported bindings without a local name#222BridgeAR wants to merge 3 commits into
Conversation
guybedford
left a comment
There was a problem hiding this comment.
I'm fine to include this, but the new tracking paths should be gated off on the min build so this doesn't affect the min build and is a full build feature only.
Also it seems like import {'a-b' as c} doesn't work - I'm unclear why we wouldn't just be using the direct string reader here?
… name
A detached `export { x }` (no `from` clause) emits an identical export
record whether `x` is a locally-declared binding or one introduced by an
`import`. A consumer that needs to resolve re-exported imports from the
leaf module's namespace (import-in-the-middle) cannot tell the two apart
and is forced onto a slow fallback.
Record the local binding names an import introduces - named specifiers
(the `as` target, or the imported name when there is no `as`), the
default binding, and a `* as ns` namespace - and, when a detached export
resolves to one of them, report it with no local name (`ln === undefined`,
`ls === le === -1`), exactly as `export { x } from` already does. A
genuine local keeps its name.
The match is by name only: a lexer cannot do scope analysis, so an import
that follows the export (single pass) or a same-named local that shadows
an import is not resolved - the same assumption the `export { x } from`
case already makes. The named part of a combined `default, { a }` clause
is not tracked; such a re-export keeps its local name.
This fixes the detached re-export classifier so import bindings with comments after `as` are still recorded by their target name. Without this, `as` itself could be mistaken for an imported binding and later local exports named `as` were reported as re-exports.
The minimal build must retain its existing export records, so it compiles the imported-binding tracking state and parser paths out.
String-named imports such as `import {'a-b' as c}` were mistaken for module specifiers by the generic quote scan. Parse the named clause before reading the module string.
3380b21 to
278b4a2
Compare
guybedford
left a comment
There was a problem hiding this comment.
One more thought before we land this actually. I did previously specify what the ideal module analysis might look like in https://github.com/tc39/proposal-esm-phase-imports#what-was-source-analysis-removed-from-this-proposal.
If we're taking all this trouble to track these things, perhaps we should just work towards something more like that model?
Could we just make this part of the major and instead treat export as a union of DirectExport | Reexport | ReexportAll where only DirectExport has a local name?
Summary
A detached
export { x }(nofromclause) emits an identical export record whetherxis a locally-declared binding (const x = 1) or one introduced by animport(import { x } from './m'). A consumer that needs to resolve re-exported imports from the leaf module's namespace — import-in-the-middle — cannot tell the two apart and is forced onto a slow fallback.This records the local binding names an
importintroduces and, when a detached export resolves to one of them, reports it with no local name (ln === undefined,ls === le === -1), exactly asexport { x } fromalready does. A genuine local keeps its name.Recorded bindings:
astarget, or the imported name when there is noas(import { a, b as c }→a,c). A string name (import { "x" as y }) binds only viaas.import d from './m'→d).import * as ns from './m'→ns), including thedefault, * as nscombination.Why
export { x }wherexis imported is semantically a re-export, so it should be reported the same wayexport { x } fromis (the C already setslocal_start = local_end = NULLthere). Today the two are indistinguishable, which forces the consumer to guess.The match is by name only — a lexer cannot do scope analysis. Two consequences, both the same limitation class the existing
fromcase already lives with:importthat follows the export is not seen when the export is finalized (single pass), soexport { x }; import { x } from './m'keepsx's local name.The named part of a combined
import d, { a } from './m'clause is not descended into, so a detached export of one of those named bindings keeps its local name (a rare form; the specifier scan there is intentionally opaque).The pure-JS reference lexer (
lexer.js) is mirrored so it stays in step with the C/Wasm and asm.js builds.Artifacts
lib/anddist/are.gitignored, so this PR carries source + tests only; CI regenerates the artifacts. Verified locally through the Docker toolchain (chomp test): all four variants (wasm, minimal wasm, asm.js, minimal asm.js) pass, 158 tests each, including the newDetached re-export of an imported bindingsuite.