diff --git a/changes/jsx-children-evaluation-order.fix.md b/changes/jsx-children-evaluation-order.fix.md new file mode 100644 index 0000000000..dd50536684 --- /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 23b1d673c0..dc90f245b1 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 fcd7a0a698..b647e20ac6 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 7f0cfb1e21..5d75ce64fb 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 b246c3ce3e..0bab8c65e4 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 e8e6ec91b6..fa3c3a9c26 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 5e5b0ae9be..876fc390a2 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 44d16e45f7..8cb826f469 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 2b25ff60b4..123ffd1247 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 0000000000..9c1df967b9 --- /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 0000000000..0a72a0d165 --- /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")}