Evaluate JSX children after the other props in plain mode

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.

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

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

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

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

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 = {

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) =

tests/build_tests/jsx_preserve_semantics/cases/Order.res

@@ -43,3 +43,22 @@ let dictBase: dict<string> = dict{}
 let dictChildren =
   <LoggedDict {...dict{...dictBase, "children": note("children"), "title": note("title")}} />
 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 = <div title={note("title")} key={note("key")}>{React.string(note("child"))}</div>
+let domChildrenOrder = log->Array.copy
+
+type childrenFirstProps = {children: string, a: string}
+module ChildrenFirst = {
+  @val external make: React.component<childrenFirstProps> = "LoggedComp"
+}
+let componentChildren = <ChildrenFirst a={note("a")}>{note("child")}</ChildrenFirst>
+let componentChildrenOrder = log->Array.copy
+
+let domManyChildren =
+  <div title={note("title")}>
+    {React.string(note("first"))}
+    {React.string(note("second"))}
+  </div>
+let domManyChildrenOrder = log->Array.copy

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 */

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 = <div title={note("t")} key={note("k")}>{React.string(note("c"))}</div>
+
+let elements =
+  <div title={note("t")}>
+    {React.string(note("c1"))}
+    {React.string(note("c2"))}
+  </div>
+
+type props = {children: string, a: string}
+module ChildrenFirst = {
+  @val external make: React.component<props> = "ChildrenFirst"
+}
+let component = <ChildrenFirst a={note("a")}>{note("c")}</ChildrenFirst>