react-compiler: keep the declaration of a local when dead code elimination keeps a store to it

[robobun]

Problem

• bun build --react-compiler can print an assignment to a local with no declaration: ReferenceError: v is not defined. With v never read, try { try {..} catch { v = 1 } .. } catch { v = 2 } prints let v = 1 in the inner handler and a bare v = 2 in the outer one. In client mode the same cause can crash the build: panic: Expected identifier to be initialized.
• Dead code elimination (src/react_compiler/optimization/dead_code_elimination.rs) keeps some stores to a local that nothing reads, but prunes let v: nothing marks the name. rewrite_instruction_kinds_based_on_reassignment then makes the first store left the declaration, in a scope that need not enclose the others. Upstream prints the same text.

Fix

• A Reassign store that the pass retains now marks its variable, so let v stays. A Destructure retained as a block value marks each variable of its pattern.
• Correct because a retained assignment needs its binding, whatever retained it. Unretained stores are still pruned.
• Verified: three new tests in test/bundler/transpiler/react-compiler.test.ts fail without the fix. All 1806 upstream fixtures print byte-identical output before and after, in both output modes. Also ran react-compiler-fixtures.test.ts and the react tests in bundler_jsx.test.ts.

Background

• The compiler lowers a function to HIR: basic blocks of SSA instructions. let v is a DeclareLocal, v = 1 a StoreLocal of kind Reassign.
• The pass marks what is referenced and sweeps the rest. A declaration stays while the variable's name is marked. A Reassign store stays while its own SSA value is read.
• Two more things keep a store: its own result is used (f(v = 2)), or it ends a block whose kind is not Block. That rule protects value blocks (a ternary branch, a for..of test). It also covers a catch handler, kind Catch.

Notes

Shapes probed (v is never read in each). Before: main 4b5862f and 1.4.3-canary.1+6a92015fc. After: this branch. "Left alone" means the compiler skips the function and the source runs as written.

shape	before	after
nested try, a store ends each handler (the report)	let v = 1 / bare v = 2, ReferenceError	let v kept, compiled
two try in sequence, a store ends each handler	same	same
[v] = p.items ends two handlers	let [v] = .. / bare [v] = .., ReferenceError	compiled
handler store, then f(v = 2) or c && (v = 2)	ReferenceError	compiled
let v = "init", handler stores, v = "final", <div>{v}</div> (the read folds to "final")	ReferenceError	compiled
for (v of xs) {} or for (v in xs) {}, then f(v = 2) or a handler store	for (let v of xs) / bare v = 2, ReferenceError	left alone
for (..; ..; v = i++) twice, client mode	panic: Expected identifier to be initialized (prune_non_escaping_scopes.rs:1060, a scope reassigns a local that was never declared)	compiled
f(v = 1); f(v = 2), c ? (v = 1) : (v = 2), a && (v = 1); b && (v = 2)	left alone (Const declaration cannot be referenced as an expression)	compiled

Nesting is not needed. In a handler nested in a try, every instruction ends its own Catch block (each one can throw to the outer handler), so every store there is kept. In a plain handler only the last instruction is.

Upstream. babel-plugin-react-compiler@1.0.0 prints let v = 1 / bare v = 2 for the two-try-in-sequence form. For the nested form it raises Expected a break target and skips the function, an older limit on nested try. 0.0.0-experimental-a1856f3-20260507 compiles the nested form and prints the same broken text. It also prints for (let v of p.items) {..} foo(v = 2). DeadCodeElimination.ts and the Rust port on facebook/react main have the same logic as UPSTREAM_PORTED.

Why not narrow the block-value rule to expression blocks. Upstream's isStatementBlockKind calls catch a statement kind, so the rule covers handlers by accident. Narrowing it removes the handler cases only. A for..of test is a real value block, and let v; for (v of xs) {}; f(v = 2) breaks the same way.

Why not fail in rewrite_instruction_kinds_based_on_reassignment. An invariant there for a Reassign store with no declaration also stops the bad output, but it leaves every such function uncompiled.

Cost. The dead stores stay in the output, as in the source. let v; for (v of xs) {} with v never read used to compile as for (const v of xs). It is now left alone, like any for (v of xs) over an outer local that is read later (Unexpected Reassign variable in for..of collection).

Not changed here. The panic above is an upstream invariant that the port writes as .expect(). Other invariants written that way also end the build where upstream skips the function.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file.}

Or wait 11 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: ec8ac22d-1b73-48ea-870c-a3fddbbdabd2

📥 Commits

Reviewing files that changed from the base of the PR and between 471b586 and 68bda28.

📒 Files selected for processing (2)

• src/react_compiler/optimization/dead_code_elimination.rs
• test/bundler/transpiler/react-compiler.test.ts

---

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

[robobun]

Status

Reproduced on main 4b5862f and on 1.4.3-canary.1+6a92015fc, client and ssr mode:

// ds.jsx (react stubbed under node_modules/react)
function App(p) {
  let v;
  try {
    try { JSON.parse(p.t); } catch { v = 1; }
    JSON.parse(p.t);
  } catch { v = 2; }
  return <div />;
}
console.log(App({ t: "{bad" }).type);

$ bun build --react-compiler --target=bun ds.jsx --outfile=c.js && bun c.js
ReferenceError: v is not defined

The same happens without nesting (two try in sequence) and without try (let v; for (v of xs) {}; f(v = 2)). babel-plugin-react-compiler 1.0.0 and 0.0.0-experimental-a1856f3-20260507 print the same text.

• Fail before: USE_SYSTEM_BUN=1 bun test test/bundler/transpiler/react-compiler.test.ts -t DeadStore fails 3 of 3. A debug build of main fails the same 3.
• Pass after: bun bd test test/bundler/transpiler/react-compiler.test.ts passes 48 of 48.
• Self-review: the PR merged before the review of the diff finished. The review had raised no concern by then, and it was stopped.

[claude]

I reviewed this PR and didn't find any bugs. Because the fix intentionally diverges from upstream DeadCodeElimination.ts in a ported optimization pass, a human sign-off on that divergence would still be worthwhile.

What was reviewed:

• reference_reassigned_variables and both call sites — the block-value site covers Destructure; the retained-instruction site sits inside the StoreLocal branch, and the comment there correctly explains why a retained non-block-value Destructure doesn't need it (rewrite_instruction prunes the dead pattern entries).
• stubReactWithEffect override — replaces react/index.js with only useEffect, but the forms don't need useState/createElement and the jsx-runtime / compiler-runtime stubs are preserved via the spread.
• The onAfterBundle regex asserts each useEffect(() => {}) was outlined/dropped, so a bailout that "left alone" a form would fail the test — the assertions can fail for the right reason.

Extended reasoning...

Overview

The PR fixes a DCE bug in Bun's React Compiler port: when a Reassign store to a local is retained (as the last instruction of a non-Block block, or because its own SSA value is used) but nothing reads the variable, the original let v was pruned, and rewrite_instruction_kinds_based_on_reassignment promoted the first surviving store to a declaration in a scope that didn't enclose the others — yielding ReferenceError at runtime or a panic: Expected identifier to be initialized in client mode. The fix adds a small helper that marks the lvalue identifier(s) of a retained reassignment as referenced, keeping the original declaration. Rust change is ~30 lines (one helper, two call sites, doc comment); ~190 lines of itBundled tests cover nested/sequential catch handlers, destructuring in a handler, handler+argument, handler+logical, every-read-folded, for..of/for..in heads under both browser and bun targets, and the for-update panic case.

Security risks

None. This is an optimization pass over compiler-internal HIR; no user input parsing, no I/O, no auth/crypto surface. The worst case for a bug here is incorrect codegen or the compiler bailing out on a function.

Level of scrutiny

Medium. The Rust change is small and the mechanism is clear (mark the reassigned variable's name so pruneable_value on the DeclareLocal returns false). The second call site sits inside the if let InstructionValue::StoreLocal branch, so its Destructure arm is dead there — but that's intentional and the adjacent comment explains why (a non-block-value Destructure is handed to rewrite_instruction, which holes out the unread pattern entries, so no dangling reference survives). The Destructure arm is exercised at the block-value call site. What raises the bar is that this is ported code and the fix is explicitly "Not in upstream" — REVIEW.md treats the reference implementation as the spec for ported passes, so a deliberate divergence, however well-argued, is the kind of decision a maintainer should acknowledge.

Other factors

The PR description is unusually thorough: it names the exact mechanism, shows a shape table before/after, confirms upstream (both 1.0.0 and an experimental build) prints the same broken text, explains why two alternative fixes (narrowing the block-value rule; adding an invariant in rewrite_instruction_kinds_based_on_reassignment) were rejected, states the cost (dead stores stay in output; for (v of xs) over a dead v is now left alone rather than compiled to for (const v of xs)), and reports all 1806 upstream fixtures byte-identical in both output modes. Tests were added to the existing react-compiler.test.ts per convention, use itBundled, cover both targets, and include an onAfterBundle assertion that the compiler actually compiled the forms (not just bailed). No CODEOWNERS entry covers these paths, no prior reviewer objections in the timeline, and the bug hunt ran to dry_streak with no findings.

[robobun]

^{Updated 5:07 PM PT - Sep 11th, 2026}

❌ @robobun, your commit 68bda28 has 2 failures in Build #114465 (All Failures):

• test/js/bun/http/serve-pending-promise-abort-leak.test.ts - code 1 on 🐧 13 x64-asan
• test/cli/run/require-cache.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 42385

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

bun-42385 --bun