[robobun]

Repro

// entry.mjs
export const fromEsm = 'E';
import c from './c.cjs';
console.log('entry got', c);

// c.cjs
let seen; try { seen = JSON.stringify(require('./entry.mjs')); } catch (e) { seen = 'err:' + e.constructor.name; }
module.exports = 'cjs saw ' + seen;

$ bun entry.mjs
entry got {}
$ bun build entry.mjs --outfile=out.js && bun out.js
entry got cjs saw {"fromEsm":"E"}       # bug: binding already initialized
$ npx esbuild entry.mjs --bundle | node
entry got cjs saw {}

Cause

When an ESM module is wrapped in a lazy __esm init closure, top-level
declarations are hoisted to an outer var and their bodies become assignments
inside the closure. generateCodeForFileInChunkJS had a fast path that, when
the initializer satisfied can_be_moved() (strings, numbers, arrows,
side-effect-free classes), pushed the whole declarator (binding and value)
into the outer var:

var import_c, fromEsm = "E";
var init_entry = __esm(() => {
  import_c = __toESM(require_c(), 1);
  console.log("entry got", import_c.default);
});

In an ESM<->CJS cycle the CJS side re-enters init_entry() while it is already
on the stack, gets the partial namespace back, and sees fromEsm already set to
"E" even though the ESM body has not reached that declaration. Bun's own
runtime returns the partial {}, Node throws on the in-progress ESM require,
and esbuild's bundle prints {}. Only Bun's bundle printed {"fromEsm":"E"}.

The same fast path existed for SClass, so export class Foo {} was emitted as
a top-level class Foo {} outside the wrapper and was likewise observable
during the cycle.

Fix

Always hoist only the bare binding and keep the initializer as an assignment
inside the closure, matching esbuild:

var import_c, fromEsm;
var init_entry = __esm(() => {
  import_c = __toESM(require_c(), 1);
  fromEsm = "E";
  console.log("entry got", import_c.default);
});

needs_wrapper_ref() in the parser predicted an empty wrapper body based on the
old hoist rule, so it is updated to agree: any SLocal with an initializer, any
SClass, and any export default that does not lower to an SFunction now
request a wrapper ref.

default/CircularTLADependency2 previously expected the first cyclic read of
c to print "c"; unbundled, both Node and Bun throw a TDZ ReferenceError
there. The bundle lowers const to var, so the first read now observes
undefined, which is the closest approximation a bundler can give. The test's
expected output is updated accordingly.

Verification

New tests in bundler_edgecase.test.ts cover the string-constant, arrow, and
class-declaration variants. Each fails on main and passes with the fix.
Output on the repro is now byte-identical to esbuild's.

---

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_edgecase.test.ts

[coderabbitai]

Walkthrough

Changes

The bundler now requires wrappers for additional initializer cases and routes top-level declarations, including classes, through wrapper hoist assignments. New tests cover cyclic ESM/CommonJS initialization and updated circular top-level-await output.

ESM wrapper cycles

Layer / File(s)	Summary
Wrapper requirement detection src/js_parser/p.rs	P::needs_wrapper_ref now requires wrappers for non-missing local initializers while excluding default function exports from this decision.
Wrapped declaration hoisting src/bundler/linker_context/generateCodeForFileInChunkJS.rs	Hoisted bindings are built through wrapper expressions, and class declarations are emitted as wrapper assignments.
Cyclic loading regression coverage test/bundler/bundler_edgecase.test.ts, test/bundler/esbuild/default.test.ts	Tests verify uninitialized cyclic ESM exports and update circular top-level-await output to begin with undefined.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title matches the main change: keeping const/class initializers inside the __esm wrapper.
Description check	✅ Passed	The description clearly explains the repro, root cause, fix, and verification, covering the required PR details.

---

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

[robobun]

^{Updated 8:50 AM PT - Jul 21st, 2026}

❌ @robobun, your commit 612c6ad has 2 failures in Build #76915 (All Failures):

• test/js/node/test/parallel/test-http2-reset-flood.js - SIGABRT on 🐧 13 x64-asan
• test/js/node/worker_threads/worker_threads.test.ts - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 34934

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

bun-34934 --bun

[github-actions]

Found 2 issues this PR may fix:

1. bun build turns const into var #19046 - PR fixes the correctness problem arising from const being lowered to var by ensuring initializers stay inside the __esm wrapper rather than being prematurely assigned in the outer var declaration
2. ReferenceError: Cannot access uninitialized variable #2927 - PR addresses circular ESM dependency issues where premature initialization made values visible before the __esm closure executed, causing incorrect uninitialized variable behavior

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #19046
Fixes #2927

🤖 Generated with Claude Code

[robobun]

Checked the two suggested issues and neither is closed by this change:

• bun build turns const into var #19046 is about the bundler lowering top-level const to var at all. That lowering is unchanged here; this PR only moves where the initializer is evaluated when the module is wrapped in __esm.
• ReferenceError: Cannot access uninitialized variable #2927 is a runtime TDZ error under bun run with circular ESM imports, not bundler output.

Leaving them off the PR body.

[claude]

Beyond the inline nit, I also checked: needs_wrapper_ref's new SExportDefault arm against convertStmtsForChunk.rs — the four lowered forms (SFunction/SClass/SExpr/Expr) line up with the function-only continue; existing wrapped-ESM tests (EsmWrapperClassHoisting, EsmWrapperElimination1, DeepImportChainWrappedTLA) assert runtime behavior rather than hoisted output shape, so they should be unaffected; and no __esm snapshot fixtures under test/bundler/ need updating.

Extended reasoning...

The nit is cosmetic. The substantive change — removing the can_be_moved() fast path so initializers stay inside the __esm closure, plus the matching needs_wrapper_ref update — reads correctly and matches esbuild. The CircularTLADependency2 expectation change is a legitimate consequence (the old "c" on the first cyclic read was the bug being fixed; undefined is the closest a const→var bundle can get to the unbundled TDZ). Deferring rather than approving because this reshapes output for every wrapped ESM module and CI hasn't reported yet.

[claude]

I didn't find any bugs, but this changes bundler output semantics for every wrapped ESM module and updates default/CircularTLADependency2's expected output — worth a maintainer's eyes.

What was reviewed:

• Traced the SLocal/SClass hoist rewrite against needs_wrapper_ref — the two now agree (any initializer / any class / any non-function export default requests a wrapper ref).
• Checked that ExportDefault::can_be_moved and G::Class::can_be_moved remain live (still used in parse_entry.rs and lower_esm_exports_hmr.rs).
• Verified EsmWrapperElimination1 / EsmWrapperClassHoisting still exercise the modified paths without expectation changes.

Extended reasoning...

Overview

Removes the can_be_moved() fast-path in the WrapKind::Esm hoist loop of generateCodeForFileInChunkJS: for SLocal and SClass, only the bare binding is now hoisted to the outer var; initializers stay inside the __esm closure as assignments. needs_wrapper_ref() in p.rs is updated to predict a non-empty wrapper body under the new rule (any SClass, any SLocal with a non-missing initializer, any SExportDefault that isn't an SFunction). Three new itBundled tests cover const/arrow/class cycle variants, and default/CircularTLADependency2's expected stdout changes from "c" to "undefined" on the first cyclic read.

Security risks

None. Pure bundler code-generation change; no untrusted-input parsing, auth, or FFI surface touched.

Level of scrutiny

Medium-high. The diff itself is small and mechanically clean, but it (a) changes emitted output for every ESM module that gets __esm-wrapped, (b) removes a size optimization (movable initializers now always become in-closure assignments), and (c) modifies an existing test's expected output. REVIEW.md flags existing-test changes as needing explicit justification — the PR body provides one (unbundled Node/Bun throw a TDZ error at that read, so undefined under const→var lowering is the closest a bundler can do, and it now matches esbuild), but a maintainer should confirm that trade-off is the desired one.

Other factors

• The prior review nit (5-line comment) was addressed in 046215b.
• SExportEquals in needs_wrapper_ref still uses can_be_moved(); that's TS export = (CJS-only), so it's not subject to the __esm hoist path and looks intentionally left alone.
• CI on the latest commit was still building at review time; the PR body notes the new tests weren't run locally ("Platform-specific test(s) that do not run on this machine"), so green CI is the actual verification signal here.

[claude]

No bugs found, but this changes bundler codegen semantics for every wrapped ESM module and updates an existing test's expected output (default/CircularTLADependency2 now prints undefined instead of "c"), so it's worth a human sign-off on the behavior/size trade-off.

What was reviewed:

• The SLocal/SClass hoist rewrite in generateCodeForFileInChunkJS.rs — initializer now always becomes an in-wrapper assignment, matching esbuild.
• needs_wrapper_ref in p.rs stays consistent with the new hoist rule; export default function still elides the wrapper.
• Checked that edgecase/EsmWrapperClassHoisting and edgecase/EsmWrapperElimination1 remain correct under the new lowering (assignments run in source order inside the closure).
• S::ExportDefault::can_be_moved() is now unused — flagged and ruled out as intentional dead code.

Extended reasoning...

Overview

Removes the can_be_moved() fast path in the WrapKind::Esm branch of generateCodeForFileInChunkJS: top-level SLocal initializers and SClass bodies are no longer hoisted into the outer var even when side-effect-free; only the bare binding is hoisted and the value becomes an assignment inside the __esm closure. needs_wrapper_ref() in the parser is updated to match (any initializer / any class / any non-function export default now requests a wrapper ref). Three new itBundled tests cover const/arrow/class in an ESM↔CJS cycle, and default/CircularTLADependency2's expected stdout is changed from c to undefined.

Security risks

None — pure bundler output-shape change with no untrusted-input handling.

Level of scrutiny

Medium-high. The diff itself is small and mechanically simple (deleting a fast path), and the reasoning is sound and well-documented against esbuild. But it is a behavior change to codegen for every wrapped ESM module: bundles get slightly larger (initializers move from the merged outer var into separate assignment statements), the wrapper-elimination optimization fires less often (needs_wrapper_ref now returns true for any module with an initialized top-level binding), and one existing test's expected output changed. That output change is arguably more correct (unbundled it's a TDZ error), but per REVIEW.md a modified existing assertion warrants a maintainer look.

Other factors

• The prior inline nit (5-line comment) was addressed in 046215b.
• The SExportEquals arm in needs_wrapper_ref still uses can_be_moved(); that path is TS export = which lowers differently, so leaving it is consistent.
• S::ExportDefault::can_be_moved() loses its only caller; a finder flagged this and it was ruled out as harmless dead code, though REVIEW.md prefers deleting dead code in the same PR.

[robobun]

On the review note about S::ExportDefault::can_be_moved() being unused: it still has a caller at src/js_parser/parse/parse_entry.rs:1072 (the non-bundle transpiler path that reorders export default for cyclical imports, see #1961). Removing it breaks the bun_js_parser build, so it stays.

[robobun]

The diff is ready. All bundler suites pass on every lane of both CI builds.

Remaining red on build 76915 is unrelated to this change:

• test/js/node/worker_threads/worker_threads.test.ts and test/js/node/test/parallel/test-http2-reset-flood.js crash on debian x64-asan with a JSC !exception() scope assertion. Both passed on build 76881, which ran the same code (612c6ad is an empty commit on 046215b), so the crash is not introduced here. This change only touches the bundler's __esm wrapper emission and the parser's needs_wrapper_ref, which is gated on options.bundle and never runs for these tests.
• darwin 14 x64 shard 0 failed at git checkout (unlinkat ... directory not empty, exit 128) and never ran any tests.

Ready for a maintainer to review/merge.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-21, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.