[robobun]

Problem

• A class expression that needs the standard decorator lowering (decorators or an accessor member) fails when it is a parameter default value or an instance field initializer and the function is called, or the outer class constructed, more than once: constructing an instance of an earlier class throws TypeError: Cannot read from private field, and with a method decorator that uses addInitializer the instance silently receives a later class's initializers.

  function dec(_value, _ctx) {}
  function make(K = class { @dec accessor x = 0 }) { return K; }
  const K1 = make(), K2 = make();
  new K1().x; // TypeError: Cannot read from private field
  
  class Outer { inner = class { @dec accessor x = 0 }; }
  const a = new Outer(), b = new Outer();
  new a.inner().x; // same

• The expression-mode lowering (src/js_parser/lower/lower_decorators.rs, phase 8) emits (_dec = [dec], _x = new WeakMap, _init = __decoratorStart(), _class = class {...}, __decorateElement(..., _x), _class) and pushes the var _class, _init, _dec, _x for those temporaries into p.nearest_stmt_list, the statement list around the enclosing statement (the function declaration, the outer class). That list runs once, the expression runs once per call / per construction, so every evaluation assigns the same variables. The class body reads them later, at construction time (__privateAdd(this, _x, ...), __runInitializers(_init, ...)), so an instance of the first class uses the second evaluation's WeakMap and initializer array, while its accessor was bound to the first one by __decorateElement.
• visit_args visits default values before the function body installs its own list (src/js_parser/visit/mod.rs), and visit_class visits field initializers with the list of the surrounding statements installed; nothing in between runs once per evaluation, so there is no list the declaration could go into.
• When nearest_stmt_list is None the declaration was silently dropped (ReferenceError: _dec is not defined); top-level enum member initializers are visited that way (js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761 covers that case by giving the enum body a list; see below).
• esbuild 0.25 emits the same shared module-level var for these inputs and fails the same way; tsc wraps every such class expression in an IIFE that declares the temporaries locally.

Fix

• New P::nearest_stmt_list_is_shared: visit_args sets it while visiting the parameter list (plain and destructuring defaults) and visit_class while visiting a non-static field or accessor initializer; visit_stmts saves it, clears it while its own list is installed, and restores it, so a function body, arrow body or static block nested inside one of those expressions hoists exactly as before. Everything else inherits the value, which is what makes nested cases right: a class inside the static initializer, computed key or extends clause of a class expression that is itself a parameter default is still created once per call, and is wrapped; the same positions in a class statement still hoist.
• Phase 8 hoists the var only when a list is installed and the flag is clear. Otherwise it emits (() => { var _class, _init, _dec, _x; return _dec = [dec], ..., _class; })() in place of the comma chain, so each evaluation of the expression gets fresh bindings. This is also the fallback for nearest_stmt_list == None.
• Why the arrow is correct there: an arrow function keeps the this, super, arguments and new.target of the code it appears in, so the class's decorator expressions, computed keys and extends clause (the only user code evaluated in the chain; everything else in it is generated) behave as they did in the source; the new test checks this, super and arguments explicitly. A parameter default value and a class field initializer are the two expression positions where await and yield are early errors, so the chain can never contain one, which is what would make a plain arrow wrapper invalid in ordinary statement context. Evaluation order is unchanged because the wrapper is called in the exact position the chain occupied. Output for every other position is byte-for-byte unchanged (the transpiler test pins a statement, a static initializer, a computed key and a body nested in an initializer).
• Both renamer paths are fine with the wrapper: the temporaries are still registered in the scope current at lowering time, so bun build / --minify give them names distinct from everything visible there, and the declaration now sits in a scope nested inside it. Sibling functions that each have such a default used to share one module-level declaration even in bundled output; they are independent now (the Bun.build test covers plain and minified output).
• Verified with test/bundler/transpiler/es-decorators.test.ts, describe "class expressions evaluated once per call or per construction" (12 tests: function, arrow, constructor and method defaults, a destructuring default, instance field initializer in JS and TS, instance accessor initializer of a class that is lowered itself, a static initializer nested in a parameter default, the top-level enum fallback, the this/super/arguments guard, transpiler output shape, Bun.build plain and minified). 11 fail on the unfixed build with the error above; the this/super/arguments test passes on both and guards the wrapper's semantics.
• Also pass with the fix: es-decorators-esbuild (the ported esbuild suite), decorators, decorator-metadata, bundler_decorator_metadata, ts-use-define-for-class-fields, ts-enum-redecl-panic, transpiler.test.js, react-compiler and react-compiler-fixtures (react refresh is the other nearest_stmt_list user; it is not affected by the flag), bundler_edgecase, bundler_minify, bundler_naming, esbuild/ts, esbuild/default, esbuild/lower; cargo clippy -p bun_js_parser clean.
• Relation to open work in this file: js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761 makes enum bodies install their own list, which is leaner than the fallback here for enums; both can land in either order. js_parser: give standard decorator lowering temporaries unique names #38734 changes how the temporaries are named and does not change where they are declared; its duplicate _dec entry in the var (visible in the output below) is unrelated to this change. Two related bugs found while working on this are tracked separately: temporaries declared with var are also shared across the iterations of a loop (statement and expression mode), and a decorated class used as a plain parameter default gets .name === "_class".

Background

• Standard decorator lowering: Bun has no runtime support for TC39 decorators or accessor, so the parser rewrites such a class into a plain class plus calls to runtime helpers. For a class expression the result has to stay an expression, so the temporaries it needs are assigned inside a comma expression and declared by a separate var statement emitted somewhere else. The constructor of the rewritten class refers to those temporaries by name, which is why it matters where and how often they are declared: _init holds the per-class array of initializers that addInitializer callbacks and field initializers go into, and each accessor gets a WeakMap (_x) that stores the per-instance value.
• nearest_stmt_list is a pointer to the list that visit_stmts emits ahead of the statements it is currently visiting; it is saved and restored around every nested statement list (function bodies, blocks, static blocks). React fast refresh uses it to insert _s = $RefreshSig$() declarations, and the decorator lowering uses it for the var above.
• Visiting order for the two positions: visit_func / the arrow visitor push the argument scope and visit the parameters before pushing the body scope and visiting the body statements, so during a default value the list installed is still the one around the function. visit_class visits member initializers directly, inside the class body scope, with the list around the class statement installed; whether a field is static is a flag on the property.

Output for the first repro, before and after (bun build --no-bundle, helper import elided)

Before:

var _class, _init, _dec, _dec, _x;
function dec(_value, _ctx) {}
function make(K = (_dec = [dec], _x = new WeakMap, _init = __decoratorStart(undefined), _class = class {
  constructor() {
    __runInitializers(_init, 5, this);
    __privateAdd(this, _x, __runInitializers(_init, 8, this, 0));
    __runInitializers(_init, 11, this);
  }
}, __decorateElement(_init, 4, "x", _dec, _class, _x), __decoratorMetadata(_init, _class), _class)) {
  return K;
}

After:

function dec(_value, _ctx) {}
function make(K = (() => {
  var _class, _init, _dec, _dec, _x;
  return _dec = [dec], _x = new WeakMap, _init = __decoratorStart(undefined), _class = class {
    constructor() {
      __runInitializers(_init, 5, this);
      __privateAdd(this, _x, __runInitializers(_init, 8, this, 0));
      __runInitializers(_init, 11, this);
    }
  }, __decorateElement(_init, 4, "x", _dec, _class, _x), __decoratorMetadata(_init, _class), _class;
})()) {
  return K;
}

For comparison, tsc 6.0 emits K = (() => { let _x_decorators; let _x_initializers = []; ...; return class { ... }; })() for the same input, and esbuild 0.25.12 emits var _x_dec, _init, _x; at module level and fails at runtime like Bun did.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2baae0f3-b3d3-4792-8061-3d5bb8de5c61

📥 Commits

Reviewing files that changed from the base of the PR and between 7d276b9 and 64d1e40.

📒 Files selected for processing (4)

• src/js_parser/lower/lower_decorators.rs
• src/js_parser/p.rs
• src/js_parser/visit/mod.rs
• test/bundler/transpiler/es-decorators.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: reproduced and fixed; waiting on CI and review.

• Reproduced on the released build (1.4.0-canary) and on a debug build of main with both repros from the report (class expression as a parameter default, and as an instance field initializer): TypeError: Cannot read from private field once the function has been called, or the outer class constructed, twice. Arrow and method defaults, a destructuring default, and a class nested in a static initializer of a parameter-default class fail the same way.
• Fix in this PR: the temporaries are declared inside an arrow wrapped around the lowered expression in those positions (hoisting is unchanged everywhere else). Tests: test/bundler/transpiler/es-decorators.test.ts, describe "class expressions evaluated once per call or per construction"; 11 of the 12 new tests fail on the unfixed build.

[github-actions]

This PR may be a duplicate of:

1. js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761 - Fixes the same defect shape (decorator lowering temporaries hoisted into a statement list that is not evaluated per-evaluation, plus the silently-dropped var when nearest_stmt_list is None) for the enum-initializer producer, and takes the opposite design decision on the None case — it makes enum bodies install their own statement list and adds a debug_assert!(nearest_stmt_list.is_some()), which contradicts this PR's None → IIFE fallback and its top-level enum test.

🤖 Generated with Claude Code

[claude]

I reviewed this PR and didn't find any bugs. Because it changes visitor state threading in the JS parser and the shape of decorator-lowered output, a human look would still be worthwhile.

What was reviewed:

• The save/set/restore of nearest_stmt_list_is_shared in visit_args, visit_class (non-static initializers only), and visit_stmts — no early-return paths skip the restore.
• The IIFE fallback in phase 8: the arrow preserves this/super/arguments/new.target, and await/yield are early errors in both target positions, so the wrapper is transparent; the None-list case (top-level enum initializers) now falls back instead of silently dropping the var.
• The other nearest_stmt_list consumer (React refresh in visit_expr.rs) does not read the new flag and is unaffected.
• Test coverage spans function/arrow/method/constructor/destructuring defaults, JS+TS instance field initializers, nested static-in-default, the enum fallback, output-shape pinning for the unchanged positions, and Bun.build plain+minified.

Extended reasoning...

Overview

This PR fixes a correctness bug in the standard-decorator lowering for class expressions. When a lowered class expression appears as a parameter default value or an instance field initializer, the temporaries (_class, _init, _dec, accessor WeakMaps) were declared once via var in the enclosing statement list but assigned once per evaluation, so multiple evaluations shared state and later instances of an earlier class read the wrong WeakMap/initializer array. The fix introduces P::nearest_stmt_list_is_shared, set during visit_args and non-static visit_class initializer visits and cleared inside every nested visit_stmts; when set (or when no list exists), phase 8 of lower_decorators.rs wraps the comma chain in an arrow IIFE that declares the temporaries locally instead of hoisting them. An iife() helper is extracted and the one existing inline IIFE construction is switched to it. runDecoratorTS is hoisted to file scope in the test file, and 12 new tests are added.

Security risks

None. This is a code-transformation correctness fix in the transpiler; no untrusted input handling, auth, crypto, or filesystem/network surface is touched.

Level of scrutiny

Moderate-to-high. The JS parser visitor and decorator lowering are core transpiler paths that every user's decorated code flows through. The change is small and surgical (one bool flag, three save/restore sites, one read site, one helper extraction), but correctness depends on subtle invariants: that the flag is cleared for every nested statement list (so hoisting still happens inside a function body nested in an initializer), that the arrow wrapper is semantically transparent everywhere it can be emitted, and that the renamer still assigns non-colliding names when the declaration moves into a nested scope. The PR description argues each of these carefully and the tests pin them, but a maintainer familiar with the parser's scope/renamer machinery should confirm.

Other factors

• No bugs were found by the bug hunting system.
• The save/restore pattern matches the existing nearest_stmt_list handling in visit_stmts, and I traced each set site to verify no early return or continue between the set and the restore.
• The test suite is thorough: it covers the full variant matrix (function/arrow/constructor/method/destructuring defaults, JS and TS field initializers, an accessor initializer in a class that is itself lowered, a static initializer nested inside a parameter default), the None-list enum fallback, a this/super/arguments semantics guard for the wrapper, an output-shape test pinning that unchanged positions still hoist, and a Bun.build plain+minified round-trip.
• The PR notes two adjacent bugs (loop-body var sharing and .name === "_class") are tracked separately, and coordinates with #38761 and #38734.
• No CODEOWNERS cover the changed files. No prior human review comments to address.

[robobun]

Not a duplicate of #38761; the two fix different producers of the same failure and compose.

• js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761 is about enum member initializers: it makes s_enum install a statement list for the enum body, so the var is hoisted into the enum closure. It does not touch visit_args, visit_class or the expression-mode emission, and it cannot fix the cases here: a parameter default or an instance field initializer always has a list installed (the one around the function or the outer class), the problem is that this list runs once while the expression runs once per call or per construction. There is no list that runs per evaluation to install, which is why this PR declares the temporaries inside the expression instead.
• On the None case: with both merged, js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761's debug_assert! holds because the enum body now installs a list, so the lowering takes the hoisting branch there and the None arm here is simply never reached; the enum test in this PR checks behavior, not output shape, and passes either way. Whichever PR lands second gets rebased: if js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761 goes first, the None arm here becomes if nearest_stmt_list_is_shared { wrap } else { hoist }, and if this one goes first, js_parser: declare temporaries hoisted out of enum initializers inside the enum closure #38761's assert is still valid as written.

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[github-actions]

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

[claude]

I reviewed this PR and didn't find any bugs. Because it adds new parser-wide state (nearest_stmt_list_is_shared) with save/restore across several visitor entry points, and the arrow-IIFE wrapper's correctness rests on a language-semantics argument (this/super/arguments/new.target preservation; await/yield being early errors in both target positions), a human look would still be worthwhile.

What was reviewed:

• Flag lifecycle: set in visit_args and non-static property.initializer, cleared in visit_stmts for nested bodies, restored on every path — no leak into sibling visits.
• Phase-8 emission: hoist only when list present and flag clear; otherwise var + return result wrapped in an arrow IIFE via the new iife helper (which also replaces the existing open-coded IIFE).
• Checked that the static-initializer-inside-a-parameter-default case inherits the flag correctly (test covers it), and that visit_class does not set the flag for property.value (methods) or static initializers.

Extended reasoning...

Overview

The PR fixes shared-temporary bugs in the standard-decorator lowering when a decorated class expression appears as a parameter default or an instance field initializer. It adds a boolean P::nearest_stmt_list_is_shared, sets it in visit_args (all defaults, including destructuring) and in visit_class for non-static property.initializer, clears/restores it in visit_stmts, and in phase 8 of lower_decorators.rs wraps the comma chain in (() => { var ...; return chain; })() instead of hoisting the var when the flag is set or no statement list is installed. A small iife helper is extracted and reused at the existing IIFE site. Tests add a 12-case describe block plus a transpiler-output-shape test and a Bun.build plain/minified round-trip; runDecoratorTS is hoisted to file scope.

Security risks

None identified. This is transpiler output-shape work; no untrusted-input parsing, allocation sizing, or FFI is touched.

Level of scrutiny

High. This is core parser/transpiler code that runs on every file containing decorators or accessor. The fix introduces new state on P that must be saved and restored correctly across nested visits, and the arrow-wrapper correctness depends on a subtle spec argument (arrows preserve this/super/arguments/new.target; parameter defaults and class field initializers are the two expression positions where await/yield are early errors, so the wrapped chain can never contain one). The PR description makes that argument carefully and a test pins this/super/arguments, but a maintainer should confirm the reasoning and the interaction with #38761 (which adds a debug_assert!(nearest_stmt_list.is_some()) on the same code path).

Other factors

• The comment-cop bot flagged four multi-line comments; the author hasn't responded yet. Those are style nags rather than correctness issues, but they're outstanding.
• Test coverage is thorough (function/arrow/constructor/method/destructuring defaults, JS+TS field initializers, nested lowering, static-in-default inheritance, top-level enum fallback, output-shape pinning for the unchanged positions, Bun.build with and without minify).
• The iife helper refactor is a straightforward de-duplication of the existing open-coded arrow+call at line ~2146; the is_async: false explicit field is dropped in favor of Default::default(), which is equivalent.

[robobun]

Status against #40833, which rewrites the standard decorator lowering: it does not cover this case. A decorated class expression in a parameter default or a field initializer still hoists its temporaries (var _class, _init, _dec, _x) to the enclosing function or module, so the second evaluation reuses the first one's WeakMap and _init. All 11 tests 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.

This PR stays open. It needs a rebase onto #40833, whose class-expression output is assembled in lower_impl (the is_expr branch pushes the var declaration to nearest_stmt_list_mut).