Repository navigation
react-compiler: treat only the last children key of a JSX call as the JSX children - #42393
Conversation
…he JSX children
The parser lowers `<div children="x">y</div>` to
`jsx("div", { children: "x", children: "y" })`. The JSX children are the
last property, so at runtime they replace every earlier `children` key.
`lower_jsx_call` read every `children` key as a JSX child and the compiled
component rendered `["x", "y"]`. The same happened for two `children`
attributes, for an inlined `{...{ children }}` spread, and a `children`
attribute before a spread lost to the spread's own `children`.
Only the last property is read as the JSX children now. An earlier
`children` key stays an attribute in its position.
The parser also inlines `<i {...{ get g() {} }} />` into the props object.
The lowering passed the getter function itself as the prop. A function
with an accessor in its JSX props is now left uncompiled, like a function
with an accessor in an object literal.
|
Updated 5:57 PM PT - Sep 11th, 2026
❌ @robobun, your commit 3182142 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 42393That installs a local version of the PR into your bun-42393 --bun |
|
Status Reproduced on release 1.4.2 and on main 4b5862f: with
PR: #42393 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. WalkthroughJSX prop lowering now treats only the final ChangesJSX prop lowering
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No actionable merge risk remains from these JSX lowering changes. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — focused fix with strong variant coverage.
What was reviewed:
index == last_indexgate:saturating_sub(1)is fine on empty props (loop doesn't run); the<div {...p} children="x" />last-attr case is behaviorally identical since codegen prints children last.record_accessor_propmirrorslower_object_method'sTodobail-out and is applied at both the automatic and classic (createElement) property loops; plain shorthand methods correctly fall through as function-valued attributes.- Test: 8 components exercise attr/spread/child ordering and the getter bail-out, asserting both runtime
propsand per-function memo-cache presence;body("Getter")'sslice(start, -1)tail is harmless for thetoContaincheck.
Extended reasoning...
Overview
The PR changes lower_jsx_call in src/react_compiler/lowering/build_hir/jsx.rs so that only the last property of the parser-flattened jsx(tag, props) object is treated as the element's JSX children, and adds record_accessor_prop to bail out (record a Todo CompilerError) when a get/set property reaches the JSX props loop via an inlined spread. The accessor guard is applied in both the automatic-runtime and classic createElement property loops. A new itBundled("react-compiler/ChildrenAttributeIsNotAJsxChild", ...) test in test/bundler/transpiler/react-compiler.test.ts covers eight component shapes and asserts both runtime output and per-component compilation status.
Security risks
None. This is a pure transpiler/HIR-lowering change with no I/O, no untrusted-input parsing beyond what the AST already carries, and no auth/crypto/permissions surface. The only new failure mode is recording a Todo compiler error, which leaves the enclosing function uncompiled — the established graceful-degradation path already used by lower_object_method.
Level of scrutiny
Medium. React Compiler lowering affects generated output correctness, but the change is small (~25 lines of Rust) and rests on a well-stated invariant: Bun's visit pass always appends JSX children as the last property of the props object. The one edge where the heuristic over-matches (<div {...p} children="x" />, last property is an attribute not a JSX child) is called out in the PR and is behaviorally a no-op because codegen re-emits children as the final key anyway. The classic-runtime loop correctly does not need the children/last-index gate since children arrive as args[2..] there, not as an object key.
Other factors
The test follows repo conventions: added to the existing react-compiler.test.ts, uses itBundled with the shared stubReact fixture, and asserts the strongest invariant available (exact JSON stdout plus onAfterBundle checking which functions got a $[n] memo cache and that Getter retains get g()). The variant matrix — attr+child, two attrs, inline spread, attr-then-spread (both spread values), spread-then-attr, attr+spread+children, and getter — covers ordering/override semantics well beyond the single repro. record_accessor_prop intentionally matches only Get/Set (not IsMethod): a shorthand method inlined into props is just a function-valued attribute and lowers fine through lower_expression_to_temporary. No CODEOWNERS entry covers these paths, no outstanding reviewer objections in the timeline, and the bug hunt exited on dry_streak with no findings.
2 participants
Problem
bun build --react-compiler,<div children="x">y</div>renders["x","y"]. A plain build renders"y". Twochildrenattributes and<div {...{ children: "sp" }}>real</div>merge the same way, and<div children="x" {...p} />ignoresp.children.jsx("div", { children: "x", children: "y" })before the compiler runs.lower_jsx_call(src/react_compiler/lowering/build_hir/jsx.rs:296) read everychildrenkey of that object as a JSX child.<i {...{ get g() { return p.a } }} />passes the getter function as the propg. The parser inlines the spread into the props object, and the lowering ignoredprop.kind.Fix
childrenkey stays an attribute in its position, so the printed keys keep their source order.Todoerror, so the function is left uncompiled.lower_object_methoddoes the same for object literals.react-compiler/ChildrenAttributeIsNotAJsxChildintest/bundler/transpiler/react-compiler.test.ts(8 components, fails on 1.4.2). Also all ofreact-compiler.test.tsandreact-compiler-fixtures.test.ts.Background
jsx(tag, props, key)calls. The compiler runs after it and decodes each call back into aJsxExpressioninstruction withpropsandchildren.childrenkey.Notes
Results of the test components, plain build and compiled build after this change (stub
jsxruntime that returns{t, p}):props.children<div children="x">y</div>"y"<div children={p.a}>{p.b}</div>with{a: 1, b: "x"}"x"<div children="a" children="b" />"b"<div {...{ children: "sp" }}>real</div>"real"<div children="x" {...p} />with{children: "q"}and{}"q","x"<div children="x" {...p}>{p.a}{p.b}</div>[1, 2]<div {...p} children="x" />"x"<i {...{ get g() { return p.a } }} />with{a: 5}props.g === 5Before:
["x","y"],[1,"x"],["a","b"],["sp","real"],"x"/"x",["x",1,2],"x", andprops.gwas a function.A
childrenattribute that is the last property and not a JSX child (<div {...p} children="x" />) is still read as a child. The output is the same object, because codegen prints the children last.The upstream compiler does not have this problem: it sees the JSX element itself and prints it back, and Babel's JSX transform then builds the same duplicate-key object as a plain build.
#42389 rewrites how
lower_jsx_calltells thejsxcall shape from thecreateElementone. It does not touch the two property loops this PR changes.