Repository navigation
Evaluate JSX children after the other props in plain mode - #9117
Conversation
Plain mode built the props record in the declaration order of its type, so JsxDOM.domProps evaluated children before most attributes, while preserve mode evaluates them last, as JSX writes them (#9104). Plain mode now builds the same Pjsx element as preserve mode and prints it as the call; the tag is still flattened outside preserve mode. JS modified (plain JSX mode): Example: let note = (s: string) => s->String.toUpperCase let element = <div title={note("t")} key={note("k")}> {React.string(note("c"))} </div> Before (js): let element = JsxRuntime.jsx("div", { children: "c".toUpperCase(), title: "t".toUpperCase() }, "k".toUpperCase()); After (js): let element = JsxRuntime.jsx("div", { title: "t".toUpperCase(), children: "c".toUpperCase() }, "k".toUpperCase()); Review: Why keep a let-bound tag when the element has no other argument? Inlining a function literal into the tag drops the name the let gives it, which React shows; the old call always had a props argument. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel <[email protected]>
Changelog🐛 Bug fix
Semantic Diff1 commit · Full page Evaluate JSX children after the other props in both JSX modesfea64ff · cknitt + Claude · Oct 28, 2026, 15:48 UTC Plain mode built the props record in the declaration order of its type, so JsxDOM.domProps evaluated children before most attributes, while preserve mode evaluates them last, as JSX writes them (#9104). Plain mode now builds the same Pjsx element as preserve mode and prints it as the call; the tag is still flattened outside preserve mode. JS modified (plain JSX mode) let note = (s: string) => s->String.toUpperCase
let element = <div title={note("t")} key={note("k")}> {React.string(note("c"))} </div>
let element = JsxRuntime.jsx("div", {
- children: "c".toUpperCase(),
- title: "t".toUpperCase()
+ title: "t".toUpperCase(),
+ children: "c".toUpperCase()
}, "k".toUpperCase());Review
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9117 +/- ##
=======================================
Coverage 81.46% 81.46%
=======================================
Files 460 460
Lines 64675 64678 +3
=======================================
+ Hits 52689 52693 +4
+ Misses 11986 11985 -1
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9117 |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
cristianoc
left a comment
cristianoc
left a comment
There was a problem hiding this comment.
Reviewed the shared JSX lowering, binding preservation and evaluation-order regression tests. No blocking issues found. CI, including jsx_preserve_semantics, passes; not rerun locally.
Plain and preserve JSX mode evaluated the parts of an element in different orders. Plain mode built the props record in the declaration order of its type, and
JsxDOM.domPropsdeclareschildrensecond, so<div title={t()}> {c()} </div>evaluatedcbeforet; preserve mode emits JSX, where the children come after the attributes and are evaluated last. The same held for any component whose props type declareschildrenbefore other fields.Plain mode now goes through the same
Pjsxelement as preserve mode (which already put the children after the other props) and prints it as the call, so both modes evaluate tag, spread, props, children and key in the same order by construction. In plain mode the object literal now listschildrenlast. Two things that were only right for preserve mode are now limited to it: keeping a tag likeM.makeunflattened (the passes inlam_pass_remove_aliasandjs_pass_flatten_and_mark_dead). AndLam_util.refine_letno longer substitutes a let-bound tag into an element whose only argument is the tag, which would inline a function literal and drop the namelet make = …gives it (React shows that name); the old call always had a props argument, so it never hit that rule.Tests:
tests/build_tests/jsx_preserve_semantics/cases/Order.reslogs children against an attribute and the key for a DOM element, for a component whose props type declareschildrenfirst, and for several children (jsxs). The suite compares plain mode with preserve mode compiled by TypeScript, and fails on master.tests/tests/src/jsx_children_order_test.resshows the plain-mode output withchildrenlast.tests/gentype_tests/typescript-react-example/src/Hooks.res.jschanges only in the position ofchildrenin props objects.Evaluation order: plain mode now follows preserve mode / JSX source order (attributes, then children, then the key), rather than preserve mode following the props type's declaration order, which JSX can only express by writing children as a
children={…}attribute. Attributes among themselves are still evaluated in the props type's declaration order in both modes, as before.Fixes #9104
🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP