From fea64ff7afc19ec85784dff123f57f80750835f8 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 28 Oct 2026 15:48:52 +0000 Subject: [PATCH] Evaluate JSX children after the other props in both JSX modes 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 =
{React.string(note("c"))}
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 Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel --- changes/jsx-children-evaluation-order.fix.md | 1 + compiler/core/j.ml | 2 +- .../core/js_pass_flatten_and_mark_dead.ml | 9 ++--- compiler/core/lam_pass_remove_alias.ml | 8 +++-- compiler/core/lam_util.ml | 8 +++-- compiler/ml/lambda.ml | 3 +- compiler/ml/translcore.ml | 14 ++++---- .../jsx_preserve_semantics/cases/Order.res | 19 ++++++++++ .../typescript-react-example/src/Hooks.res.js | 20 +++++------ tests/tests/src/jsx_children_order_test.mjs | 36 +++++++++++++++++++ tests/tests/src/jsx_children_order_test.res | 19 ++++++++++ 11 files changed, 109 insertions(+), 30 deletions(-) create mode 100644 changes/jsx-children-evaluation-order.fix.md create mode 100644 tests/tests/src/jsx_children_order_test.mjs create mode 100644 tests/tests/src/jsx_children_order_test.res diff --git a/changes/jsx-children-evaluation-order.fix.md b/changes/jsx-children-evaluation-order.fix.md new file mode 100644 index 00000000000..dd505366848 --- /dev/null +++ b/changes/jsx-children-evaluation-order.fix.md @@ -0,0 +1 @@ +- Evaluate JSX children after the other props in plain JSX mode, as in preserve mode. diff --git a/compiler/core/j.ml b/compiler/core/j.ml index 23b1d673c07..dc90f245b18 100644 --- a/compiler/core/j.ml +++ b/compiler/core/j.ml @@ -117,7 +117,7 @@ and expression_desc = multi: bool; fragment: bool; } - (** A JSX element in JSX preserve mode, from [Lambda.Pjsx]. js_dump + (** A JSX element, from [Lambda.Pjsx]. In JSX preserve mode, js_dump prints it as JSX when every part can be written in JSX, and otherwise as the call [callee(tag, {...spread, props, children}, key)]. [props] holds diff --git a/compiler/core/js_pass_flatten_and_mark_dead.ml b/compiler/core/js_pass_flatten_and_mark_dead.ml index fcd7a0a698a..b647e20ac62 100644 --- a/compiler/core/js_pass_flatten_and_mark_dead.ml +++ b/compiler/core/js_pass_flatten_and_mark_dead.ml @@ -277,10 +277,11 @@ let subst_map (substitution : J.expression Hash_ident.t) = | Some _ | None -> super.expression self x) | Jsx ({tag = {expression_desc = Static_index _ | Array_index _}} as jsx) - -> - (* Keep a JSX tag like [M.make] as it is: the field's value may be a - variable with a lowercase name, which JSX reads as an intrinsic - element, so js_dump would have to print the call instead (#7432) *) + when !((Clflags.current ()).jsx_preserve) -> + (* In JSX preserve mode, keep a JSX tag like [M.make] as it is: the + field's value may be a variable with a lowercase name, which JSX + reads as an intrinsic element, so js_dump would have to print the + call instead (#7432) *) let map = self.expression self in { x with diff --git a/compiler/core/lam_pass_remove_alias.ml b/compiler/core/lam_pass_remove_alias.ml index 7f0cfb1e21e..5d75ce64fb7 100644 --- a/compiler/core/lam_pass_remove_alias.ml +++ b/compiler/core/lam_pass_remove_alias.ml @@ -57,14 +57,16 @@ let simplify_alias (meta : Lam_stats.t) (lam : Lambda.t) : Lambda.t = let inline_depth = ref 0 in let rec simpl (lam : Lambda.t) : Lambda.t = match lam with - (* Keep a JSX tag like [M.make] as it is: flattening it may give a variable - with a lowercase name, which JSX reads as an intrinsic element (#7432) *) + (* In JSX preserve mode, keep a JSX tag like [M.make] as it is: flattening + it may give a variable with a lowercase name, which JSX reads as an + intrinsic element (#7432) *) | Lprim { primitive = Pjsx _ as primitive; args = (Lprim {primitive = Pfield (_, _)} as tag) :: rest; loc; - } -> + } + when !((Clflags.current ()).jsx_preserve) -> let rest' = Ext_list.map_sharing rest simpl in if rest' == rest then lam else Lambda.prim ~primitive ~args:(tag :: rest') loc diff --git a/compiler/core/lam_util.ml b/compiler/core/lam_util.ml index b246c3ce3e1..0bab8c65e4b 100644 --- a/compiler/core/lam_util.ml +++ b/compiler/core/lam_util.ml @@ -53,8 +53,10 @@ nothing is refined it is handed back untouched rather than rebuilt. *) let refine_let ?original ~kind param (arg : Lambda.t) (l : Lambda.t) : Lambda.t = - let is_block_constructor = function - | Lambda.Pmakeblock _ -> true + (* A JSX element whose only argument is its tag keeps the tag bound: a + function bound by [let] gets the binding's name, which React shows *) + let keeps_binding = function + | Lambda.Pmakeblock _ | Pjsx _ -> true | _ -> false in (* SafeAlias is the predicate that justifies the (Alias) rewrite @@ -99,7 +101,7 @@ let refine_let ?original ~kind param (arg : Lambda.t) (l : Lambda.t) : Lambda.t cannot refer back to `x`, and we avoid generating a redundant local. *) arg | _, _, Lprim {primitive; args = [Lvar w]; loc; _} - when Ident.same w param && not (is_block_constructor primitive) -> + when Ident.same w param && not (keeps_binding primitive) -> (* When we immediately feed the binding into a primitive, like `{ let x = value; Array.length(x) }`, we inline the primitive call with `value`. This only happens for primitives that are pure and do not diff --git a/compiler/ml/lambda.ml b/compiler/ml/lambda.ml index e8e6ec91b69..fa3c3a9c268 100644 --- a/compiler/ml/lambda.ml +++ b/compiler/ml/lambda.ml @@ -119,8 +119,7 @@ type import_source = rather than primitives. See [builtin]. *) type eliminated = Identity | Ignore -(* A JSX element in JSX preserve mode, built by [Translcore] from a call the - JSX PPX produced. The arguments of [Pjsx] are, in evaluation order: the tag, +(* A JSX element, built by [Translcore] from a call the JSX PPX produced. The arguments of [Pjsx] are, in evaluation order: the tag, the spread props (if [jsx_spread]), one value per [jsx_props] entry, the children ([jsx_children] of them) and the key (if [jsx_key]). *) type jsx_element = { diff --git a/compiler/ml/translcore.ml b/compiler/ml/translcore.ml index 5e5b0ae9bed..876fc390a24 100644 --- a/compiler/ml/translcore.ml +++ b/compiler/ml/translcore.ml @@ -949,8 +949,9 @@ let extract_directive_for_fn exp = if txt = "directive" then Ast_payload.semantic_string_of_payload payload else None) -(* JSX preserve mode: a call of a JSX external as the JSX PPX produces it, - which [transl_jsx_element] turns into [Pjsx]. The JSX module's names +(* A call of a JSX external as the JSX PPX produces it, which + [transl_jsx_element] turns into [Pjsx] in both JSX modes, so that they + evaluate the parts of an element in the same order. The JSX module's names ([jsx], [jsxs], [jsxKeyed], [jsxsKeyed], [jsxFragment]) are fixed by the PPX, unlike the JS names of the externals. *) type jsx_props = @@ -1206,8 +1207,7 @@ and transl_exp0 (e : Typedtree.expression) : Lambda.t = funct; args; transformed_jsx = true; - } as apply) - when !((Clflags.current ()).jsx_preserve) -> ( + } as apply) -> ( match jsx_call e.exp_env funct p ~val_type args with | Some call -> check_callee_attributes funct; @@ -1675,9 +1675,9 @@ and transl_let rec_flag pat_expr_list body = in Lambda_scc.bind_rec (Ext_list.map pat_expr_list transl_case) body -(* JSX preserve mode: see [jsx_call]. The arguments of [Pjsx] are in the - order the call evaluates them, except that the children come after the - other props, as in JSX. *) +(* See [jsx_call]. The arguments of [Pjsx] are in the order the call + evaluates them, except that the children come after the other props, as in + JSX, whatever the order of the fields in the props type (#9104). *) and transl_jsx_element loc ({jsx_name; jsx_ffi; jsx_multi; jsx_fragment; jsx_tag; jsx_props; jsx_key} : jsx_call) = diff --git a/tests/build_tests/jsx_preserve_semantics/cases/Order.res b/tests/build_tests/jsx_preserve_semantics/cases/Order.res index 44d16e45f71..8cb826f469b 100644 --- a/tests/build_tests/jsx_preserve_semantics/cases/Order.res +++ b/tests/build_tests/jsx_preserve_semantics/cases/Order.res @@ -43,3 +43,22 @@ let dictBase: dict = dict{} let dictChildren = let dictChildrenOrder = log->Array.copy + +// Children are evaluated after the other props, as JSX writes them, even when +// the props type declares children first, and the key last +let domChildren =
{React.string(note("child"))}
+let domChildrenOrder = log->Array.copy + +type childrenFirstProps = {children: string, a: string} +module ChildrenFirst = { + @val external make: React.component = "LoggedComp" +} +let componentChildren = {note("child")} +let componentChildrenOrder = log->Array.copy + +let domManyChildren = +
+ {React.string(note("first"))} + {React.string(note("second"))} +
+let domManyChildrenOrder = log->Array.copy diff --git a/tests/gentype_tests/typescript-react-example/src/Hooks.res.js b/tests/gentype_tests/typescript-react-example/src/Hooks.res.js index 2b25ff60b42..123ffd12471 100644 --- a/tests/gentype_tests/typescript-react-example/src/Hooks.res.js +++ b/tests/gentype_tests/typescript-react-example/src/Hooks.res.js @@ -16,30 +16,30 @@ function Hooks(props) { children: "Hooks example " + (props.vehicle.name + (" clicked " + (String(count) + " times"))) }), JsxRuntime.jsx("button", { - children: "Click me", - onClick: param => setCount(param => count + 1 | 0) + onClick: param => setCount(param => count + 1 | 0), + children: "Click me" }), JsxRuntime.jsxs(ImportHooks.make, { person: { name: "Mary", age: 71 }, + renderMe: x => x.randomString, children: [ "child1", "child2" - ], - renderMe: x => x.randomString + ] }), JsxRuntime.jsxs(ImportHookDefault.make, { person: { name: "DefaultImport", age: 42 }, + renderMe: x => x.randomString, children: [ "child1", "child2" - ], - renderMe: x => x.randomString + ] }) ] }); @@ -129,8 +129,8 @@ let make = React.forwardRef((props, ref) => { return null; } else { return JsxRuntime.jsx("button", { - children: props.vehicle.name, - ref: Primitive_option.some(ref) + ref: Primitive_option.some(ref), + children: props.vehicle.name }); } }); @@ -140,8 +140,8 @@ let WithRef = { }; let input = React.forwardRef((r, ref) => JsxRuntime.jsx("div", { - children: r.x, - ref: Primitive_option.some(ref) + ref: Primitive_option.some(ref), + children: r.x })); let ForwardRef = { diff --git a/tests/tests/src/jsx_children_order_test.mjs b/tests/tests/src/jsx_children_order_test.mjs new file mode 100644 index 00000000000..9c1df967b91 --- /dev/null +++ b/tests/tests/src/jsx_children_order_test.mjs @@ -0,0 +1,36 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as JsxRuntime from "react/jsx-runtime"; + +function note(s) { + return s.toUpperCase(); +} + +let element = JsxRuntime.jsx("div", { + title: "t".toUpperCase(), + children: "c".toUpperCase() +}, "k".toUpperCase()); + +let elements = JsxRuntime.jsxs("div", { + title: "t".toUpperCase(), + children: [ + "c1".toUpperCase(), + "c2".toUpperCase() + ] +}); + +let ChildrenFirst = {}; + +let component = JsxRuntime.jsx(globalThis.ChildrenFirst, { + a: "a".toUpperCase(), + children: "c".toUpperCase() +}); + +export { + note, + element, + elements, + ChildrenFirst, + component, +} +/* element Not a pure module */ diff --git a/tests/tests/src/jsx_children_order_test.res b/tests/tests/src/jsx_children_order_test.res new file mode 100644 index 00000000000..0a72a0d1651 --- /dev/null +++ b/tests/tests/src/jsx_children_order_test.res @@ -0,0 +1,19 @@ +@@config({flags: ["-bs-jsx", "4"]}) + +// Children are evaluated after the other props, as JSX writes them, although +// JsxDOM.domProps declares children before most attributes (#9104) +let note = (s: string) => s->String.toUpperCase + +let element =
{React.string(note("c"))}
+ +let elements = +
+ {React.string(note("c1"))} + {React.string(note("c2"))} +
+ +type props = {children: string, a: string} +module ChildrenFirst = { + @val external make: React.component = "ChildrenFirst" +} +let component = {note("c")}