From fc156dd5d1857ce64ae61643b1e2f7420040d0c8 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Tue, 4 Jun 2024 04:01:45 +0200 Subject: [PATCH] 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 --- CHANGES.md | 5 ++++ ppx/reason_react_ppx.ml | 56 +++++++++++++++++++----------------- ppx/test/component.t/run.t | 18 +++++++----- ppx/test/fragment.t/input.re | 1 + ppx/test/fragment.t/run.t | 54 +++++++++++++++++++++++++++------- 5 files changed, 90 insertions(+), 44 deletions(-) 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 diff --git a/ppx/reason_react_ppx.ml b/ppx/reason_react_ppx.ml index 0b8f240d5..cbceb3437 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") } @@ -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 index 0e5b91102..29ff6dd64 100644 --- 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 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 421251412..62789e138 100644 --- 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, ()), + ), + |]), + (), + ), );