React fragment's children type change and JSX4 adaptation (#6238)

* add tests

* fix JSX to adopt to change of React.fragment type

* artifacts

* Revert "artifacts"

This reverts commit 649a0f7.

* stale comment

Author: mununki

CHANGELOG.md

@@ -21,6 +21,7 @@
 - Remove unnecessary require and import statements when using dynamic imports. https://github.com/rescript-lang/rescript-compiler/pull/6232
 - Fix option unboxing logic in the presence of untagged variants. https://github.com/rescript-lang/rescript-compiler/pull/6233
 - Fix printing of local module with type. https://github.com/rescript-lang/rescript-compiler/issues/6212
+- Adapting JSX4 to React.fragment's children type change (`'children` -> `React.element`) https://github.com/rescript-lang/rescript-compiler/pull/6238
 
 #### :nail_care: Polish
 

docs/JSXV4.md

@@ -358,7 +358,7 @@ function has the name of the enclosing module/file.
 // is transformed to
 
 // v4
-ReactDOMRe.createElement(ReasonReact.fragment, [comp1, comp2, comp3])
+React.createElement(React.fragment, {children: [comp1, comp2, comp3]})
 
 // v4 @ new jsx transform
 React.jsxs(React.jsxFragment, {children: [comp1, comp2, comp3]})

jscomp/syntax/src/reactjs_jsx_v4.ml

@@ -1360,26 +1360,40 @@ let expr ~config mapper expression =
       let recordOfChildren children =
         Exp.record [(Location.mknoloc (Lident "children"), children)] None
       in
-      let args =
-        [
-          (nolabel, fragment);
-          (match config.mode with
-          | "automatic" -> (
-            ( nolabel,
-              match childrenExpr with
-              | {pexp_desc = Pexp_array children} -> (
-                match children with
-                | [] -> emptyRecord ~loc:Location.none
-                | [child] -> recordOfChildren child
-                | _ -> recordOfChildren childrenExpr)
-              | _ -> recordOfChildren childrenExpr ))
-          | "classic" | _ -> (nolabel, childrenExpr));
-        ]
+      let applyReactArray expr =
+        Exp.apply
+          (Exp.ident
+             {txt = Ldot (Lident "React", "array"); loc = Location.none})
+          [(Nolabel, expr)]
       in
       let countOfChildren = function
         | {pexp_desc = Pexp_array children} -> List.length children
         | _ -> 0
       in
+      let transformChildrenToProps childrenExpr =
+        match childrenExpr with
+        | {pexp_desc = Pexp_array children} -> (
+          match children with
+          | [] -> emptyRecord ~loc:Location.none
+          | [child] -> recordOfChildren child
+          | _ -> (
+            match config.mode with
+            | "automatic" -> recordOfChildren @@ applyReactArray childrenExpr
+            | "classic" | _ -> emptyRecord ~loc:Location.none))
+        | _ -> (
+          match config.mode with
+          | "automatic" -> recordOfChildren @@ applyReactArray childrenExpr
+          | "classic" | _ -> emptyRecord ~loc:Location.none)
+      in
+      let args =
+        (nolabel, fragment)
+        :: (nolabel, transformChildrenToProps childrenExpr)
+        ::
+        (match config.mode with
+        | "classic" when countOfChildren childrenExpr > 1 ->
+          [(nolabel, childrenExpr)]
+        | _ -> [])
+      in
       Exp.apply
         ~loc (* throw away the [@JSX] attribute and keep the others, if any *)
         ~attrs:nonJSXAttributes
@@ -1390,7 +1404,11 @@ let expr ~config mapper expression =
             Exp.ident ~loc {loc; txt = Ldot (Lident "React", "jsxs")}
           else Exp.ident ~loc {loc; txt = Ldot (Lident "React", "jsx")}
         | "classic" | _ ->
-          Exp.ident ~loc {loc; txt = Ldot (Lident "ReactDOM", "createElement")})
+          if countOfChildren childrenExpr > 1 then
+            Exp.ident ~loc
+              {loc; txt = Ldot (Lident "React", "createElementVariadic")}
+          else
+            Exp.ident ~loc {loc; txt = Ldot (Lident "React", "createElement")})
         args)
   (* Delegate to the default mapper, a deep identity traversal *)
   | e -> default_mapper.expr mapper e

jscomp/syntax/tests/ppx/react/expected/fragment.res.txt

@@ -1,19 +1,59 @@
 @@jsxConfig({version: 4, mode: "classic"})
 
-let _ = ReactDOM.createElement(React.fragment, [])
-let _ = ReactDOM.createElement(React.fragment, [ReactDOM.createDOMElementVariadic("div", [])])
-let _ = ReactDOM.createElement(React.fragment, [ReactDOM.createElement(React.fragment, [])])
+let _ = React.createElement(React.fragment, {})
+let _ = React.createElement(
+  React.fragment,
+  {children: ReactDOM.createDOMElementVariadic("div", [])},
+)
+let _ = React.createElementVariadic(
+  React.fragment,
+  {},
+  [ReactDOM.createDOMElementVariadic("div", []), ReactDOM.createDOMElementVariadic("div", [])],
+)
+let _ = React.createElement(React.fragment, {children: React.createElement(React.fragment, {})})
 let _ = React.createElement(Z.make, {})
 let _ = React.createElement(Z.make, {children: ReactDOM.createDOMElementVariadic("div", [])})
+let _ = React.createElement(
+  Z.make,
+  {a: "a", children: ReactDOM.createDOMElementVariadic("div", [])},
+)
+let _ = React.createElementVariadic(
+  Z.make,
+  {children: React.null},
+  [ReactDOM.createDOMElementVariadic("div", []), ReactDOM.createDOMElementVariadic("div", [])],
+)
 let _ = ReactDOM.createDOMElementVariadic("div", [])
 let _ = ReactDOM.createDOMElementVariadic("div", [ReactDOM.createDOMElementVariadic("div", [])])
+let _ = ReactDOM.createDOMElementVariadic(
+  "div",
+  ~props={id: "id"},
+  [ReactDOM.createDOMElementVariadic("div", [])],
+)
+let _ = ReactDOM.createDOMElementVariadic(
+  "div",
+  [ReactDOM.createDOMElementVariadic("div", []), ReactDOM.createDOMElementVariadic("div", [])],
+)
 
 @@jsxConfig({version: 4, mode: "automatic"})
 
 let _ = React.jsx(React.jsxFragment, {})
 let _ = React.jsx(React.jsxFragment, {children: ReactDOM.jsx("div", {})})
+let _ = React.jsxs(
+  React.jsxFragment,
+  {children: React.array([ReactDOM.jsx("div", {}), ReactDOM.jsx("div", {})])},
+)
 let _ = React.jsx(React.jsxFragment, {children: React.jsx(React.jsxFragment, {})})
 let _ = React.jsx(Z.make, {})
 let _ = React.jsx(Z.make, {children: ReactDOM.jsx("div", {})})
+let _ = React.jsx(Z.make, {a: "a", children: ReactDOM.jsx("div", {})})
+let _ = React.jsxs(
+  Z.make,
+  {children: React.array([ReactDOM.jsx("div", {}), ReactDOM.jsx("div", {})])},
+)
 let _ = ReactDOM.jsx("div", {})
 let _ = ReactDOM.jsx("div", {children: ?ReactDOM.someElement(ReactDOM.jsx("div", {}))})
+let _ = ReactDOM.jsx("div", {id: "id", children: ?ReactDOM.someElement(ReactDOM.jsx("div", {}))})
+let _ = ReactDOM.jsxs(
+  "div",
+  {children: React.array([ReactDOM.jsx("div", {}), ReactDOM.jsx("div", {})])},
+)

jscomp/syntax/tests/ppx/react/expected/noPropsWithKey.res.txt

@@ -22,8 +22,9 @@ module V4C = {
   type props = {}
 
   let make = (_: props) =>
-    ReactDOM.createElement(
+    React.createElementVariadic(
       React.fragment,
+      {},
       [
         JsxPPXReactSupport.createElementWithKey(~key="k", V4CA.make, {}),
         JsxPPXReactSupport.createElementWithKey(~key="k", V4CB.make, {}),
@@ -63,10 +64,10 @@ module V4C = {
     React.jsxs(
       React.jsxFragment,
       {
-        children: [
+        children: React.array([
           React.jsxKeyed(V4CA.make, {}, ~key="k", ()),
           React.jsxKeyed(V4CB.make, {}, ~key="k", ()),
-        ],
+        ]),
       },
     )
   let make = {

jscomp/syntax/tests/ppx/react/expected/removedKeyProp.res.txt

@@ -14,7 +14,7 @@ module Foo = {
 module HasChildren = {
   type props<'children> = {children: 'children}
 
-  let make = ({children, _}: props<_>) => ReactDOM.createElement(React.fragment, [children])
+  let make = ({children, _}: props<_>) => React.createElement(React.fragment, {children: children})
   let make = {
     let \"RemovedKeyProp$HasChildren" = (props: props<_>) => make(props)
 
@@ -24,8 +24,9 @@ module HasChildren = {
 type props = {}
 
 let make = (_: props) =>
-  ReactDOM.createElement(
+  React.createElementVariadic(
     React.fragment,
+    {},
     [
       JsxPPXReactSupport.createElementWithKey(~key="k", Foo.make, {x: "x", y: "y"}),
       React.createElement(Foo.make, {x: "x", y: "y"}),

jscomp/syntax/tests/ppx/react/fragment.res

@@ -2,18 +2,28 @@
 
 let _ = <></>
 let _ = <><div /></>
+let _ = <><div /><div /></>
 let _ = <><></></>
 let _ = <Z />
 let _ = <Z><div /></Z>
+let _ = <Z a="a"><div /></Z>
+let _ = <Z><div /><div /></Z>
 let _ = <div />
 let _ = <div><div /></div>
+let _ = <div id="id"><div /></div>
+let _ = <div><div /><div /></div>
 
 @@jsxConfig({version:4, mode: "automatic"})
 
 let _ = <></>
 let _ = <><div /></>
+let _ = <><div /><div /></>
 let _ = <><></></>
 let _ = <Z />
 let _ = <Z><div /></Z>
+let _ = <Z a="a"><div /></Z>
+let _ = <Z><div /><div /></Z>
 let _ = <div />
-let _ = <div><div /></div>
+let _ = <div><div /></div>
+let _ = <div id="id"><div /></div>
+let _ = <div><div /><div /></div>