From 8b40919d32c1946e3c87fae4aa46b010e278a517 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Fri, 30 Sep 2022 01:12:36 +0900 Subject: [PATCH 1/4] record-based representation for jsx lowercase --- cli/reactjs_jsx_v4.ml | 37 ++++++++++++--------- tests/ppx/react/expected/forwardRef.res.txt | 11 +++--- tests/ppx/react/expected/lowercases.res.txt | 9 +++++ tests/ppx/react/lowercases.res | 6 ++++ 4 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 tests/ppx/react/expected/lowercases.res.txt create mode 100644 tests/ppx/react/lowercases.res diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index a237572a..04c173aa 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -27,7 +27,8 @@ let getLabel str = | Optional str | Labelled str -> str | Nolabel -> "" -let optionalAttr = [({txt = "ns.optional"; loc = Location.none}, PStr [])] +let optionalAttr = ({txt = "ns.optional"; loc = Location.none}, PStr []) +let optionalAttrs = [optionalAttr] let constantString ~loc str = Ast_helper.Exp.constant ~loc (Pconst_string (str, None)) @@ -211,7 +212,7 @@ let recordFromProps ~loc ~removeKey callArguments = let id = getLabel arg_label in if isOptional arg_label then ( {txt = Lident id; loc = pexp_loc}, - {pexpr with pexp_attributes = optionalAttr} ) + {pexpr with pexp_attributes = optionalAttrs} ) else ({txt = Lident id; loc = pexp_loc}, pexpr) in let fields = props |> List.map processProp in @@ -288,9 +289,9 @@ let makeLabelDecls ~loc namedTypeList = namedTypeList |> List.map (fun (isOptional, label, _, interiorType) -> if label = "key" then - Type.field ~loc ~attrs:optionalAttr {txt = label; loc} interiorType + Type.field ~loc ~attrs:optionalAttrs {txt = label; loc} interiorType else if isOptional then - Type.field ~loc ~attrs:optionalAttr {txt = label; loc} + Type.field ~loc ~attrs:optionalAttrs {txt = label; loc} (Typ.var @@ safeTypeFromValue @@ Labelled label) else Type.field ~loc {txt = label; loc} @@ -543,19 +544,25 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs (nolabel, childrenExpr); ] | nonEmptyProps -> - let propsCall = - Exp.apply - (Exp.ident - {loc = Location.none; txt = Ldot (Lident "ReactDOM", "domProps")}) + let propsRecord = + Exp.record (nonEmptyProps - |> List.map (fun (label, expression) -> - (label, mapper.expr mapper expression))) + |> List.filter_map (fun (label, expression) -> + match label with + | Labelled txt | Optional txt -> + Some + ( {txt = Lident txt; loc = Location.none}, + if isOptional label then + Exp.attr (mapper.expr mapper expression) optionalAttr + else mapper.expr mapper expression ) + | Nolabel -> None)) + None in [ (* "div" *) (nolabel, componentNameExpr); (* ReactDOM.domProps(~className=blabla, ~foo=bar, ()) *) - (labelled "props", propsCall); + (labelled "props", propsRecord); (* [|moreCreateElementCallsHere|] *) (nolabel, childrenExpr); ] @@ -688,14 +695,14 @@ let argToType ~newtypes ~(typeConstraints : core_type option) types in match (type_, name, default) with | Some type_, name, _ when isOptional name -> - (true, getLabel name, [], {type_ with ptyp_attributes = optionalAttr}) + (true, getLabel name, [], {type_ with ptyp_attributes = optionalAttrs}) :: types | Some type_, name, _ -> (false, getLabel name, [], type_) :: types | None, name, _ when isOptional name -> ( true, getLabel name, [], - Typ.var ~loc ~attrs:optionalAttr (safeTypeFromValue name) ) + Typ.var ~loc ~attrs:optionalAttrs (safeTypeFromValue name) ) :: types | None, name, _ when isLabelled name -> (false, getLabel name, [], Typ.var ~loc (safeTypeFromValue name)) :: types @@ -1052,7 +1059,7 @@ let transformStructureItem ~config mapper item = { patternWithoutConstraint with ppat_attributes = - (if isOptional arg_label then optionalAttr else []) + (if isOptional arg_label then optionalAttrs else []) @ pattern.ppat_attributes; } ) :: patternsWithLabel) @@ -1068,7 +1075,7 @@ let transformStructureItem ~config mapper item = { pattern with ppat_attributes = - optionalAttr @ pattern.ppat_attributes; + optionalAttrs @ pattern.ppat_attributes; } ) :: patternsWithNolabel) expr diff --git a/tests/ppx/react/expected/forwardRef.res.txt b/tests/ppx/react/expected/forwardRef.res.txt index a9f2701d..73af6f9c 100644 --- a/tests/ppx/react/expected/forwardRef.res.txt +++ b/tests/ppx/react/expected/forwardRef.res.txt @@ -82,12 +82,11 @@ module V4C = { [ ReactDOM.createDOMElementVariadic( "input", - ~props=ReactDOM.domProps( - ~type_="text", - ~className?, - ~ref=?{Js.Nullable.toOption(ref)->Belt.Option.map(React.Ref.domRef)}, - (), - ), + ~props={ + type_: "text", + ?className, + ref: ?Js.Nullable.toOption(ref)->Belt.Option.map(React.Ref.domRef), + }, [], ), children, diff --git a/tests/ppx/react/expected/lowercases.res.txt b/tests/ppx/react/expected/lowercases.res.txt new file mode 100644 index 00000000..67fd28b9 --- /dev/null +++ b/tests/ppx/react/expected/lowercases.res.txt @@ -0,0 +1,9 @@ +@@jsxConfig({version: 4, mode: "classic"}) + +let _ = ReactDOM.createDOMElementVariadic("div", []) +let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: "k"}, []) +let _ = ReactDOM.createDOMElementVariadic("div", ~props={x: x}, []) +let _ = ReactDOM.createDOMElementVariadic( + "div", + [ReactDOM.createDOMElementVariadic("p", [{React.string(x)}])], +) diff --git a/tests/ppx/react/lowercases.res b/tests/ppx/react/lowercases.res new file mode 100644 index 00000000..5440dc29 --- /dev/null +++ b/tests/ppx/react/lowercases.res @@ -0,0 +1,6 @@ +@@jsxConfig({version:4, mode:"classic"}) + +let _ =
+let _ =
+let _ =
+let _ =

{React.string(x)}

\ No newline at end of file From f5c59d41c00eeb768ddbad3e90a789c6ffec7a8a Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Fri, 30 Sep 2022 09:01:28 +0900 Subject: [PATCH 2/4] spread props for lowercase --- cli/reactjs_jsx_v4.ml | 13 +------------ tests/ppx/react/expected/lowercases.res.txt | 6 ++++++ tests/ppx/react/lowercases.res | 8 +++++++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index 04c173aa..02d20ee5 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -545,18 +545,7 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs ] | nonEmptyProps -> let propsRecord = - Exp.record - (nonEmptyProps - |> List.filter_map (fun (label, expression) -> - match label with - | Labelled txt | Optional txt -> - Some - ( {txt = Lident txt; loc = Location.none}, - if isOptional label then - Exp.attr (mapper.expr mapper expression) optionalAttr - else mapper.expr mapper expression ) - | Nolabel -> None)) - None + recordFromProps ~loc:Location.none ~removeKey:false nonEmptyProps in [ (* "div" *) diff --git a/tests/ppx/react/expected/lowercases.res.txt b/tests/ppx/react/expected/lowercases.res.txt index 67fd28b9..262c7aac 100644 --- a/tests/ppx/react/expected/lowercases.res.txt +++ b/tests/ppx/react/expected/lowercases.res.txt @@ -7,3 +7,9 @@ let _ = ReactDOM.createDOMElementVariadic( "div", [ReactDOM.createDOMElementVariadic("p", [{React.string(x)}])], ) +let _ = ReactDOM.createDOMElementVariadic("div", ~props=str, []) +let _ = ReactDOM.createDOMElementVariadic("div", ~props={...str, x: "x"}, []) + +// syntax error +// let _ =
+// let _ =
diff --git a/tests/ppx/react/lowercases.res b/tests/ppx/react/lowercases.res index 5440dc29..1dba63d9 100644 --- a/tests/ppx/react/lowercases.res +++ b/tests/ppx/react/lowercases.res @@ -3,4 +3,10 @@ let _ =
let _ =
let _ =
-let _ =

{React.string(x)}

\ No newline at end of file +let _ =

{React.string(x)}

+let _ =
+let _ =
+ +// syntax error +// let _ =
+// let _ =
\ No newline at end of file From a82949fb2571fd810a1926396a6a9b2e87302f12 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Sat, 8 Oct 2022 23:24:37 +0900 Subject: [PATCH 3/4] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da897ab1..3f4856d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,10 @@ - Functions with consecutive dots now print as multiple arrow functions like in JavaScript. +#### :nail_care Polish + +- Change the internal representation of props for the lowercase components to record. https://github.com/rescript-lang/syntax/pull/665 + ## ReScript 10.0 - Fix printing for inline nullary functor types [#477](https://github.com/rescript-lang/syntax/pull/477) From 07190b4957a989f3fbbe10412682da8a7a02884c Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Sun, 9 Oct 2022 00:51:41 +0900 Subject: [PATCH 4/4] fix build --- cli/reactjs_jsx_v4.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index 02d20ee5..8303ff85 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -463,7 +463,7 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs | Exact children -> [ ( labelled "children", - Exp.apply ~attrs:optionalAttr + Exp.apply ~attrs:optionalAttrs (Exp.ident { txt = Ldot (Lident "ReactDOM", "someElement");