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 10 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?
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?
* perf(lexer): skip identifier and number runs On the 3,057 KiB sample corpus, Wasm drops from 15,356.31 to 13,010.46 us per sweep on Node 18.20.8 (-15.28%) and from 11,724.34 to 9,856.69 us on Node 24.18.0 (-15.93%). The asm.js build drops from 15,934.34 to 12,945.57 us on Node 24.18.0 (-18.76%). The fast path adds 379 raw / 105 gzip bytes to the full Wasm binary. Measurements used 1,500 warmup sweeps followed by 9 interleaved trials of 50 sweeps, dropping the best and worst trials.
|
I will have a look into this and determine if #215 can be replaced with that as well |
lexer.js reimplemented the WebAssembly lexer in hand-written JS, kept in sync by hand, but was unreachable through the package `exports` (only `.` and `./js` are exposed - the wasm and asm.js builds) and `chomp test` never ran it, so it could drift silently. The asm.js build already covers the no-WebAssembly case, and the removed `if (!js)` test guards now run unconditionally, matching their existing behavior under the wasm and asm suites.
A dynamic import whose entire argument is a template literal with substitutions
returned n: undefined, so a consumer resolving the specifier (a bundler or glob
importer) had nothing to work with. It now reports the static skeleton as a
glob with each ${...} collapsed to a single "*": import(`./locales/${x}.js`)
yields "./locales/*.js".
Only a lone template literal qualifies. A template concatenated with anything
else (import(`a` + b)) has no static skeleton and still returns undefined.
Fixes: guybedford#137
The glob walker that builds a dynamic-import template skeleton skips over
strings, nested templates, and comments inside each ${...} substitution, but it
cannot tell a regex literal from division without the main parser's token
context. A regex carrying a "}" closed the substitution early and emitted a
wrong specifier instead of bailing: import(`a${ /x}y/g }b`) reported "a*y/g }b"
rather than undefined.
skipInterpolation now flags a bare "/" (one that does not open a // or /*
comment) and the three decoders drop n to undefined rather than guess. This
over-bails the rare division case (import(`a${ b/c }d`)), which is acceptable:
a missing glob is recoverable, a wrong one is not.
Fixes: guybedford#137
The interpolated-template glob walked each ${...} substitution with a
hand-rolled scanner in all three decoders. None of them could tell a regex
literal from division without token context, so a regex carrying a "}" closed
the substitution early: import(`a${ /x}y/g }b`) reported "a*y/g }b". The prior
fix bailed to undefined on any bare "/", which also dropped legitimate division
(import(`a${ b/c }d`)).
The parser already resolves regex vs division for the whole source and descends
into ${ ... } for nested-import detection, so it now records each top-level
substitution's end on the dynamic import (struct TemplateSpan). The decoders
splice a "*" per span and jump the body, dropping their skipInterpolation /
skipQuoted / skipComment scanners and the interpolationError bail. Both
ambiguous cases now resolve correctly: "a*b" and "a*d".
Fixes: guybedford#137
…ects ## Summary Interpolated template glob tracking leaked into minimal builds and diverged between decoders. Missing parser spans could evaluate a substitution, reload an exhausted span list into a loop, or lose the outer glob around a nested import. ## Why Keep span tracking behind LEXER_MIN, use parser-recorded spans as the substitution boundary, and copy static source without eval. The full Wasm and asm.js builds now agree while minimal output stays unchanged. Static parts remain raw source, so escapes are not cooked and a literal * stays literal. ## Test plan - chomp test Fixes: guybedford#137
## Summary - Add `StaticReexportStar` as import type 8 for `export * from`. - Report the matching `*` export with a span that correlates to the import. - Cover full and minimal builds without reading fields omitted by the minimal API. ## Why Star re-exports are dependency edges and exports, but the lexer only exposed their specifier as a plain static import. Consumers had to recover the missing distinction by parsing the source again. ## Test plan - `chomp test:wasm test:minimal:wasm` Refs: guybedford#76 Refs: nodejs/import-in-the-middle#259
… 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.
278b4a2 to
dc60ec0
Compare
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.