From bd803209c1f5b5abbf55ee809c2c8f40fad45617 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Mon, 3 Jun 2024 17:32:02 +0200 Subject: [PATCH 1/5] support jsx transform with fragment --- ppx/reason_react_ppx.ml | 22 ++++++++++------------ ppx/test/component.t/run.t | 2 +- ppx/test/fragment.t/run.t | 11 +++++------ 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/ppx/reason_react_ppx.ml b/ppx/reason_react_ppx.ml index 0b8f240d5..1decd85eb 100644 --- a/ppx/reason_react_ppx.ml +++ b/ppx/reason_react_ppx.ml @@ -59,18 +59,17 @@ module Binding = struct ( { loc; txt = Ldot (Lident "React", "componentLike") }, [ props; return ] ) - let jsxFragment ~loc = - Builder.pexp_ident ~loc - { loc; txt = Ldot (Lident "React", "jsxFragment") } + 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 "React", "jsx") }) + [ (nolabel, fragment); (nolabel, children) ] end module ReactDOM = struct - let createElement ~loc ~attrs element children = - 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 @@ -1410,10 +1409,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 + 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 index 0e5b91102..3f4f5de2d 100644 --- a/ppx/test/component.t/run.t +++ b/ppx/test/component.t/run.t @@ -27,7 +27,7 @@ We need to output ML syntax here, otherwise refmt could not parse it. ""[@@mel.obj ] let make = ((fun ?(name= "") -> - ReactDOM.createElement React.jsxFragment + React.jsx React.jsxFragment [|(ReactDOM.jsx "div" (((ReactDOM.domProps)[@merlin.hide ]) ~children:(React.string ("First " ^ name)) ()));( diff --git a/ppx/test/fragment.t/run.t b/ppx/test/fragment.t/run.t index 421251412..6a6b99a19 100644 --- a/ppx/test/fragment.t/run.t +++ b/ppx/test/fragment.t/run.t @@ -1,15 +1,14 @@ $ ../ppx.sh --output re input.re - let fragment = foo => - [@bla] ReactDOM.createElement(React.jsxFragment, [|foo|]); + let fragment = foo => [@bla] React.jsx(React.jsxFragment, [|foo|]); let poly_children_fragment = (foo, bar) => - ReactDOM.createElement(React.jsxFragment, [|foo, bar|]); + React.jsx(React.jsxFragment, [|foo, bar|]); let nested_fragment = (foo, bar, baz) => - ReactDOM.createElement( + React.jsx( React.jsxFragment, - [|foo, ReactDOM.createElement(React.jsxFragment, [|bar, baz|])|], + [|foo, React.jsx(React.jsxFragment, [|bar, baz|])|], ); let nested_fragment_with_lower = foo => - ReactDOM.createElement( + React.jsx( React.jsxFragment, [| ReactDOM.jsx( From 6edd9f827f3bbe0dd15270c3c41f9c8b2bd04059 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Mon, 3 Jun 2024 18:35:20 -0700 Subject: [PATCH 2/5] fix children --- ppx/reason_react_ppx.ml | 38 ++++++++++++++++++++++---------------- ppx/test/fragment.t/run.t | 38 +++++++++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/ppx/reason_react_ppx.ml b/ppx/reason_react_ppx.ml index 1decd85eb..5f7004751 100644 --- 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") } @@ -66,15 +79,12 @@ module Binding = struct in Builder.pexp_apply ~loc ~attrs (Builder.pexp_ident ~loc { loc; txt = Ldot (Lident "React", "jsx") }) - [ (nolabel, fragment); (nolabel, children) ] - end - - 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 + [ + (nolabel, fragment); + ( nolabel, + ReactDOM.domProps ~applyLoc:loc ~loc + [ (labelled "children", children); (nolabel, Builder.unit) ] ); + ] end end @@ -496,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 @@ -576,7 +582,7 @@ let jsxMapper = { txt = Lident key_var_txt; loc = parentExpLoc } ); component; props; - (nolabel, unit); + (nolabel, Builder.unit); ]) in @@ -627,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 diff --git a/ppx/test/fragment.t/run.t b/ppx/test/fragment.t/run.t index 6a6b99a19..e46b7c0e5 100644 --- a/ppx/test/fragment.t/run.t +++ b/ppx/test/fragment.t/run.t @@ -1,19 +1,39 @@ $ ../ppx.sh --output re input.re - let fragment = foo => [@bla] React.jsx(React.jsxFragment, [|foo|]); + let fragment = foo => + [@bla] + React.jsx( + React.jsxFragment, + ([@merlin.hide] ReactDOM.domProps)(~children=[|foo|], ()), + ); let poly_children_fragment = (foo, bar) => - React.jsx(React.jsxFragment, [|foo, bar|]); + React.jsx( + React.jsxFragment, + ([@merlin.hide] ReactDOM.domProps)(~children=[|foo, bar|], ()), + ); let nested_fragment = (foo, bar, baz) => React.jsx( React.jsxFragment, - [|foo, React.jsx(React.jsxFragment, [|bar, baz|])|], + ([@merlin.hide] ReactDOM.domProps)( + ~children=[| + foo, + React.jsx( + React.jsxFragment, + ([@merlin.hide] ReactDOM.domProps)(~children=[|bar, baz|], ()), + ), + |], + (), + ), ); let nested_fragment_with_lower = foo => React.jsx( React.jsxFragment, - [| - ReactDOM.jsx( - "div", - ([@merlin.hide] ReactDOM.domProps)(~children=foo, ()), - ), - |], + ([@merlin.hide] ReactDOM.domProps)( + ~children=[| + ReactDOM.jsx( + "div", + ([@merlin.hide] ReactDOM.domProps)(~children=foo, ()), + ), + |], + (), + ), ); From 3a3c078d3f815404b6ae9cce9bc2ae8036e42193 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Mon, 3 Jun 2024 18:36:04 -0700 Subject: [PATCH 3/5] fix test --- ppx/test/component.t/run.t | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ppx/test/component.t/run.t b/ppx/test/component.t/run.t index 3f4f5de2d..ce2af42a7 100644 --- a/ppx/test/component.t/run.t +++ b/ppx/test/component.t/run.t @@ -28,12 +28,15 @@ We need to output ML syntax here, otherwise refmt could not parse it. let make = ((fun ?(name= "") -> React.jsx 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" ()))|]) + (((ReactDOM.domProps)[@merlin.hide ]) + ~children:[|(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 From 47d2c13fb717da1444a88371cf82cdf1dff903c9 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Mon, 3 Jun 2024 18:50:19 -0700 Subject: [PATCH 4/5] wrap in array --- ppx/reason_react_ppx.ml | 2 +- ppx/test/component.t/run.t | 17 +++++++------- ppx/test/fragment.t/input.re | 1 + ppx/test/fragment.t/run.t | 43 +++++++++++++++++++++++------------- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/ppx/reason_react_ppx.ml b/ppx/reason_react_ppx.ml index 5f7004751..cbceb3437 100644 --- a/ppx/reason_react_ppx.ml +++ b/ppx/reason_react_ppx.ml @@ -1417,7 +1417,7 @@ let jsxMapper = in (* throw away the [@JSX] attribute and keep the others, if any *) Binding.React.jsxFragment ~loc ~attrs:nonJSXAttributes - childrenExpr) + (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 index ce2af42a7..29ff6dd64 100644 --- a/ppx/test/component.t/run.t +++ b/ppx/test/component.t/run.t @@ -29,14 +29,15 @@ We need to output ML syntax here, otherwise refmt could not parse it. ((fun ?(name= "") -> React.jsx React.jsxFragment (((ReactDOM.domProps)[@merlin.hide ]) - ~children:[|(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" ()))|] ())) + ~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 index 5a94e02b0..be43bad6f 100644 --- 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 index e46b7c0e5..62789e138 100644 --- a/ppx/test/fragment.t/run.t +++ b/ppx/test/fragment.t/run.t @@ -3,24 +3,36 @@ [@bla] React.jsx( React.jsxFragment, - ([@merlin.hide] ReactDOM.domProps)(~children=[|foo|], ()), + ([@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) => React.jsx( React.jsxFragment, - ([@merlin.hide] ReactDOM.domProps)(~children=[|foo, bar|], ()), + ([@merlin.hide] ReactDOM.domProps)( + ~children=React.array([|foo, bar|]), + (), + ), ); let nested_fragment = (foo, bar, baz) => React.jsx( React.jsxFragment, ([@merlin.hide] ReactDOM.domProps)( - ~children=[| - foo, - React.jsx( - React.jsxFragment, - ([@merlin.hide] ReactDOM.domProps)(~children=[|bar, baz|], ()), - ), - |], + ~children= + React.array([| + foo, + React.jsx( + React.jsxFragment, + ([@merlin.hide] ReactDOM.domProps)( + ~children=React.array([|bar, baz|]), + (), + ), + ), + |]), (), ), ); @@ -28,12 +40,13 @@ React.jsx( React.jsxFragment, ([@merlin.hide] ReactDOM.domProps)( - ~children=[| - ReactDOM.jsx( - "div", - ([@merlin.hide] ReactDOM.domProps)(~children=foo, ()), - ), - |], + ~children= + React.array([| + ReactDOM.jsx( + "div", + ([@merlin.hide] ReactDOM.domProps)(~children=foo, ()), + ), + |]), (), ), ); From 01ee8c63d845373b6c5c84a04de01372073048ea Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Mon, 3 Jun 2024 19:00:00 -0700 Subject: [PATCH 5/5] add changelog entry --- CHANGES.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 63abea680..4e861e85f 100644 --- 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