Skip to content

react-compiler: judge only the operands of a JSX element in a default value - #45991

Open
robobun wants to merge 1 commit into
robobun/8af83e6b/react-compiler-param-default-candidatefrom
robobun/630de100/jsx-default-operands
Open

robobun wants to merge 1 commit into
robobun/8af83e6b/react-compiler-param-default-candidatefrom
robobun/630de100/jsx-default-operands

Conversation

@robobun

@robobun robobun commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #45550.

Problem

  • bun build --react-compiler compiles a function with JSX in a default value ({ icon = <Spinner /> }) or case test in production and silently leaves it as written in development: (BuildHIR::node.lowerReorderableExpression) Expression type `CallExpression` cannot be safely reordered. An effect that depends on the default runs on every render in development only.
  • is_reorderable_expression (src/react_compiler/lowering/build_hir/expr.rs:1428) judges every argument of the parser's call. jsxDEV ends in isStaticChildren, source, self, and self is this.

Fix

  • automatic_jsx_call (build_hir/jsx.rs) returns the operands of an automatic-runtime call: tag, props, key. A new arm of is_reorderable_expression judges only those. lower_jsx_call reads it too.
  • Development takes production's verdict. Production, classic and bun lint verdicts stay.
  • Verified: react-compiler/JsxInDefaultValue-* in test/bundler/transpiler/react-compiler.test.ts (23 forms, 3 JSX modes). Development fails on the base.
  • Self-reviewed: 6 concerns raised, 6 addressed.

Background

  • The parser lowers JSX to a call before the React Compiler runs, which decodes it.
  • A default is evaluated out of source order, so its operands must be safe to reorder. Upstream never takes JSX there. Bun does (jsx_can_be_reordered, hir/environment_config.rs:91).
  • Considered this as reorderable (same output, the check still walks non-operands) and a callee check in the arm (a copy of the decoder's rule).

Downsides

  • Development behaviour changes: these functions are memoized. Fuzz matrix, 14,836 candidates: 5,054 more compile in development, 0 production verdicts change.
  • The default build stops matching upstream here. Draft bun test: Vitest compatibility #44833 pins the match for one function (WithADefault: []).
  • Other callers: none measured. 1,482 Host::jsx_import_kind calls on the fixture corpus and a 106,911,456-byte release binary, before and after.
Notes

Repro

mkdir -p node_modules/react
echo 'export const c = n => new Array(n).fill(Symbol.for("react.memo_cache_sentinel"));' > node_modules/react/compiler-runtime.js
echo 'export const jsxDEV = (type, props) => ({ type, props }), jsx = jsxDEV, jsxs = jsxDEV;' > node_modules/react/jsx-dev-runtime.js
cp node_modules/react/jsx-dev-runtime.js node_modules/react/jsx-runtime.js
echo 'const Spinner = () => null; export function Card({ title, icon = <Spinner /> }) { return <div title={title}>{icon}</div>; }' > card.jsx
for mode in development production; do
  NODE_ENV=$mode bun build ./card.jsx --react-compiler --outfile out.js > /dev/null
  grep -q memo_cache_sentinel out.js && echo "$mode: memoized" || echo "$mode: left as written"
done

Before: development: left as written, production: memoized. With --jsx-runtime=classic development is memoized too, because createElement(tag, props, ...children) has no trailing this. A debug build prints the Todo with BUN_DEBUG_react_compiler=1. The report came from a fuzz run. There is no user report.

Direction: this needs a maintainer's word

Upstream compiles a JSX default in neither mode: its isReorderableExpression has no JSX case. bun lint keeps that verdict (jsx_can_be_reordered: false). bun build compiles it in production by the rule at environment_config.rs:91, and this PR makes development agree. The other consistent direction is jsx_can_be_reordered: false for bun build. Then production stops compiling these functions and this PR is not needed.

Draft #44833 has react-compiler/WhichFunctionsAreCompiled, whose expected sizes come from babel-plugin-react-compiler. Its row WithADefault: [] is for ({ depth, render = () => <b /> }). Development gives [] there today only because of the trailing this. Production gives a compiled function for the same source. With this PR development gives one too. The two branches merge without a textual conflict in that row, so whichever lands second has to change it.

What changes in development, measured with React 18.3.1 on debug builds of the base and of this branch

  • Card({ title, icon = <Spinner /> }) with useEffect(fn, [icon]), five renders: the effect runs 5 times in development and 1 time in production on the base. On this branch it runs 1 time in both.
  • ({ icon = <b title={later} />, later }) throws ReferenceError in development on the base and not in production, because the compiled function reads the whole pattern before it evaluates a default. On this branch it throws in neither. The same holds for the pattern in a const and for an array pattern.
  • A function that is not compiled still gets no message, in any mode. bun lint reports it.
  • The classic runtime can still differ from both: icon = <Spinner {...{ size: 1 }} /> compiles in development and production and not with the classic runtime, because only the automatic transform folds that spread into the props.

Why it is stacked on #45550

On main a function in a parameter list takes the compile of its component (#45550). For item = ({ value, fallback = <Spinner /> }) => ... that compile fails in development today, for the reason this PR removes. So with this PR alone on main, that arrow would get the cache hook in development as it already does in production: with React 18.3.1, two rows and then three rows, Rendered more hooks than during the previous render. On top of #45550 neither mode compiles the arrow. The row ArrowInParameterList holds that order: it passes only on top of #45550.

What the check covers

All three callers of lower_reorderable_expression go through the new arm: defaults in binding patterns (stmt.rs lower_default_value), defaults in parameters and assignment patterns (helpers.rs lower_assignment_pattern_default), and case tests (stmt.rs). The controls of the test hold each clause of the arm:

Control Clause
MemberOfLocalTag the tag is judged
SpreadProp, MemberOfLocalChild the props are judged
MemberOfLocalKey the key is judged
LocalInArrow, LocalTag, LocalKey the caller's rule for locals is passed on
MemberOfLocalChild with the classic runtime the arm keys on the callee: createElement(Spinner, null, 1, 2, 3, rest.id) has six arguments too

Self-review: what was raised and what changed

  1. The first wording said that this as reorderable would change the result for a user-written this. It would not: lowering refuses every this. The reason to prefer the decoder is structure, and Background says so now.
  2. No form took the path of the WithADefault row. ArrowReturningJsx does now, and the row is named above.
  3. The defect in the parameter list had an open PR. This PR is stacked on it and links it.
  4. The controls only failed through the props, and one control could not fail at all. Six rows were added and that control was removed.
  5. Three sentences in DESIGN.md and in comments were not exact: lowering reads isStaticChildren, and a case test is evaluated earlier, not later. They are corrected.
  6. An eight-line doc comment became one line that points at DESIGN.md.

lower_jsx_call keeps its read of the key (args.get(2)) and its comment on args[3..6], so this branch merges with #42935 without a conflict.

Measurements (debug and release builds of the base, #45550 at f9b900b, and of this branch, linux-x64)

  • JSX in a default or case test (17 forms + 6 controls), functions compiled in development / production / classic development: base 0 / 17 / 17, PR 17 / 17 / 17. Functions with the reorder Todo, development vs production: base 23 vs 6, PR 6 vs 6.
  • Fuzz matrix, 362 files / 14,836 candidates: compiled in production only 0 (base 5,054), in development only 0 (base 0), candidates whose bail reasons differ by mode 0 (base 399). Production records are identical for 14,836 of 14,836 candidates, and 362 of 362 production outputs are byte-identical.
  • Host::jsx_import_kind calls (gdb breakpoint hit counts): one build of 1,714 fixture inputs, base 1,482, PR 1,482. One-line repro export function C({ i = <a /> }) { return <b>{i}</b>; }, development / production: base 2 / 2, PR 3 / 3.
  • is_reorderable_expression entries for icon = <Spinner />, development / production: base 8 / 4, PR 4 / 3.
  • HIR dumps (BUN_REACT_COMPILER_DUMP_HIR=1) and outputs, 1,807 fixture files x 2 JSX modes, base vs PR: 0 dumps differ, 0 outputs differ.
  • lower_jsx_call indirect calls in the release build: base 4 (2 are dyn Host), PR 3, plus 1 in automatic_jsx_call (2 are dyn Host). automatic_jsx_call is not inlined: each lowered element makes one more direct call. AST allocations on an input with no default (two components): base 1,460, PR 1,460.
  • Release binary: 106,911,456 bytes stripped and 106,739,638 bytes of text, before and after. lower_jsx_call 5,321 -> 5,255 bytes, is_reorderable_expression 889 -> 1,018 bytes, automatic_jsx_call 126 bytes (new).
  • is_reorderable_expression: +9 / -0 lines, all in one Bun-only arm. Upstream arms changed: 0.

Suites run on the debug build

test/bundler/transpiler/react-compiler.test.ts (124 pass), react-compiler-fixtures.test.ts (3,293 pass), react-compiler-fixtures-run.test.ts, test/cli/lint/react-compiler.test.ts and test/cli/lint/conformance.test.ts (878 pass together), test/bundler/bundler_jsx.test.ts and test/bundler/transpiler/transpiler.test.js (302 pass). No snapshot entry changes. JsxInDefaultValue-Development fails on a debug build of the base (17 functions not compiled). The production and classic cases pass before and after: they pin the verdict that development has to match.

The fixture and lint suites needed longer timeouts on my machine, for the base binary too: one full-corpus build took 70 to 140 s there.

Not changed here

… default value

A default value and a case test are lowered only if each operand is safe to
reorder. The check took a JSX element as a plain call and judged every
argument. A development `jsxDEV` call ends in `isStaticChildren, source,
self`, and `self` is `this`, which is not reorderable. So a function with
JSX in a default was compiled in production and left as written in
development.

`automatic_jsx_call` in build_hir/jsx.rs says which call is an
automatic-runtime call and returns its callee, props and operands (tag,
props, key). `lower_jsx_call` takes the call shape from it, and a new arm
of `is_reorderable_expression` judges only the operands.
@robobun

robobun commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced with the script in the PR notes on bun 1.4.3-canary.1+367d939d9 and on a debug build of the base of this PR: development: left as written, production: memoized. With BUN_DEBUG_react_compiler=1 the debug build prints the Todo (BuildHIR::node.lowerReorderableExpression) Expression type `CallExpression` cannot be safely reordered for Card in development only.

The fix is this PR (#45991). It is stacked on #45550.

@robobun

robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:05 PM PT - Oct 18th, 2026

❌ @robobun, your commit 65f015d has some failures in Build #128650 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 45991

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

bun-45991 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it changes which functions bun build --react-compiler memoizes in development (and the description itself asks for a maintainer's call on whether JSX in a default should be reorderable at all, versus upstream's verdict), a human look at the direction is still worthwhile.

What was reviewed:

  • lower_jsx_call refactor through automatic_jsx_call: the old (callee, props_arg) match and the new ?-based early returns accept exactly the same calls (callee in jsx/jsxs/jsxDEV and args[1] an E::Object); the args.len() == 6 isStaticChildren branch and the Jsxs fallback are unchanged.
  • The new is_reorderable_expression arm: sits after the jsx_can_be_reordered arm so bun lint is unaffected, only fires when the callee is the auto-imported runtime symbol (so classic createElement calls still take the generic arm, which the MemberOfLocalChild control pins), and the dropped callee judgement was always true for that symbol.
  • && let in a match guard is already used in src/glob/matcher.rs on the pinned nightly, so it compiles; jsx_import_kind remains used in jsx.rs, no dead imports left.
  • Test trio: each clause of the arm has a negative control (tag, props, key), and the dev/prod/classic verdicts are asserted as exact objects.
Extended reasoning...

The change touches the React Compiler's HIR lowering (src/react_compiler/lowering/build_hir/expr.rs and jsx.rs), adding an automatic_jsx_call helper, a new reorderability arm for transpiled automatic-runtime JSX calls, a DESIGN.md paragraph, and a 200-line itBundled test trio across development, production and classic JSX modes. It touches no security-sensitive surface. The Rust diff is small and the refactor is verifiably behavior-preserving, but it changes development-mode compile verdicts for user code and is stacked on another open PR whose base this diff assumes, and the author explicitly flags a design question (whether bun build should compile JSX defaults at all, diverging from upstream) that only a maintainer can settle, which decided defer over approve.

@robobun

robobun commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

The arrow arm next to the one that this PR adds needs the same decision, so one answer can cover both.

cb = () => { return null; } as a default is compiled only with --minify-syntax. The minifier marks a block that is one return to print as => value, and the arrow arm of is_reorderable_expression reads that mark as an expression body. The two builds can agree in two ways:

  1. Upstream's verdict in every build. The compiler reads from the source how the arrow is written and refuses a block with a statement. Branch robobun/371cd3aa/react-compiler-arrow-body-as-written has this. Minified builds then compile less than they do today.
  2. Every build compiles what is safe to move, and bun lint keeps upstream's verdict. jsx_can_be_reordered does this for JSX in a default, and this PR does it for development builds.

#46426 ships only the part that holds either way. Its notes have the measurements for both.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants