js_parser: keep top-level classes module-scoped when lowering top-level using

[robobun]

Stacked on #38284 (this PR's base is its branch); see the sequencing bullet under Fix. Only the last commit is this PR.

Problem

• In a module with a top-level using / await using, built for any target that lowers using (browser, node; --target=bun is unaffected), top-level class declarations end up in the wrong scope. Affects bun build, bun build --no-bundle, Bun.Transpiler, the bake client bundle and the REPL. Reproduces on 1.4.0 and main; found by inspection, no user report for this exact shape.
• An exported class is hoisted above the generated try/catch, so it runs before the using values it comes after are initialized: static v = foo.v throws TypeError: Cannot read properties of undefined (reading 'v'), and a field like static v = foo?.v is silently undefined. export class Sub extends Base {} with a non-exported Base throws ReferenceError: Base is not defined, because only Sub is hoisted.
• A non-exported class stays inside the try block as a block-scoped declaration, while function declarations are hoisted out of it, so an exported function that uses the class throws ReferenceError: Bar is not defined when called. Same for the export getters of --format=cjs / iife output.
• Cause: the SClass arm of LowerUsingDeclarationsContext::finalize (src/js_parser/p.rs), added in hoisting of exports when there is top level using #14313 for export and using keywords cannot be used in the same file #13734, moved export class out of the try block because an export is not allowed inside a block, and let the others fall into it unchanged. The comment above will_wrap_module_in_try_catch_for_using in src/js_parser/parse/parse_entry.rs describes the class being turned into a var, but nothing implemented that.

Fix

• s_class (src/js_parser/visit/visit_stmt.rs): when the module is going to be wrapped for using and the class is at module scope, the class statement returned by lower_class is replaced with var Foo = class Foo {}, keeping its export flag (convert_class_stmt_to_var). It then takes the path export const / export let already take in this mode: finalize moves the binding into the export clause it emits after the try/catch, the export scan (which runs after visiting) picks it up from there, and when bundling js_parser: hoist the declarations of a lowered top-level using out of the ESM wrapper and the REPL IIFE #38284 relocates the var like every other one in the block.
• The SClass arm in finalize is removed. Module-level classes now arrive there as locals, and a class statement in a nested block is never exported, so the arm had nothing left to do. The export and using keywords cannot be used in the same file #13734 shape it was added for (export class declared before the using that instantiates it) is still covered by the existing edgecase/UsingExportClass, now through the export clause.
• Why this is correct: the var is initialized at the same point in evaluation order as the declaration was; it is module-scoped, so the hoisted function declarations and the importing modules can reach it; and later assignments to the binding (decorator lowering emits Foo = __decorateElement(..., Foo) after the class) still reach the export through the clause. The class expression keeps its name, so .name and references to Foo from inside the class body behave as before. esbuild emits the same thing for this input (var Bar = class {...} and var Baz = class {...} inside the try, export { Baz } after it).
• Decorated classes go through the same replacement: the extra statements lower_class emits around a class (both decorator flavors) refer to the class by symbol, so they keep working against the var. Also checked by hand: TS class/namespace merging, --minify, --format=cjs / iife, and the REPL.
• Sequencing: when bundling, a module that is also reached through import() is evaluated inside an __esm(...) wrapper, and the linker only hoists top-level statements out of it. The vars inside the try block are hoisted by js_parser: hoist the declarations of a lowered top-level using out of the ESM wrapper and the REPL IIFE #38284, so this PR is based on it and has to land after it. On main alone, this change would turn an exported class that does not touch the using values in such a module from working (it was hoisted above the try) into a ReferenceError at the export getter, the state export const is in today. edgecase/UsingTopLevelClassDeclarationsInEsmWrapper pins this down: it passes on this stack, fails on js_parser: hoist the declarations of a lowered top-level using out of the ESM wrapper and the REPL IIFE #38284's branch without this change (TypeError, class evaluated too early) and fails on main plus this change alone (var Exported left inside the wrapper), so CI enforces the order.
• Tests: test/bundler/bundler_edgecase.test.ts (edgecase/UsingTopLevelClassDeclarations, UsingTopLevelClassDeclarationsInEsmWrapper, AwaitUsingTopLevelClassDeclarations, UsingTopLevelClassStandardDecorators, UsingTopLevelClassExperimentalDecorators, UsingTopLevelClassBakeDev) and test/bundler/transpiler/transpiler.test.js (using top level turns class declarations into vars, a snapshot of the non-bundled Bun.Transpiler output, which js_parser: hoist the declarations of a lowered top-level using out of the ESM wrapper and the REPL IIFE #38284 does not affect). All seven fail on the base branch without the src/ change and pass with it. The bundler tests only assert Foo = class for the rewritten classes, since the var is relocated when bundling and the expression's name is not what they are about.
• Also passing on the stack: the rest of bundler_edgecase (including js_parser: hoist the declarations of a lowered top-level using out of the ESM wrapper and the REPL IIFE #38284's tests and UsingExportClass), bundler_browser, bundler_minify, esbuild/{lower,ts,default}, transpiler/{transpiler,decorators,es-decorators,es-decorators-esbuild,decorator-metadata}, bundler_decorator_metadata, lower-using-bun-target, explicit-resource-management, bake/dev-and-prod (using runtime import).
• Related PRs: Convert top-level class statements to var declarations when bundling #32655 adds the same class-to-var rewrite to these s_class lines for all bundled output (size motivation; its condition already includes the using mode but it has no using coverage and has been open since June), so the two conflict textually; if it is picked up it can widen the condition here and add its name dropping inside convert_class_stmt_to_var. js_parser: keep export default function/class when lowering top-level using #38287 handles export default function/class in this mode and removes the finalize arm adjacent to the one removed here, so whichever of the two lands second needs a trivial rebase. js_parser: keep destructured exports when lowering top-level using #38279 (destructured exports) touches a different part of the same function.

Background

• Lowering using: for targets without native using, the parser rewrites using x = v into var x = __using(stack, v) and wraps the statements that follow in try { ... } catch { ... } finally { __callDispose(stack, ...) }. At module level the whole module body ends up inside that try block. will_wrap_module_in_try_catch_for_using is set before the visit pass so statements can be adjusted while they are visited; finalize builds the try/catch afterwards.
• Function declarations are kept outside the try block because ESM hoists them: an importer in a cycle may call them before this module's body has run. Anything such a function refers to therefore has to be visible at module scope, which is why this mode already turns top-level let / const into var (select_local_kind); a class declaration is block-scoped like let, so it needs the same treatment.
• export declarations are only valid directly at module level, so an exported binding inside the try block is exported through an export { ... } clause placed after it. Bun's export list (named_exports) is computed from the visited statements, so the clause is what makes the export exist.
• __esm wrapper: when bundling, a module that must be evaluated lazily (for example because it is import()ed) has its body wrapped in a closure, and the linker turns the module's top-level declarations into declarations outside the closure plus assignments inside it, so the export getters can reach them. Declarations nested in the try block are invisible to that pass unless the parser relocates them first, which is what js_parser: hoist the declarations of a lowered top-level using out of the ESM wrapper and the REPL IIFE #38284 adds.
• lower_class returns the class statement plus any statements decorator lowering adds around it; the replacement swaps out the class statement within that list.

Repro and output

// cls.mjs
using foo = { v: 1, [Symbol.dispose]() {} };
class Bar { static v = foo.v; }
export function fn() { return new Bar().constructor.v; }
export class Baz { static v = foo.v; }

// entry.mjs
import { fn, Baz } from "./cls.mjs";
console.log("fn():", fn(), "Baz.v:", Baz.v);

$ node entry.mjs
fn(): 1 Baz.v: 1
$ bun build --target=browser entry.mjs --outfile=out.mjs && node out.mjs   # 1.4.0
TypeError: Cannot read properties of undefined (reading 'v')

cls.mjs section of the bundle before:

function fn() { return new Bar().constructor.v; }
class Baz { static v = foo.v; }          // runs before `foo` is assigned
let __stack = [];
try {
  var foo = __using(__stack, { v: 1, [Symbol.dispose]() {} }, 0);
  class Bar { static v = foo.v; }        // scoped to the try block; fn() cannot see it
} catch (_catch) { var _err = _catch, _hasErr = 1; } finally { __callDispose(__stack, _err, _hasErr); }

After (on top of #38284, which is what turns the block's vars into assignments plus separate top-level declarations; in an __esm wrapper the linker merges those into one var __stack, foo, Bar, Baz; outside the wrapper):

function fn() { return new Bar().constructor.v; }
let __stack = [];
try {
  foo = __using(__stack, { v: 1, [Symbol.dispose]() {} }, 0);
  Bar = class Bar { static v = foo.v; };
  Baz = class Baz { static v = foo.v; };
} catch (_catch) { var _err = _catch, _hasErr = 1; } finally { __callDispose(__stack, _err, _hasErr); }
var foo;
var Bar;
var Baz;

bun build --no-bundle / Bun.Transpiler output (no relocation outside the bundler):

export function fn() { return new Bar().constructor.v; }
let __bun_temp_ref_1$ = [];
try {
  var foo = __using(__bun_temp_ref_1$, { v: 1, [Symbol.dispose]() {} }, 0);
  var Bar = class Bar { static v = foo.v; };
  var Baz = class Baz { static v = foo.v; };
} catch (__bun_temp_ref_2$) { ... } finally { ... }
export { Baz };

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 43 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: 21e6e136-071b-466e-8106-2947a13df0cc

📥 Commits

Reviewing files that changed from the base of the PR and between 18391f6 and bd48c94.

⛔ Files ignored due to path filters (1)

• test/bundler/transpiler/__snapshots__/transpiler.test.js.snap is excluded by !**/*.snap

📒 Files selected for processing (4)

• src/js_parser/p.rs
• src/js_parser/visit/visit_stmt.rs
• test/bundler/bundler_edgecase.test.ts
• test/bundler/transpiler/transpiler.test.js

---

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

[robobun]

Reproduced on bun 1.4.0 and main with bun build --target=browser entry.mjs on the two-file repro in the PR description (TypeError: Cannot read properties of undefined (reading 'v'); with that class fixed, ReferenceError: Bar is not defined), and with bun build --no-bundle --target=browser cls.mjs, which prints the same mis-scoped output. esbuild 0.25.12 keeps both classes inside the try block.

This PR is stacked on #38284 (its branch is the base here) and should land after it; edgecase/UsingTopLevelClassDeclarationsInEsmWrapper fails if the two are applied in the other order. The seven new tests fail on the base branch without the src/ change and pass with it.

CI on 612112d: every test job passed; the build is marked failed only because the two darwin 14 aarch64 - test-bun jobs expired waiting for an agent (the remaining annotations are retries that passed). Not re-running it now, since this will be rebased onto main once #38284 lands, which re-runs CI anyway.

[claude]

I reviewed this PR and didn't find any bugs. Since it rewrites how the parser lowers top-level class declarations (with knock-on interactions across decorators, TS namespaces, HMR, and finalize), a human look would still be worthwhile.

Checked: data and the SClass in lowered share the same arena StoreRef, so mem::take(&mut data.class) moves the (already-visited/lowered) class body; the later was_export_inside_namespace read of data.class.class_name is unreachable because a namespace body is never the module scope; and the removed SClass arm in finalize is dead now that module-level classes arrive as SLocal.

Extended reasoning...

Overview

The PR fixes miscompiled output when a module containing top-level using/await using is built for a target that lowers using (browser/node). Previously, exported class declarations were hoisted above the generated try/catch (evaluating before the using value was assigned) and non-exported ones stayed block-scoped inside it (invisible to hoisted function declarations). The fix rewrites each module-scope class statement into var Foo = class Foo {} during the visit pass — the class analogue of what select_local_kind already does for top-level let/const in this mode — and removes the now-dead SClass arm from LowerUsingDeclarationsContext::finalize. Five itBundled tests (exported/non-exported, subclass, await using, both decorator flavors, internal_bake_dev) and one transpiler snapshot cover the change.

Security risks

None. This is a bundler/transpiler output-correctness fix; no untrusted-input parsing surface changes.

Level of scrutiny

Medium-high. The change is small (~50 lines of Rust plus tests) and well-localized, but it sits in the class-lowering path and interacts with several subsystems: lower_class (both decorator flavors emit extra statements around the class), TS namespace merging (was_export_inside_namespace), the export-clause construction in finalize's SLocal arm, and HMR export scanning. The PR description explains each interaction and the tests exercise them at runtime, but AST rewrites that change emitted semantics warrant a maintainer glance.

Other factors

• The gate p.current_scope().parent.is_none() matches the existing gates at visit_stmt.rs:512 and p.rs:5858, so nested-scope classes are untouched (verified by class G {} inside f() in the transpiler snapshot).
• data: &mut S::Class and the SClass slot found in lowered deref the same arena StoreRef (both lower_class paths keep the original stmt's handle), so mem::take(&mut data.class) moves the correct, post-lower_class body; data.is_export is read after the take but lives on the outer S::Class, not the taken G::Class.
• The was_export_inside_namespace block that reads data.class.class_name after the rewrite requires enclosing_namespace_arg_ref.is_some(), which never holds at module scope, so the post-take None is never observed.
• The removed SClass arm in finalize handled only is_export classes; since all module-scope classes now reach finalize as SLocal (routed through the existing export-clause path), and non-module-scope class statements are never exported, the arm is dead.
• Test coverage is thorough (runtime assertions on .name, self-reference, prototype chain, decorator return values, plus output-shape assertions that don't over-constrain the linker), and robobun confirmed all six fail on the release build.

[robobun]

^{Updated 6:05 AM PT - Aug 14th, 2026}

❌ @robobun, your commit 612112d has some failures in Build #95645 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38292

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

bun-38292 --bun

[claude]

I reviewed this PR and didn't find any bugs. Because it changes bundler/transpiler output semantics for top-level class lowering, is stacked on #38284, and interacts with adjacent open PRs (#38287, #38279, #32655), a human look would still be worthwhile to confirm the sequencing and merge order.

What was reviewed:

• Traced convert_class_stmt_to_var against lower_class's output: the SClass in lowered shares arena storage with data (both plain and lower_standard_decorators_stmt push original_stmt), so mem::take(&mut data.class) moves the right body.
• Confirmed the module-scope guard (current_scope().parent.is_none()) matches select_local_kind's condition and is mutually exclusive with was_export_inside_namespace (namespace bodies are nested scopes), so the later data.class.class_name.expect(...) at line 1095 is not reached after mem::take.
• Checked the removed SClass arm in finalize has nothing left to handle: module-level classes now arrive as SLocal, and nested-block classes are never exported.

Extended reasoning...

Overview

The PR fixes top-level class declaration scoping when a module contains a top-level using/await using and is built for a target that lowers using (browser/node). The source change is ~35 lines: a new convert_class_stmt_to_var helper in src/js_parser/visit/visit_stmt.rs that rewrites the SClass returned by lower_class into var Foo = class Foo {} when at module scope in this mode, plus removal of the now-dead SClass arm in LowerUsingDeclarationsContext::finalize. Seven new tests cover bundled output, __esm-wrapped modules, await using, both decorator flavors, the bake dev format, and a Bun.Transpiler snapshot.

Security risks

None. This is an AST transformation in the parser's visit pass; no untrusted input handling, no I/O, no auth/crypto/permissions surface.

Level of scrutiny

Medium-high. The change is small and well-tested, matches the documented intent above will_wrap_module_in_try_catch_for_using in parse_entry.rs, and mirrors esbuild's output for the same input. But it changes emitted code for a real (if narrow) input class across every non-bun target, is stacked on an unlanded PR whose var-relocation is load-bearing for the __esm-wrapper case, and sits next to three other open PRs touching the same function. The sequencing constraint (UsingTopLevelClassDeclarationsInEsmWrapper fails if applied to main without #38284) is real and worth a human confirming the base has landed before this merges.

Other factors

• Test coverage is thorough: runtime assertions on .name, static field values, prototype chains, and hoisted-function visibility, plus output-shape checks and a snapshot; the PR states all seven fail on the base without the src/ change.
• The .expect("infallible: class statements are always named") matches the two identical assertions already in s_class (anonymous classes are only expressions or export default, never SClass).
• The comment-cop bot's four flags were addressed in 612112d (condensed to a one-line pointer to select_local_kind); no other reviewer feedback is outstanding.
• No prior review from me on this PR.

[robobun]

This bug came up again on main at f04caca. The repro reads an earlier const from the extends expression:

// u.mjs
const log = [];
using r = { v: 1, [Symbol.dispose]() {} };
function base() { log.push("extends"); return Object; }
export class A extends base() {}
export class C { static x = r.v; }
console.log(log.join(), C.x);

On main, bun build u.mjs --target=node --outfile=out.mjs && node out.mjs throws TypeError: Cannot read properties of undefined (reading 'push'). node u.mjs prints extends 1. The src/ change of this PR applies cleanly to f04caca. With it, both classes stay inside the try in source order as var A = class A extends base() {} and var C = class C { static x = r.v; }, and the output prints extends 1.

#40833 (open) also changes the SClass arm of LowerUsingDeclarationsContext::finalize. It keeps an exported class inside the try only when the class has static blocks or computed keys, because lowered standard decorators need that. This PR removes the arm and converts every module-level class in s_class, so the two PRs conflict in that arm. The PR that lands second needs a small rebase there. After this PR lands, the arm in #40833 has no module-level class left to handle.