js_parser: declare decorator lowering temporaries per iteration inside loops (stacked on #38734)

[robobun]

Stacked on #38734 (base branch farm/55355bb8/decorator-temp-names); the diff here is only the last commit.

Problem

• The standard decorator / accessor lowering declares a class's temporaries with var: _init, _dec, _computedKey, _base, one _<name> WeakMap or WeakSet per lowered member, and _class for a class expression (src/js_parser/lower/lower_decorators.rs, temp_decl and the hoisted declaration in Phase 8).
• Inside a loop body that is one binding for every iteration, but the class an iteration creates keeps using the temporaries after the next iteration has reassigned them: its constructor reads _init, _computedKey and the WeakMaps on every new, and the getters/setters __decorateElement installed hold the WeakMap of the iteration that created the class.
• So every class a loop creates ends up on the last iteration's data: instances run the wrong addInitializer callbacks and get the wrong computed key, and accessor fields (decorated or not) and #private members of lowered classes throw TypeError: Cannot read from private field. Same under bun run, bun build --no-bundle and bun build.

let n = 0;
function dec(value, ctx) { const id = ++n; ctx.addInitializer(function () { this.id = id; }); }
const classes = [];
for (let i = 0; i < 2; i++) { class K { @dec m() {} } classes.push(K); }
console.log(new classes[0]().id, new classes[1]().id); // "2 2", expected "1 2"

const objs = [];
for (let i = 0; i < 2; i++) { class K { accessor x = i; } objs.push(new K()); }
console.log(objs[0].x); // TypeError: Cannot read from private field

• tsc gives every evaluation of the class its own temporaries. esbuild 0.25.12 has the same bug.

Fix

• temp_decl_kind: when the class is lowered inside a loop body (fn_or_arrow_data_visit.is_inside_loop, set by visit_loop_body for all five loop statements and reset for nested functions, arrows and static blocks) the temporaries are declared with let instead of var. That covers statement mode (the declarations sit next to the class), expression mode (the declaration goes to the top of the innermost statement list, which is inside the loop body; a single-statement body is turned into a block) and the receiver-capture temporaries declared with them. var_decl/var_decl2 are renamed temp_decl/temp_decl2 because they no longer always emit var; the capture temporaries declare_capture_temps_in_fn_body puts at the top of method bodies stay var.
• Correct because a let in the loop body is a new binding on every iteration, which is what every evaluation of the class gets in the spec. Every use of a temporary is emitted after its declaration in the same list or sits in the class's own code, which runs later, so no use can reach the binding before it is initialized.
• Outside loops the statement list runs once per function invocation, so var and let would bind the same; var is kept there so that output stays unchanged and stays hoisted (a class expression in the parameter default of a hoisted function can run before the declaration statement, e.g. through an import cycle).
• Unlike var, an initialized let is a candidate for the single-use inlining in visit_stmts (--minify-syntax, which the runtime transpiler has on), and that pass trusts use_count_estimate. The lowering created some references to temporaries as bare E::Identifiers, which do not count: the _base argument of __decoratorStart, _init in the __decorateElement calls, the storage WeakMap handed to __privateAdd for decorated private fields and accessors, and the second reference to a _computedKey. These now go through use_ref, and temp_decl_kind documents the rule. The only shape where the miscount was adjacent to a consumer is @dec class K extends Base {} (no members) in a loop: let _base = Base is directly followed by let _init = __decoratorStart(_base), so the first version of this PR inlined _base, dropped its declaration and left class K extends _base throwing ReferenceError: _base is not defined. The other corrections are the same defect without a consumer next to it today.
• Why this needs js_parser: give standard decorator lowering temporaries unique names #38734: without a renamer (bun run, bun build --no-bundle) the temporaries print under the names they ask for, so with let two classes in one loop body (or accessor x next to #x) would go from silently sharing a var to a duplicate declaration SyntaxError. js_parser: give standard decorator lowering temporaries unique names #38734 gives them file-unique names. It also registers each temporary with the var-hoisting scope; that stays valid for a let in a block of that scope (a name unique across the scope is unique inside the block too, and the let _<Class> binding already worked this way).
• Not changed: a class expression in a loop head (while (f(class { ... })), for (;; x = class { ... })) is evaluated repeatedly without a statement list of its own per evaluation and would need a per-evaluation wrapper; parameter defaults and field initializers, which have the same problem, are js_parser: declare decorator lowering temporaries per evaluation in parameter defaults and field initializers #38904.
• Verified:
  • test/bundler/transpiler/es-decorators.test.ts, "lowering temporaries of a class created in a loop" (17 tests): class statements (initializers, accessor storage, private members, two classes in one body, one class with every kind of lowered member including a capture temporary, class decorator + extends clause only) and class expressions (instances created after and during the loop, the five loop statements with block and single-statement bodies, a nested block, class decorator + extends + computed key, switch cases, a static field of a class created in the loop). 16 fail on the base branch built without this diff; "class decorator and extends clause only" passes there and fails on the first version of this PR (the _base inlining), as does the MinifySyntax bundler test below.
  • test/bundler/bundler_edgecase.test.ts, edgecase/DecoratorLoweringTempsInLoops, ...MinifyIdentifiers, ...MinifySyntax, ...InWrappedModule: module-level and function-level loops through the bundler, with each minify flag, and inside an __esm wrapper. All four fail on the base branch.
  • es-decorators, es-decorators-esbuild, decorators, decorator-metadata, bundler_edgecase, bundler_decorator_metadata, bundler_minify, bundler_naming, transpiler.test.js, runtime-transpiler and esbuild/{ts,lower,default} pass; cargo clippy -p bun_js_parser is clean. bun build --no-bundle output for classes outside loops is byte-identical to the base branch.

Background

• Lowering shape: class K { @dec m() {} } becomes var _dec = [dec]; var _init = __decoratorStart(); class K { constructor() { __runInitializers(_init, 5, this); } m() {} } __decorateElement(_init, 1, "m", _dec, K);. _init holds the class's decoration state (the initializers decorators registered). An accessor x field keeps its value in a _x WeakMap: the constructor runs __privateAdd(this, _x, value), and the getter/setter read _x either through the binding (undecorated) or through the WeakMap object handed to __decorateElement (decorated). A class expression is lowered to a comma chain assigning the same temporaries, which are declared once, without initializers, at the top of the enclosing statement list.
• is_inside_loop is the visitor flag the parser already uses to validate break/continue: true while a loop body of the current function is being visited, false again inside nested function, arrow and static block bodies.
• Single-use inlining: after visiting a block, visit_stmts (minify-syntax mode, non-module scopes) turns let x = init; use(x) into use(init) when x's use_count_estimate is 1 and the use is the next statement's first evaluated expression. var declarations are exempt because their uses may not all have been visited yet. use_count_estimate is maintained by record_usage; use_ref in the lowering is record_usage plus the identifier.
• Renaming: bun build runs a renamer, so two symbols that ask for the same name print differently; the runtime transpiler and bun build --no-bundle print the requested names as they are. js_parser: give standard decorator lowering temporaries unique names #38734 makes the requested names unique for that path.

Output for the first repro, before and after

Before:

for (let i = 0; i < 2; i++) {
  var _dec = [dec];
  var _init = __decoratorStart(undefined);
  class K { constructor() { __runInitializers(_init, 5, this); } m() {} }
  __decorateElement(_init, 1, "m", _dec, K);
  __decoratorMetadata(_init, K);
  let _K = K;
  classes.push(K);
}

After: the two vars are lets. A class expression in a loop body goes from var _class, _init, _dec, _x; at the top of the body to let _class, _init, _dec, _x;. Classes outside loops print as before.

First version of this PR

The first push only switched the keyword. Review found that this made @dec class K extends Base {} inside a loop throw ReferenceError: _base is not defined under bun run and bun build --minify-syntax, because _base had a recorded use count of 1 (only the extends reference was recorded) and the single-use inliner substituted it into __decoratorStart. The current version records every reference (the use_ref hunks) and adds the two tests that fail on the first version.

[robobun]

Reproduced on Bun 1.4.0 and on the base branch: a decorated class (or a class with an accessor field) created inside a loop body shares its var temporaries across iterations, so instances of an earlier iteration's class run the last iteration's initializers, get its computed key, or throw TypeError: Cannot read from private field (statement and expression mode; bun run, --no-bundle and bun build).

Fix in this PR: the temporaries are declared with let when the class is lowered inside a loop body, and every reference to a temporary is now recorded so the single-use inliner (minify-syntax) leaves the lets alone. Of the 21 new tests, 20 fail on the base branch; the remaining one and the MinifySyntax bundler test fail on the first version of this PR, which did not record the references. Stacked on #38734, which gives the temporaries unique names; with let, colliding names would be a SyntaxError instead of silent sharing.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes transpiler output for decorator/accessor lowering and is stacked on #38734, a human look would still be worthwhile before landing.

What was reviewed:

• temp_decl_kind() reads fn_or_arrow_data_visit.is_inside_loop, which visit_loop_body sets for all five loop kinds and which resets to false in nested functions, arrows, and static blocks — matches the PR's claim.
• All var_decl/var_decl2 call sites are renamed; declare_capture_temps_in_fn_body correctly stays KVar (per-invocation, not per-iteration).
• Checked the switch-in-loop test: two let decls in sibling case clauses share the switch block scope, but #38734's unique names prevent redeclaration and only one case runs per iteration, so no TDZ read.
• Expression-mode hoisted decl at nearest_stmt_list_mut() uses temp_decl_kind() evaluated at the class expression's visit position, which aligns with the innermost statement list being inside the loop body.

Extended reasoning...

Overview

This PR fixes a correctness bug in the standard-decorator / accessor lowering: temporaries (_init, _dec, _computedKey, _base, per-member WeakMaps/WeakSets, _class) were declared with var, so inside a loop body all iterations shared one binding. Classes created in earlier iterations would read the last iteration's temporaries when constructed later. The fix adds a 7-line temp_decl_kind() helper that returns KLet when is_inside_loop is set, and threads it through the ~15 declaration sites (mostly a mechanical rename of var_decl→temp_decl). 18 new tests across es-decorators.test.ts and bundler_edgecase.test.ts.

Security risks

None. This is a JS-to-JS lowering change with no I/O, no untrusted input parsing beyond what the parser already does, and no allocation-size arithmetic.

Level of scrutiny

Moderate-to-high. The Rust diff is small and mechanical, and the flag it reads (fn_or_arrow_data_visit.is_inside_loop) is an existing, well-understood parser state used for break/continue validation. But this changes transpiler output for every decorated class inside a loop, across bun run, --no-bundle, and bun build. I verified the flag's lifecycle (set in visit_loop_body at src/js_parser/visit/mod.rs:708-715, reset to default when entering functions/arrows/static blocks at mod.rs:84, mod.rs:855, visit_expr.rs:2428), that all old var_decl/var_decl2 call sites were updated, and that the method-body capture path (declare_capture_temps_in_fn_body) intentionally stays KVar.

Other factors

• Stacked PR: this depends on #38734 for unique temp names. Without it, let would turn silent var sharing into a duplicate-declaration SyntaxError. A human should confirm the stacking/merge order.
• Test coverage is excellent: all five loop statement types, block vs single-statement bodies, class statements vs expressions, every lowered-member kind (including receiver-capture temps), nested blocks, switch-in-loop, minified identifiers, and __esm-wrapped modules. The PR states all 18 fail on the base branch and pass with the fix.
• Acknowledged gaps: class expressions in loop heads, parameter defaults, and instance field initializers are explicitly left for a follow-up (they have no per-evaluation statement list).
• The expression-mode path pushes the let to nearest_stmt_list_mut(); temp_decl_kind() is evaluated at the class expression's visit position, so is_inside_loop is true exactly when that statement list is inside the loop body — the two align by construction.

[robobun]

On the stacking question: #38734 has to land first. Once it does, GitHub retargets this PR to main and the diff stays this single commit; if #38734 changes, I will rebase this on top of it. Landing this one on its own is not an option: without the unique names, two lowered classes in one loop body (or accessor x next to #x) would print two let declarations of the same name in one block, which is a SyntaxError, where the vars today merely share a binding.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes decorator-lowering codegen in the transpiler (var→let scoping with knock-on effects on single-use inlining and __esm hoisting) and is stacked on #38734, a human look would still be worthwhile before landing.

What was reviewed:

• temp_decl_kind gates on fn_or_arrow_data_visit.is_inside_loop, which is set by visit_loop_body and reset for nested fns/arrows/static blocks — matches the described scope.
• The bare E::Identifier → use_ref conversions (base_ref, init_ref, computed_key_refs, private_storage_ref/private_extra_ref) keep use_count_estimate ≥ 2 so minify-syntax single-use inlining can't drop a let temp still read elsewhere; the _base/extends and computed-key cases are covered by the new tests.
• let in a module-level loop is not touched by maybe_relocate_vars_to_top_level (only KVar is relocated), so the wrapped-__esm case keeps per-iteration bindings; covered by the InWrappedModule test.
• Switch-case temporaries share the switch block scope but get unique names from #38734, so two lets per iteration don't collide.

Extended reasoning...

Overview

Single-commit change stacked on #38734. In src/js_parser/lower/lower_decorators.rs, standard decorator/accessor lowering temporaries (_init, _dec, _base, _computedKey, per-member WeakMap/WeakSet refs, the class-expression temp, and receiver-capture temps declared alongside them) are now emitted as let when the class is lowered inside a loop body, and stay var otherwise. var_decl/var_decl2 are renamed to temp_decl/temp_decl2; drain_capture_temp_decls gains a kind parameter (still KVar for the per-method-body path, temp_decl_kind() for the class-level path). Several bare E::Identifier { ref_ } sites become p.use_ref(ref_, ..) so use_count_estimate reflects every reference. 15 new runtime-transpiler tests in es-decorators.test.ts and 4 new itBundled tests in bundler_edgecase.test.ts.

Security risks

None. This is JS/TS-to-JS lowering; no auth, crypto, filesystem, or network surface is touched. The change picks a declaration keyword and adjusts symbol use-count bookkeeping.

Level of scrutiny

High. Transpiler codegen is user-visible for every file that uses standard decorators or accessor, and var→let changes hoisting semantics, TDZ, and interacts with the bundler's minify-syntax single-use-let inlining and its __esm var-relocation pass. The PR is also stacked: correctness of the let output for two classes in one loop body (or accessor x next to #x) depends on #38734's unique naming, so land order matters.

Other factors

The reasoning is careful and the test matrix is broad: all five loop forms, block and single-statement bodies, statement and expression classes, nested blocks, switch cases, static-field-nested class expressions, minify-identifiers, minify-syntax, and the __esm wrapped-module path. I traced the specific hazards the diff introduces — the single-use-inlining risk that motivates the use_ref conversions is real (a let with count 1 followed by an immediately-consumed reference is inlined by visit_stmts), and each converted site does have a second use the count now reflects. The one open edge the author calls out (class expressions in loop heads / param defaults / instance field initializers) is explicitly left for a follow-up and is not made worse here. Given the subtlety and the stacking dependency, I'd like a human to confirm the use_ref additions don't over-count in any path (they look correct to me) and to sequence the merge behind #38734.

[robobun]

^{Updated 12:15 PM PT - Aug 15th, 2026}

✅ @robobun, your commit da891b29460f53c3d3773b3aae0cafafa78b5c43 passed in Build #98096! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38933

That installs a local version of the PR into your bun-38933 executable, so you can run:

bun-38933 --bun

[robobun]

On the over-counting question, counted per use_ref site against what the printer emits (checked with bun build --no-bundle on the debug build):

• _base: printed in extends _base and in __decoratorStart(_base); recorded twice. Exact.
• _init in the member and class __decorateElement calls: each call expression ends up in exactly one of the suffix lists, so one recording per printed call. The remaining _init references (__runInitializers, __decoratorMetadata) already went through use_ref. Exact.
• The storage WeakMap stored on the init entry (prop_shallow.key for private members, .value for public decorated accessors): printed once, in that entry's __privateAdd; recorded once. Exact.
• _computedKey: decorated methods, getters, setters and fields print it twice (the member's own key or the this[_computedKey] assignment, plus the decorate call) and record twice. A decorated accessor [k] prints it once, because the member itself is replaced by what __decorateElement installs, while two references are recorded. That is the one over-count: it can only keep the inliner away from a declaration, and none of the generated statements consumes _computedKey as its first expression anyway, so it changes no output. Recording per emitting site is deliberately kept simple rather than special-casing accessors, since under-counting is the direction that breaks.

Over-counting in general is harmless here: the inliner and the unused-name removal only act on counts of 1 and 0, and var temporaries were never inlined at all, so nothing that was inlined before stops being inlined. I amended the commit (da891b2) to make the code comment at the computed-key site say this; no code or test changes.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes parser lowering semantics (var→let with inliner interactions) and is stacked on #38734, a human look would still be worthwhile before landing.

What was reviewed:

• Verified is_inside_loop is live at both lowering entry points (lower_class in visit_stmt, lower_standard_decorators_expr in visit_expr) and resets correctly for functions/arrows/static blocks.
• Checked each new use_ref site corresponds to an emitted reference; the one over-count (_computedKey on decorated accessors) is documented and harmless.
• Checked the switch-case test for TDZ hazards — each iteration gets a fresh switch block scope and #38734's unique names keep the two cases' lets distinct.

Extended reasoning...

Overview

The PR changes src/js_parser/lower/lower_decorators.rs so that the temporaries the standard-decorator/accessor lowering declares (_init, _dec, _base, _computedKey, per-member WeakMaps/WeakSets, receiver-capture temps, and the class-expression hoisted decl) use let instead of var when the class is lowered inside a loop body, so each iteration gets its own bindings. It also converts several bare E::Identifier constructions to use_ref so use_count_estimate is accurate for the single-use inliner, and renames var_decl/var_decl2 → temp_decl/temp_decl2. 21 new tests span the runtime transpiler (es-decorators.test.ts) and the bundler with minify variants (bundler_edgecase.test.ts).

Security risks

None. This is a code-generation correctness fix in the JS parser; no auth, crypto, filesystem, or network surface is touched.

Level of scrutiny

Medium-high. Decorator lowering is correctness-critical (wrong output silently miscompiles user code), and the fix interacts with two other subsystems: the visit_stmts single-use inliner (via use_count_estimate) and the temp-naming pass from #38734. The author already found and fixed one such interaction (the _base inlining regression) after the first push, which is exactly the kind of subtlety that argues for a human pass.

Other factors

• Stacked PR: depends on #38734 for unique temp names; landing this alone would turn silent var sharing into let duplicate-declaration SyntaxErrors. The base PR must land first.
• I confirmed fn_or_arrow_data_visit.is_inside_loop is set by visit_loop_body (all five loop forms) and reset via FnOrArrowDataVisit::default() on function/arrow/static-block entry, and that both lower_standard_decorators_stmt (via lower_class in visit_stmt.rs) and lower_standard_decorators_expr (in visit_expr.rs) run inside the visit pass where the flag is valid.
• declare_capture_temps_in_fn_body correctly stays var (method-body scope, fresh per invocation); the class-level drain_capture_temp_decls correctly follows temp_decl_kind.
• The use_ref audit in the author's follow-up comment matches what I traced: exact for _base, _init, and storage refs; one benign over-count for _computedKey on decorated accessors, which can only prevent inlining (never cause it) and is documented at the site.
• Test coverage is thorough: statement and expression mode, all five loop forms, block vs single-statement bodies, nested blocks, switch cases, static field initializers, the minify-syntax inlining hazard, and the __esm-wrapped module path. The author states 20/21 fail on the base branch and the remaining one guards the first-version regression.
• Known unfixed cases (loop-head class expressions, parameter defaults, field initializers) are explicitly called out and deferred to #38904.

[robobun]

Confirming this review is against the amended head (da891b2), which only reworded the code comment at the computed-key site; the code and tests are the ones reviewed in the previous round. Nothing further is planned for this branch unless CI turns something up, so the remaining step is landing #38734 and then this.

[robobun]

Status against #40833, which rewrites the standard decorator lowering: it does not cover this case. A class declared in a loop body still gets var temporaries, so every iteration's class shares one _init and one _x WeakMap. All 16 tests and the 4 bundler cases added here fail on that branch with TypeError: Cannot read from private field. esbuild 0.25 produces the same shape and fails the same way.

The unique-name part this PR was stacked on (#38734) is covered by #40833, including the three cases it missed (ported in 1c58acd), so #38734 is closed. This PR stays open for the per-iteration declarations. It needs a rebase onto #40833, where new_sym now registers every temporary with the var-hoisting scope.

[robobun]

#40895 merged as 5822b76 and carries the unique temporary names that #38734 had, so the base branch of this PR (farm/55355bb8/decorator-temp-names, from the closed #38734) is superseded. To move forward, retarget this PR to main and rebase the per-iteration let change onto P::generate_temp_var.