Repository navigation
Conversation
…e export loop A namespace that merges with an exported function or class of the same block registers its member map under the symbol of that function or class. The export loop of `parse_type_script_namespace_stmt` then stored `Property` under the same symbol and replaced the map. A later block of the namespace got an empty map, and a read of an enum member through the merged name did not fold. No reader of `ref_to_ts_namespace_member` matches `Property`. Remove the three stores: the function arm, the class arm, and `define_exported_namespace_binding`.
…s not write it The namespace arm and the enum arm stored the record that they just read back under the same key. Remove both stores. `ts::Data` is `Copy`, so `clone_ts_member_data` has no caller left.
`enum E { A = -3 }` followed by `E.A ** n` printed `-3 ** n`, which is
a SyntaxError. The `**` arm now looks through the inlined enum to the
number.
A read of an enum member through a namespace that merges with an
exported function or class folds since the two parser commits before
this one, so it reaches the same arm.
A later block of a namespace that merges with an exported function or class now gets the members of the first block. It has the gaps that a later block of a plain namespace has: a write to an export of the first block, a read under a binding with the name of the namespace, and a bare read of an enum of the first block. Four `it.todo` rows hold the values that tsc gives.
|
Status Reproduced on main 449877f and on bun 1.4.3. Save this as namespace F { export function fn() {} export namespace fn { export const x = 1 } }
namespace F { export namespace fn { console.log(x) } }bun throws Pull request: #47001. The tests that fail on main are in |
|
Updated 1:13 PM PT - Oct 26th, 2026
✅ @robobun, your commit 97999edd3066160539045c6dd49edb26ce712872 passed in 🧪 To try this PR locally: bunx bun-pr 47001That installs a local version of the PR into your bun-47001 --bun |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Retain members across namespace merges src/js_parser/p.rs, src/js_parser/parse/parse_typescript.rs, src/jsc/RuntimeTranspilerCache.rs, test/bundler/bundler_edgecase.test.ts, test/bundler/transpiler/transpiler.test.js |
The parser no longer stores exported function and class bindings as Property entries in the namespace reference map. It copies stored namespace member data into parent namespaces. Tests cover later namespace blocks, enum values, runtime execution, and unchanged handling of calls, constructions, and non-enum properties. Four follow-up cases remain marked todo. The runtime transpiler cache version increases to 44. |
Check unwrapped enum values for exponentiation src/js_printer/lib.rs, test/bundler/transpiler/transpiler.test.js |
The printer applies exponentiation precedence rules to the unwrapped left operand. Tests expect negative inlined enum values to be parenthesized before **, while positive values remain unparenthesized. |
Suggested reviewers: jarred-sumner, alii
Priority:
Merge Risk: 
97999
-
No actionable merge-blocking risk was found in the reviewed changes; normal test checks remain appropriate.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the main change: stopping Propertyentries from being stored under a symbol reference in the namespace export loop. It is specific and related to the changeset.Description check The description provides a detailed problem statement, fix, scope, known limitations, performance data, and verification results. Although it does not use the template headings exactly, it fully cover… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description provides a detailed problem statement, fix, scope, known limitations, performance data, and verification results. Although it does not use the template headings exactly, it fully covers what the PR does and how it was verified.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked that no reader of ref_to_ts_namespace_member branches on Property (the only remaining Property constructions are the three exported_members.put calls, so the deleted stores had no consumer for non-merged bindings), that unwrap_inlined() returns the expression unchanged for anything other than EInlinedEnum (the ** printer change is a no-op for existing shapes), and that the scope-snapshot rollback in src/js_parser/p.rs still prunes stale map entries, so the Copy read-only arms do not change rollback behavior.
Extended reasoning...
The change removes three Property inserts into the parser's namespace-member map, makes the SNamespace/SEnum export arms read-only, adds a ** left-operand parenthesization for inlined negative enum members in the printer, and bumps the runtime transpiler cache version; it touches no security-sensitive surface. The inline findings cover the semantic deviations from tsc in later merged-namespace blocks and test nits, so a human look is already signaled; this note only records what else was examined.
The spawned `bun -e` check now compares stdout, stderr and the exit code in one assertion. Each `it.todo` row of a later block has a comment with the value that bun gives today.
There was a problem hiding this comment.
The latest push addresses the two nits from my earlier pass (combined stdout/stderr/exitCode assertion, observed value on the fourth todo); I re-ran the review on this version and found no new bugs, but since this is a deliberate deviation from esbuild in namespace-merge semantics, a maintainer should still make the call on the later-block trade-off already discussed inline.
What was reviewed this run:
- Every reader of
ref_to_ts_namespace_member(p.rs:2225, :2289, :4734, :9437 and the three enum sites in visit_stmt.rs) matches onlyNamespace/Enum*variants, so the three deletedPropertystores had no consumer and their removal only stops overwriting a merged namespace's map. - The
**left-operand check now goes throughunwrap_inlined();Expris by-value so no borrow issue, and the arms below it are unchanged. EXPECTED_VERSIONis bumped for the changed transpiler output; no CODEOWNERS entry covers the changed paths.
Extended reasoning...
The change deletes three inserts of ts::Data::Property into ref_to_ts_namespace_member in src/js_parser/p.rs and src/js_parser/parse/parse_typescript.rs, drops a now-unused clone helper, makes the ** parenthesization check in src/js_printer/lib.rs look through EInlinedEnum, bumps the runtime transpiler cache version to 44, and adds bundler and transpiler tests. It touches no security-sensitive surface. The source diff is small and I confirmed no reader of the map matches Property, but the fix intentionally diverges from esbuild and extends the pre-existing plain-namespace later-block gaps (silent wrong value where main threw) to function/class-merged namespaces, which the author documents as todos; that trade-off is a maintainer judgment, so defer rather than approve. The newest commit only addressed test nits; the substantive inline threads from the prior run remain open with no reply.
…now does The build for --aot asks the checker which enum reads the transpiler replaces by values (resolve_member_path). The checker left a bare name that another part of the namespace exports to run time, as the transpiler did. The transpiler follows that name now, so the checker does too. Without this an --aot executable and plain Bun printed different values in test/aot/parallel/enums/enums-namespaces-read-paths-brackets.ts. is_declared_around is gone. It kept three other names out as well: - The variable that a function of a block leaves behind in sloppy code. The first name must now be bound at run time to an enum or a namespace, and that keeps this one out. - An enum of a switch clause that a later clause reads, and an enum that is the body of an if or a loop. The transpiler puts their values in place, so the checker agrees with it there now. One name is followed that the transpiler does not follow yet: a namespace that shares a block with an exported function or class of its name, read by the bare name from another part. The checker already follows it by the full path and in its own block. #47001 changes the transpiler, and a program with a Todo line holds the difference until then. Tests: three fixtures of test/aot/checker-queries, two programs of test/aot/parallel/enums, and a line of the minifying measure for ts/SiblingNamespaceEnumShadowedArgument, where bun run reads M.Z under a parameter M (#41984). Two programs of test/aot/parallel/enums said in their first line that plain Bun differs. Each line says what holds now.
Problem
ReferenceError: x is not defined:namespace G { export enum Y { P = F.fn.E.A, R } },Risundefined.parse_type_script_namespace_stmt(src/js_parser/parse/parse_typescript.rs:298,:316) storedPropertyover the namespace's member map.Fix
Propertyintoref_to_ts_namespace_member. No reader of that map matchesProperty.**. The new fold can reach that SyntaxError.test/bundler/transpiler/transpiler.test.js(22 new tests fail on main) andtest/bundler/bundler_edgecase.test.ts(2). Self-reviewed: 11 concerns raised, 10 addressed.Background
ref_to_ts_namespace_membermaps it to the namespace members.ts_parser.go:1690,:1701,:1745) and output. tsc is the reference.Downsides
x(js_parser: rewrite assignments to exports of sibling TypeScript namespace blocks #38709). A bare read of a first-block enum givesundefined(js_parser: register the closure argument of a TypeScript namespace block #46938).Notes
Where it comes from. No user reported it. The shape was found in the review of #46938, which lists it as not covered. #12144 ported this loop with an assert that the record is a namespace. #12425 crashed on that assert with this source shape. #12610 removed the assert and kept the store.
Reach. 0 of 121,210 transpiler outputs change: 20,289 units of the TypeScript test cases in
test/cli/check/typescript-go/bundle.zstand the 3,953 tracked.ts,.tsx,.mtsand.ctsfiles, each under fiveBun.Transpileroption sets (default,minify.syntax, all ofminify,treeShakingwithtrimUnusedImports,inlinewith target bun).The printer line.
9into a SyntaxError at load, so they must not land without the line:enum E { A = -3 }followed byE.A ** n. Main prints-3 /* A */ ** nfor it, and so does esbuild 0.28.2, so there is no esbuild fix to copy.import.meta.mainnode and the left operand of**by level #33447 rewrites the arm.-3 /* N */ ** n. js_printer: parenthesize inlined negative enum value on LHS of ** #36133 owns that form.Landing order. No pull request has to land first. If #36133, #33447, #46938 or #46693 lands before this one, the printer commit is dropped on rebase. #46938, #46693, #46553 and #33447 also take
EXPECTED_VERSION44, so each one that lands later moves up by one.Not in this change. Each line is the behavior of a plain namespace on main.
An enum that bun visits before the namespace body still gets
undefined: a top-levelenum Y { P = F.fn.E.A, R }, or an enum in the block of the pair. tsc gives 8. js_parser: fold enum members nested in an earlier namespace into enum initializers #42118 owns the visit order.A bare
fn.E.Afrom another block ofFdoes not fold. js_parser: register the closure argument of a TypeScript namespace block #46938 registers the closure argument that this needs.A later block of the merged namespace (first block
export namespace fn { ... }afterexport function fn() {}, values fromBun.Transpilerandnew Function):fnx = 5; x++; export const y = x, with an outervar x = "outer"[F.fn.y, F.fn.x, x][6, 1, 6][1, 1, 6][6, 6, "outer"]export function g(fn) { return x }F.fn.g({ x: "param" })"param"1export enum Y { P = E.A, R }F.fn.Y.Rundefined8export function f(fn) { return E.A }F.fn.f({ E: { A: "param" } })"param"7Owners: js_parser: rewrite assignments to exports of sibling TypeScript namespace blocks #38709 (the write), js_parser: rename the TS namespace closure argument when a nested binding shadows it #41984 (the parameter with the name of the namespace), js_parser: register the closure argument of a TypeScript namespace block #46938 (the enum rows). Four
it.todorows hold the tsc values.In a bundle, the write of the first row goes to a variable
xof another module. On main the same program throws a ReferenceError at run time, or fails the build withCannot assign to import "x"when the import has that name. A plain namespace overwrites the other module's variable on main (js_parser: rewrite assignments to exports of sibling TypeScript namespace blocks #38709).import X = F.fnandimport E2 = F.fn.Eas the path to the member do not fold, with or without a merged function. No pull request covers that form.The three
exported_members.put(name, Property)calls stay. Sibling blocks resolve bare names through them.Output text that changes for code that works.
F.fn.E.Aand a same-blockfn.E.Aprint as7 /* A */. A program that replacesF.fnat run time reads the old value, as with a plain namespace.F["fn"]()prints asF.fn(). A negative member left of**prints as(-3) /* A */ ** n.Measurements. Release builds of main 449877f and this branch in one container. Instructions are user-mode counts from
perf_event_open(perf,valgrind,bloatyandstraceare not installed there). Call counts are gdb breakpoint counts on the map functions..text(size)parse_type_script_namespace_stmt(two instantiations)define_exported_namespace_bindingprint_expr(five instantiations)src/js/node/dns.ts(threedeclare namespacewithexport let): map insertssrc/js/node/dns.ts: instructions per transform (2,000 transforms, 5 interleaved rounds, spread under 1,100)src/js/node/http2.ts(no namespace), 200 transformsgeneratedContextualTyping.ts: +69,436 (+1.31%). Its map holds 24 entries where main holds 36, so the table grows 3 times, not 4, and its 773 lookups probe longer.ts.transpileModule): main differs from tsc in 12 programs, this branch in 0. Withminify.syntax: 9 and 0.Self-review. 11 concerns raised, 10 addressed.
enum.enumform is stated, because a maintainer asked about it on js_printer: parenthesize inlined negative enum value on LHS of ** #36133.undefinedfor a bare enum read of a later block is stated.exponentiation of an inlined enum member).it.todorows hold the tsc values for the later-block forms.import X = F.fnform. It is a separate defect and is named above.Tests.
transpiler.test.jshas 23 new tests and 4it.todorows. 22 of the 23 fail on main, and the control passes. Without the printer commit, 2 of them fail.bundler_edgecase.test.tshas 2 new tests, and both fail on main withReferenceError: Z is not defined.Also ran (debug build, all pass): the rest of
transpiler.test.jsandbundler_edgecase.test.ts,esbuild/{ts,dce,importstar_ts,tsconfig},bundler_{merged_enum,decorator_metadata,minify,regressions,naming},transpiler/{decorator-metadata,decorators,runtime-transpiler,ts-enum-redecl-panic,template-literal,scope-mismatch-panic},cli/run/transpiler-cache,regression/issue/{25648,28170,13251}.cargo clippy -p bun_js_parser -p bun_js_printeris clean.test/internal/source-lints: 180 pass. The 3 tests ofsema-parser-recursion.test.tstime out at 5 s on the debug build and pass on the release build.