[robobun]

Problem

• With reactCompiler, a local named like a JSX runtime import gets a new name. const jsx = props.a after a nested function with JSX prints as jsx_0 (also jsxs_0, jsxDEV_0, Fragment_0).
• The cause is gather_captured_context (src/react_compiler/lowering/build_hir/mod.rs:382). It captures the JSX runtime imports as context of each nested function with JSX. Each context identifier reserves its name.
• The callee and the Fragment tag of <> share a position. The function sorts a std HashMap by position only, so the order of the two entries differs between processes. No printed output depends on it.

Fix

• gather_captured_context skips a symbol on module_scope.generated, as resolve_identifier does. The output of the 1806 upstream fixtures is unchanged.
• The sort key is now the position and the symbol index. Two captured locals can still share a position: React Fast Refresh lists hooks at Loc::EMPTY. Such a function is not compiled, so no test shows it.
• Verified: test/bundler/transpiler/react-compiler.test.ts (four new cases, two fail without the fix). Also react-compiler-fixtures.test.ts and bundler_jsx.test.ts.
• Self-reviewed: 5 concerns raised, 4 addressed. Rejected: drop the second sort key. The Fast Refresh case still ties.

Background

• Context: the variables that a nested function uses from the enclosing function. Each gets an HIR identifier with a unique name, so a later local with that name gets _0.
• The compiler runs after the visit pass. By then <>a</> is the call jsxDEV(Fragment, {...}). Both identifiers have the position of the <.
• The visit pass declares the JSX runtime imports on module_scope.generated as Kind::Other, not in module_scope.members.

Notes

• Every writer of module_scope.generated makes a module-level symbol: declare_generated_symbol, import namespace and import item refs, CommonJS export symbols, module-level temporaries, Host::new_generated / new_import_item. hoist_symbols only appends to the generated list of a block scope. The compiler only takes module-level functions. So the new check cannot drop a real capture.
• The Fast Refresh function is not compiled because its var _s = $RefreshSig$() is a Todo (Handle var kinds in VariableDeclaration).
• Why the order does not reach the output: lowering never loads the callee or the Fragment tag (lower_jsx_call does not make the callee an operand, and it turns the Fragment tag into JsxFragment). Dead code elimination of the nested function then removes both context entries, before the passes that build scopes.
• How the order was found: a dump of the HIR after EliminateRedundantPhi for the whole fixture corpus, two runs of one binary. One fixture differed, multiple-calls-to-hoisted-callback-from-other-callback.js. The context of the arrow b was (jsxDEV, Fragment, onClick) in one run and (Fragment, jsxDEV, onClick) in the next, and the declaration ids followed.
• A probe build that logs two context entries with one position: 2 events on the corpus without the generated check (both Fragment / jsxDEV), 0 with it. With --react-fast-refresh and the check, a component with const Inner = () => { useA(); useB(); ... } (both hooks from props) logs useA / useB at position -1.
• Output comparison: one Bun.build of all fixture inputs with the options of react-compiler-fixtures.test.ts. It gives 1585 outputs and 221 expected errors. diff -r of the outputs of the two builds is empty, also with minify: { syntax: true }.
• The new cases use a CommonJS runtime stub for the names: the bundler prints its exports as $jsx, so a suffix on a local can only be the _0 of the compiler. With the ES module stub (or an external react) the import prints as jsx, and the local gets its number from the renamer of the bundler (jsx2), like a local of a function that the compiler does not touch.
• Sites with the same checks, left as they are: is_module_level_or_global (build_hir/expr.rs) has no generated arm, but the JSX imports and runtime helpers are EImportIdentifier, which returns before it. is_context_identifier only answers for reassigned locals. resolve_identifier needs the finer ModuleLocal / Global answer, so one shared helper does not fit all three.
• No other std HashMap / HashSet in src/react_compiler/lowering feeds an ordered result: the others are used for lookups, or collected into an unordered set.
• route react compiler sorts through the shared index sort #39506 changes the same sort_unstable_by_key line to the shared index sort. The comparator there needs the same second key.

[coderabbitai]

Warning

Review paused — included plan limit reached

Keep your review moving with free on-demand reviews.

• Run this review for free

On-demand reviews are free for one more day.

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Promotion and pricing details

On-demand reviews are free for one more day. After that, they cost $0.25 per reviewed file.

Review limit details

Or wait 6 minutes for your next included review.

Check out review usage here.

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 029afec8-a9a4-47da-a9f1-ba3c9480b091

📥 Commits

Reviewing files that changed from the base of the PR and between 9b7c982 and a9a52c7.

📒 Files selected for processing (2)

• src/react_compiler/lowering/build_hir/mod.rs
• test/bundler/transpiler/react-compiler.test.ts

---

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

[robobun]

^{Updated 2:01 PM PT - Sep 19th, 2026}

✅ @robobun, your commit a9a52c77ff953eca80cd7d9aac9be0d011fbb70c passed in Build #118451! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 43528

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

bun-43528 --bun

[claude]

LGTM — a small, self-contained fix that brings gather_captured_context in line with the module_scope.generated check resolve_identifier already makes.

What was reviewed:

• Traced every writer of module_scope.generated in the parser (declare_generated_symbol, import namespace refs, CJS export shims, Host::new_generated/new_import_item, hoisting): all are module-level, so the new skip cannot drop a real captured local; Host::new_local and declare_temp_var append to the function scope, not the module scope.
• The (pos, inner_index) sort key makes the HashMap collection order total; no consumer depends on the previous tie order.
• The four new itBundled cases cover Dev/Prod × CJS/ESM runtime stubs; the CJS variants assert both that Comp was compiled (memo cache read) and the exact unsuffixed local names, and all variants assert runtime stdout. The ESM variants pass with and without the fix, but they guard the bundler's renamer path rather than the compiler, which is a reasonable inclusion.

Extended reasoning...

Overview

The PR touches one function in the React Compiler HIR lowering (/home/claude/bun/src/react_compiler/lowering/build_hir/mod.rs, gather_captured_context) and the existing test file /home/claude/bun/test/bundler/transpiler/react-compiler.test.ts. The code change is two lines of logic: skip symbols that live on module_scope.generated when collecting the captured context of a nested function, and add the symbol index as a tiebreaker to the position sort so HashMap iteration order cannot leak into the result. The test change hoists the fake react fixture into a shared object, derives an ESM variant, and adds four itBundled cases.

Security risks

None. This is compile-time transpiler logic for an opt-in feature (reactCompiler); it does not parse untrusted network input, touch file system paths, or affect runtime security boundaries. The fixture rewrite (exports.X = to export const X =) is test-only.

Level of scrutiny

Moderate. The change is small, but the correctness question is whether module_scope.generated can ever contain a non-module-level binding that a nested function legitimately captures. I checked every append site in src/js_parser/p.rs, fold.rs, and react_compiler_host.rs: the ones that write to module_scope_mut().generated are all module-level constructs (JSX/runtime auto-imports via declare_generated_symbol, import namespace refs, CJS export symbols, Host::new_generated/new_import_item), while Host::new_local, declare_temp_var, generate_temp_ref, and hoisting write to current_scope/the nearest hoisting scope, which is the function scope when inside a function. So the skip is sound and mirrors the identical check already in resolve_identifier (hir_builder.rs:1030). The sort tiebreaker is pure determinism with no output consumer depending on the old tie order.

Other factors

No debug build was present in this checkout, so I did not execute the tests; the assessment rests on reading the diff and the parser/lowering sources. The new tests follow the existing file's conventions (itBundled, run.stdout, onAfterBundle), cover both jsx.development states and both module formats for the runtime stub, and the CJS variants assert exact local names rather than substring presence. The changed paths are not covered by CODEOWNERS, there are no prior review threads, and the bug hunt exited on dry_streak. The two ruled-out candidates (dropped real capture; ESM cases passing both ways) were re-examined and hold: the former is excluded by the writer audit above, and the latter is a variant-matrix inclusion guarding the bundler renamer rather than a vacuous assertion.

[robobun]

Status: ready for review. CI is running.

How to reproduce on main

// entry.jsx
export function Comp({ items }) {
  function cb() {
    return <>{items}</>;
  }
  const Fragment = items.length;
  return <div onClick={cb} title={Fragment} />;
}

bun build --react-compiler --target=browser --external '*' entry.jsx

• On main the local prints as let Fragment_0 = items.length;. The compiler renames it, because the context of cb reserved the name Fragment.
• With this branch it prints as let Fragment2 = items.length;. The bundler numbers it, because the file imports Fragment. This is what happens to a local of a function that the compiler does not touch.

Test

bun bd test test/bundler/transpiler/react-compiler.test.ts -t JsxRuntimeImportIsNotContextOfANestedFunction

The -Dev and -Prod cases fail on main (jsxDEV_0, Fragment_0, jsx_0, jsxs_0) and pass with this branch. The two -EsmRuntime cases pass on both. They check that the bundle still runs when the imports print under their own names.