feat: lex type-only TypeScript imports and exports - #220
Conversation
fdfdeca to
c18c022
Compare
guybedford
left a comment
There was a problem hiding this comment.
Thanks Ruben — this is a strong first increment: the write-head sentinel guards are correct, the minimal build reader/typings are properly insulated, and the inline-modifier semantics get the subtle case right (import { type A } keeps a runtime edge in Node's stripping output, so statement-level tp: false there is exactly right). Requesting changes on the items below.
Bug: import type from 'x' must be tp: false
Verified against both Node stripTypeScriptTypes (v26) and tsc 5.9 verbatimModuleSyntax: this form is a value import of the default export bound to the local name type — both keep it verbatim. Only import type from from 'x' (default import named from) is type-only:
import type from 'm'; => kept verbatim (value import)
import type from from 'm'; => stripped (type-only)
The PR marks the first tp: true, and the test comment ("Node strips the whole import") is incorrect. Since import type from 'x' is also valid plain JavaScript, a consumer eliding tp: true imports would drop a real runtime import from a pure-JS file — this breaks the JS-superset guarantee the PR is built on. The type follower exclusions need a third case: from followed (after whitespace/comments) by a quote. Please add import type from from 'x' as the disambiguation test.
Gating must be complete — zero cost to the minimal build
src/lexer.c is fully behind #ifdef LEX_TS, but src/lexer.h is not:
bool type_only;onstruct Import/struct Export- the
type_only = falsestores inaddImport/addExport - the
itp()/etp()accessors
The minimal build currently pays two live stores per record and +4 bytes heap per export (the bool lands between pointer fields in the min Export layout), and only DCE keeps itp/etp out of the min wasm. All of these need #ifdef LEX_TS. This composes with the asm requirement below: the asm build defines LEX_TS and gets the field for real; the minimal build carries zero trace.
asm.js build must be included
The hand-maintained-dictionary blocker no longer exists: the build now auto-extracts the keyword dictionary from the fastcomp memory image (the lib/lexer.asm.in.js task scans lexer.layout.js.mem and substitutes {{WORDS}}/{{OFFSET}}), so adding YPE propagates automatically. Concretely:
-D LEX_TSand_itp/_etpinEXPORTED_FUNCTIONSon both fastcomp tasks (lib/lexer.emcc.asm.jsand the layout sidecar must keep identical layout-affecting flags)src/lexer.asm.jsnon-minimal branch:tp: !!asm.itp()instead of hardcodedfalse- run the
test/typescript/suites underASM=1—_harness.cjshard-importsdist/lexer.js; give it the sameWASM/ASMenv switch the legacy suites use - fix the now-stale "MANUAL ASM DICTIONARY CONSTRUCTION" note at the top of
lexer.c, and the README/lexer.tsJSDoc claims that the asm build is JS-only
Type declarations must be tracked
export type Foo = ... and export interface Foo {} need to record tp: true exports — top-level type / interface dispatch from the statement-start check exactly like import/export. Implementation notes:
- For
type Foo, read the name, then stop at the=and skip the RHS. Note default type parameters put a=before the real one (type Foo<T = string> = ...), so after the name skip a balanced<...>region first (>>closes two,=>in function types doesn't close,{}/[]/()nest inside). The same angle skipper servesinterface Foo<T> extends Bar<T> {. - Interface bodies should be skipped opaquely (brace-matching, not the main token loop): a member like
import(): voidotherwise matches the dynamic-import dispatch and records a bogus import edge — a hazard that exists latently today for bareinterfaceblocks. - Guards for the JS superset: require an identifier-start char after the keyword (
type = 5,type(x)are plain JS) and no line break betweentypeand the name (type\nX = 5is two JS statements via ASI). - Dotted names can't be declared (
export type Foo.Baris invalid TS); qualified names only appear as RHS references (export type Bar = Foo.Bar;), which the RHS skip covers for free.import Bar = Foo.Bar/export importare non-erasable and stay out of scope. - Scope boundary: bare
interface Foo {}+ laterexport { Foo }should not be resolved — Node stripping does no cross-binding analysis (erasable TS requiresexport { type Foo }), so only directly-exported declaration forms are marked.
Smaller items
isTsTypeKeywordfollower set missesimport type/*c*/{ A } from 'm'(verified: Node strips it; the lexer reportstp: false) andimport type* as ns from 'm'. Accepting/and*as followers is safe — the callers' savePos/restore logic already disambiguates.- The single-token
aslookahead in the export brace loop mishandlesexport { type as/*c*/T }andexport { type as as X }(type modifier on a specifier namedas). Pathological, but worth a deliberate call — at minimum a comment. - README opening ("lexes the erasable TypeScript syntax that Node.js type stripping accepts") overstates this increment; scope it to the type-only subset until the annotation work lands.
- Untested:
export type * from 'x'(traced as correct — import edgetp: true— but pin it), and thetp: falseadditions in the JS builds once the harness is env-parameterized. - Nit:
js-build-unchanged.cjsruns against the wasm build;js-superset.cjswould name what it actually asserts.
c2fe15e to
e32a1c8
Compare
|
This is still a WIP PR with some issues and not yet ready. It should move in the right direction though. |
guybedford
left a comment
There was a problem hiding this comment.
This is really nicely done — pinning the accept/reject boundary to Node's stripTypeScriptTypes and backing it with a differential fuzzer is exactly the right approach for an opaque skipper, and it clearly earned its keep (several tests are annotated "fuzzer-found"). The minimal build staying zero-cost behind #ifdef LEX_TS and the JS-superset guarding are both thorough.
Two things I'd like addressed before merge:
1. import type from'x' (no space before the quote) is mis-marked type-only. In the import type detection:
if (!typeIsBinding && nextCh == 'f' && memcmp(pos + 1, &ROM[0], 3 * 2) == 0 && isBrOrWs(*(pos + 4))) {The from-followed-by-quote check requires whitespace after from. import type from'x' is valid JS (type default binding, from keyword, 'x' with no separating space), but it fails isBrOrWs(*(pos + 4)), so typeIsBinding stays false and the import is reported tp: true — dropping a real runtime edge from a file that is also valid JavaScript, which breaks the "no JS consumer observes a change" invariant. The fuzzer misses it because the import-type-from form always emits a space before the specifier. Fix is to also accept a quote here, e.g. isBrOrWs(c) || isQuote(c), and add the no-space case to the fuzzer's form.
2. Type/interface declarations nested in a block still leak their import(...) types. Both bare triggers gate on openTokenDepth == 0:
if (*(pos + 1) == 'y' && openTokenDepth == 0 && keywordStart(pos))
tryTsTypeDeclaration(true);So function f() { type X = import('m').T; } — which Node strips cleanly — reports import('m') as a runtime edge. This is a remaining gap rather than a regression (everything leaked before), and the openTokenDepth == 0 guard is a reasonable defense against expression-context misfires, so I'm fine leaving the behaviour for a follow-up. But the README currently says type and interface declarations are skipped "whether exported or not" with no nesting caveat — please add a one-line scope note there so the limitation is documented, and ideally extend the fuzzer to emit nested declarations (it only generates top-level statements today, so it can't surface this class).
3. Prefer forcing the keyword dictionary contiguous over reconstructing it from a gappy span. The gen-asm-in.mjs change reconstructs the dictionary across the whole first-to-last span, filling gaps with NUL, because adding the new tables made the compiler place nterface at a non-adjacent offset. That works, but scanning first-to-last across the entire memory image is sensitive to any unrelated printable-char16 constant landing between the tables (it would widen the span and bloat words with a long NUL run).
The split happens only because each keyword is a separate static const char16_t[] object, which the compiler/linker may reorder or interleave. Merging them into one array and offsetting into it keeps the object contiguous on both toolchains (no linker script or section attributes needed — those don't actually guarantee intra-section ordering):
static const char16_t KEYWORDS[] = {
'x','p','o','r','t', // xport
'm','p','o','r','t', // mport
/* ... */
'y','p','e', // ype
'n','t','e','r','f','a','c','e', // nterface
};
#define XPORT (KEYWORDS + 0)
#define MPORT (KEYWORDS + 5)
/* ... */The call sites already pass explicit lengths (memcmp(pos + 1, &MPORT[0], 5 * 2)) and never use sizeof, so only &NAME[0] becomes NAME. That lets the asm extraction go back to "grab the single contiguous run", drops the NUL-gap span logic and its end - start fragility, and the contiguous blob (no gap NULs) is likely smaller than today's span.
229afd6 to
a03578e
Compare
Review — type-only TypeScript import/export lexingThe architecture is sound and test coverage is reasonable, but the new speculative TS-skipping paths in Correctness (all confirmed)1. 2. 3. 4. 5. These five share a root cause worth fixing at altitude: after an opaque declaration erasure, the skipper should restore tokenizer state to what an erased statement looks like — leave Efficiency (confirmed, lower severity)6. 7. Minor
|
d0e2fa7 to
593ca50
Compare
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.
guybedford
left a comment
There was a problem hiding this comment.
LGTM, we must just make sure to land this on v3 not main.
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.
Detached exports can precede their imports, so source-order tracking cannot preserve their module request. Resolve complete module bindings and link reexports to the imports array. On Node 24 / V8 13.6, imports plus a direct export measured 5.08 -> 5.28 us, while 1,000 detached reexports measured 169.3 -> 145.2 us export-first and 166.5 -> 159.0 us import-first (seven trials, drop best and worst). The minimal build remained within noise.
Binding capture was gated on a JS-side prescan of the whole source, so a module's classification depended on its length and both wrappers paid the scan on every parse. On the sample corpus that scan cost more than the lexing it guarded: 722 us of the 878 us angular.js regressed, and effectively all of the asm.js build's. Capturing unconditionally is never slower, because the binding scanner also replaces a tolerant clause loop that called commentWhitespace per character: 1000 imports with direct exports measure 98.9 us captured against 141.7 us through that loop. The gate, eac(), the record re-parse and the loop are gone, and one scanner serves every source. The single scanner exposed four defects the size gate had hidden: 1. `export * from <non-string>` stored through a null import head; in the asm.js build that overwrote a keyword table, silently dropping every import from every later parse in the process. 2. A bare `export *` emitted a reexport record with no import, so both wrappers threw a TypeError instead of a ParseError. 3. An import-clause character that cannot start a binding was re-read forever. 4. `as` directly followed by a comment did not read as a rename. The asm.js analysis arena is now bounded in C and re-parsed into a larger buffer on overflow instead of truncating records silently, which retires the last two full-source scans. Against the previous commit the corpus runs 26% faster and the Wasm binary drops 3197 bytes. The feature now costs 2.3% and 2802 bytes over the unmodified lexer.
`br` is loop invariant, so testing it on every character of a comment body only costs. Comment-heavy sources gain the most: angular.js drops 7.8%, 8.6% and 8.9% across three runs on Node 24.18.0 (best of 21 interleaved trials), for 37 bytes of Wasm.
Export origin analysis needs to know which import a re-exported local name is
bound by, and storing a record per binding during the parse bills every module
that imports names. The answer is only read for detached exports — `export
{ a }` where `a` came from an import — which most modules do not have. The
bindings are still in the source, so a module that ends the parse with an
unresolved export re-reads its import clauses in the finalize pass and resolves
the names against the export table as it goes; a module without one never reads
a clause twice.
`ImportBinding` and its list are gone, along with the per-binding allocation
and its capacity check.
This speeds up every module with a named import clause. Only the finalize pass
resolves bindings, and only when a detached export exists to consume them, so
the main loop needs the position of the closing brace and nothing else. A scan
stepping over the four constructs able to hide a brace — string specifiers,
line comments, block comments and `\u{...}` escapes — replaces the specifier
reader there, and one entry point picks between the two so they cannot
disagree about where a clause ends.
Wasm, min of 21 runs in one process: imports without detached exports -10.4%,
detached export-first -6.3%, detached import-first -6.0%, a module ending in a
clause export -4.0%, angular.js -4.2%.
Open addressed inserts probe for a free slot, so every export sharing a local
name walks past all the earlier ones and a clause builds its table in quadratic
time. Exporting one local under 200,000 names — `export { a as x0, a as x1 }`,
valid and reachable from a generator — took 10.7 seconds where the same count
of distinct names took 11 milliseconds. Bundlers and CDNs run this parser on
untrusted input.
Chaining the bucket makes an insert an unconditional prepend with no comparison
at all, and one walk resolves every export a name covers. The same 200,000
aliases now take 17 milliseconds, and a corpus A/B is unchanged.
`ensureAnalysisCapacity()` inlines into every record allocation, and carrying the page-grow branch along costs the allocators more than the bounds test it guards. Record-dense sources measured 9.8% slower than an unchecked allocator; with the grow path out of line they are within noise of it at 0.5%, so the arena keeps its bound without the records paying for it.
A module with a clause export and no import clause — `const a = 1; export
{ a }` — allocates, zeroes, populates and sweeps a hash table that has nothing
to match against, and hashes every pending export name to fill it. Only a
binding introduced by an import clause ever resolves one of those exports.
The clause readers now record that the module has such a binding, and the name
hash moves to the table insert so it happens only where a table exists. That
shape drops from 156.6 ns to 147.2 ns, against 136.2 ns for the same build with
the analysis removed entirely, and the rest of the corpus is unchanged.
Re-reading import clauses in the finalize pass is the dominant cost of the
analysis: 8 of the 12 microseconds a 120-import module spends over a build
without it, and 13 of 15 on a clause-heavy one. Most of those clauses cannot
contain the name being looked for.
An import statement now records the 32-bit set of characters its binding names
are drawn from, accumulated in the brace-skip on code units it already loads. A
pending export name that occurs in a clause necessarily has its first character
in that set, so an import missing every pending first character is skipped
without being read. An escape sets the mask to all ones on either side, because
`\u{61}` and the `a` it denotes share no code unit.
That module goes from 63.4 us to 59.7 us, against 56.2 us for the same build
with the walk removed outright. Sources where every import really does bind an
exported name are unchanged.
…ding export" This reverts commit 96e7c1b. The filter was measured against fixtures carrying 120 and 400 import statements. Real modules that reach the finalize walk carry a median of 2: across 878 ESM files in two node_modules trees, 50 run the walk at all, between them re-reading 186 import clauses, of which the filter skips 34. Parsing that corpus measures 0.4% to 3.3% slower with the filter over four runs, and slower in every one of them on the 93 files that run the walk. The per-code-unit mask accumulation costs more than the clause reads it avoids.
## Summary Export metadata reads the same import records that carry interpolated-template spans. Keep the span fields, readers, and parser recording when export analysis is enabled so dynamic import globs remain available. ## Why The full build needs both metadata sets from one import record. Minimal builds compile the glob fields and readers out. ## Test plan - chomp test
## Summary Teach the full Wasm and asm.js builds to lex type-only imports, exports, type aliases, and interfaces accepted by Node.js type stripping. Type-only module edges carry `tp: true`, and erased declaration bodies do not leak nested `import()` types into the runtime graph. ## Why Plain JavaScript keeps the same graph. Minimal builds remain JavaScript-only and compile the TypeScript paths out. Import metadata uses bit 16 above the public import-type range, while export metadata uses existing struct padding, so star re-export type 8 remains distinct. ## Test plan - chomp test - TypeScript differential fuzzer against `stripTypeScriptTypes` Fixes: guybedford#72
593ca50 to
e6bf1b0
Compare
Summary
Teaches the full Wasm and asm.js builds to lex the type-only TypeScript subset that Node's
stripTypeScriptTypeserases: type-only import/export clauses andtype/interfacedeclarations. Type-only module edges are reported withtp: true, and declaration bodies are skipped opaquely so nestedimport()types do not leak into the runtime graph.Why
A single lexer avoids a separate TypeScript transform and gives module-graph consumers both runtime and type-only edges. Plain JavaScript remains metadata-compatible. TypeScript declaration lookahead only runs after shallow inline keyword gates, and
tpreuses existing import/export reader values instead of adding a Wasm boundary call per specifier. Minimal builds remain JavaScript-only and compile out the type-only fields and paths.The supported boundary follows Node's type stripping for these forms. Value-position annotations and generics,
as/satisfies, non-null!, and non-erasable syntax such asenumand runtimenamespaceremain out of scope.Fixes: #72