support jsx transform with fragment (#835)

* support jsx transform with fragment * fix children * fix test * wrap in array * add changelog entry --------- Co-authored-by: Antonio Nuno Monteiro <[email protected]>
reasonml · Jun 4, 2024 · fc156dd · fc156dd
1 parent 992096b
commit fc156dd
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 44 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,3 +1,8 @@
+# Unreleased
+
+* Support JSX transform with fragments (@tatchi in
+  [#835](https://github.com/reasonml/reason-react/pull/835))
+
 # 0.14.0
 
 * Wrap the `React` library, exposing just a single top-level module

diff --git a/ppx/reason_react_ppx.ml b/ppx/reason_react_ppx.ml
@@ -17,6 +17,11 @@ module Builder = struct
   let value_binding ~loc ~attrs ~pat ~expr =
     let vb = Ast_builder.Default.value_binding ~loc ~pat ~expr in
     { vb with pvb_attributes = attrs }
+
+  let unit =
+    pexp_construct ~loc:Location.none
+      { txt = Lident "()"; loc = Location.none }
+      None
 end
 
 (* [merlinHide] tells merlin to not look at a node, or at any of its
@@ -44,6 +49,14 @@ let optional str = Optional str
 module Binding = struct
   (* Binding is the interface that the ppx uses to interact with the bindings.
      Here we define the same APIs as the bindings but it generates Parsetree *)
+  module ReactDOM = struct
+    let domProps ~applyLoc ~loc props =
+      Builder.pexp_apply ~loc:applyLoc
+        (Builder.pexp_ident ~loc:applyLoc ~attrs:merlinHideAttrs
+           { loc; txt = Ldot (Lident "ReactDOM", "domProps") })
+        props
+  end
+
   module React = struct
     let null ~loc =
       Builder.pexp_ident ~loc { loc; txt = Ldot (Lident "React", "null") }
@@ -59,23 +72,19 @@ module Binding = struct
         ( { loc; txt = Ldot (Lident "React", "componentLike") },
           [ props; return ] )
 
-    let jsxFragment ~loc =
-      Builder.pexp_ident ~loc
-        { loc; txt = Ldot (Lident "React", "jsxFragment") }
-  end
-
-  module ReactDOM = struct
-    let createElement ~loc ~attrs element children =
+    let jsxFragment ~loc ~attrs children =
+      let fragment =
+        Builder.pexp_ident ~loc
+          { loc; txt = Ldot (Lident "React", "jsxFragment") }
+      in
       Builder.pexp_apply ~loc ~attrs
-        (Builder.pexp_ident ~loc
-           { loc; txt = Ldot (Lident "ReactDOM", "createElement") })
-        [ (nolabel, element); (nolabel, children) ]
-
-    let domProps ~applyLoc ~loc props =
-      Builder.pexp_apply ~loc:applyLoc
-        (Builder.pexp_ident ~loc:applyLoc ~attrs:merlinHideAttrs
-           { loc; txt = Ldot (Lident "ReactDOM", "domProps") })
-        props
+        (Builder.pexp_ident ~loc { loc; txt = Ldot (Lident "React", "jsx") })
+        [
+          (nolabel, fragment);
+          ( nolabel,
+            ReactDOM.domProps ~applyLoc:loc ~loc
+              [ (labelled "children", children); (nolabel, Builder.unit) ] );
+        ]
   end
 end
 
@@ -497,11 +506,7 @@ let makeExternalDecl fnName loc namedArgListWithKeyAndRef namedTypeList =
 
 (* TODO: some line number might still be wrong *)
 let jsxMapper =
-  let unit =
-    Builder.pexp_construct ~loc:Location.none
-      { txt = Lident "()"; loc = Location.none }
-      None
-  and key_var_txt = "Key" in
+  let key_var_txt = "Key" in
   let transformUppercaseCall3 ~caller ~ctxt modulePath mapper parentExpLoc attrs
       callArguments =
     let children, argsWithLabels = extractChildren callArguments in
@@ -577,7 +582,7 @@ let jsxMapper =
                    { txt = Lident key_var_txt; loc = parentExpLoc } );
                component;
                props;
-               (nolabel, unit);
+               (nolabel, Builder.unit);
              ])
   in
 
@@ -628,7 +633,7 @@ let jsxMapper =
                (label, Builder.pexp_ident ~loc { txt = Lident key_var_txt; loc });
                component;
                props;
-               (nolabel, unit);
+               (nolabel, Builder.unit);
              ])
     | None -> Builder.pexp_apply ~loc ~attrs jsxExpr [ component; props ]
   in
@@ -1410,10 +1415,9 @@ let jsxMapper =
               let childrenExpr =
                 transformChildrenIfList ~loc ~ctxt ~mapper:self listItems
               in
-              let fragment = Binding.React.jsxFragment ~loc in
               (* throw away the [@JSX] attribute and keep the others, if any *)
-              Binding.ReactDOM.createElement ~loc ~attrs:nonJSXAttributes
-                fragment childrenExpr)
+              Binding.React.jsxFragment ~loc ~attrs:nonJSXAttributes
+                (Binding.React.array ~loc childrenExpr))
       (* Delegate to the default mapper, a deep identity traversal *)
       | e -> super#expression ctxt e
     [@@raises Invalid_argument]

diff --git a/ppx/test/component.t/run.t b/ppx/test/component.t/run.t
@@ -27,13 +27,17 @@ We need to output ML syntax here, otherwise refmt could not parse it.
           ""[@@mel.obj ]
       let make =
         ((fun ?(name= "") ->
-            ReactDOM.createElement React.jsxFragment
-              [|(ReactDOM.jsx "div"
-                   (((ReactDOM.domProps)[@merlin.hide ])
-                      ~children:(React.string ("First " ^ name)) ()));(
-                React.jsx Hello.make
-                  (Hello.makeProps ~children:(React.string ("2nd " ^ name))
-                     ~one:"1" ()))|])
+            React.jsx React.jsxFragment
+              (((ReactDOM.domProps)[@merlin.hide ])
+                 ~children:(React.array
+                              [|(ReactDOM.jsx "div"
+                                   (((ReactDOM.domProps)[@merlin.hide ])
+                                      ~children:(React.string ("First " ^ name))
+                                      ()));(React.jsx Hello.make
+                                              (Hello.makeProps
+                                                 ~children:(React.string
+                                                              ("2nd " ^ name))
+                                                 ~one:"1" ()))|]) ()))
         [@warning "-16"])
       let make =
         let Output$Upper_case_with_fragment_as_root

diff --git a/ppx/test/fragment.t/input.re b/ppx/test/fragment.t/input.re
@@ -1,5 +1,6 @@
 let fragment = foo => [@bla] <> foo </>;
 
+let just_one_child = foo => <> bar </>;
 let poly_children_fragment = (foo, bar) => <> foo bar </>;
 let nested_fragment = (foo, bar, baz) => <> foo <> bar baz </> </>;
 

diff --git a/ppx/test/fragment.t/run.t b/ppx/test/fragment.t/run.t
@@ -1,20 +1,52 @@
   $ ../ppx.sh --output re input.re
   let fragment = foo =>
-    [@bla] ReactDOM.createElement(React.jsxFragment, [|foo|]);
+    [@bla]
+    React.jsx(
+      React.jsxFragment,
+      ([@merlin.hide] ReactDOM.domProps)(~children=React.array([|foo|]), ()),
+    );
+  let just_one_child = foo =>
+    React.jsx(
+      React.jsxFragment,
+      ([@merlin.hide] ReactDOM.domProps)(~children=React.array([|bar|]), ()),
+    );
   let poly_children_fragment = (foo, bar) =>
-    ReactDOM.createElement(React.jsxFragment, [|foo, bar|]);
+    React.jsx(
+      React.jsxFragment,
+      ([@merlin.hide] ReactDOM.domProps)(
+        ~children=React.array([|foo, bar|]),
+        (),
+      ),
+    );
   let nested_fragment = (foo, bar, baz) =>
-    ReactDOM.createElement(
+    React.jsx(
       React.jsxFragment,
-      [|foo, ReactDOM.createElement(React.jsxFragment, [|bar, baz|])|],
+      ([@merlin.hide] ReactDOM.domProps)(
+        ~children=
+          React.array([|
+            foo,
+            React.jsx(
+              React.jsxFragment,
+              ([@merlin.hide] ReactDOM.domProps)(
+                ~children=React.array([|bar, baz|]),
+                (),
+              ),
+            ),
+          |]),
+        (),
+      ),
     );
   let nested_fragment_with_lower = foo =>
-    ReactDOM.createElement(
+    React.jsx(
       React.jsxFragment,
-      [|
-        ReactDOM.jsx(
-          "div",
-          ([@merlin.hide] ReactDOM.domProps)(~children=foo, ()),
-        ),
-      |],
+      ([@merlin.hide] ReactDOM.domProps)(
+        ~children=
+          React.array([|
+            ReactDOM.jsx(
+              "div",
+              ([@merlin.hide] ReactDOM.domProps)(~children=foo, ()),
+            ),
+          |]),
+        (),
+      ),
     );