From c8a9e8e7a3f5235bd0b02039b1f2bdb3107a9c29 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Tue, 20 Sep 2022 18:13:05 +0900 Subject: [PATCH 1/5] fix the usage of spreaProps withouth other props --- cli/reactjs_jsx_v4.ml | 9 +++++---- tests/ppx/react/expected/spreadProps.res.txt | 10 ++++++++++ tests/ppx/react/spreadProps.res | 10 ++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 tests/ppx/react/expected/spreadProps.res.txt create mode 100644 tests/ppx/react/spreadProps.res diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index b118406e..63ccfdaa 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -208,20 +208,21 @@ let recordFromProps ~loc ~removeKey callArguments = let spreadFields = propsToSpread |> List.map (fun (_, expression) -> expression) in - match spreadFields with - | [] -> + match (fields, spreadFields) with + | [], [spreadProps] | [], spreadProps :: _ -> spreadProps + | _, [] -> { pexp_desc = Pexp_record (fields, None); pexp_loc = loc; pexp_attributes = []; } - | [spreadProps] -> + | _, [spreadProps] -> { pexp_desc = Pexp_record (fields, Some spreadProps); pexp_loc = loc; pexp_attributes = []; } - | spreadProps :: _ -> + | _, spreadProps :: _ -> { pexp_desc = Pexp_record (fields, Some spreadProps); pexp_loc = loc; diff --git a/tests/ppx/react/expected/spreadProps.res.txt b/tests/ppx/react/expected/spreadProps.res.txt new file mode 100644 index 00000000..1dbf2e16 --- /dev/null +++ b/tests/ppx/react/expected/spreadProps.res.txt @@ -0,0 +1,10 @@ +let c0 = React.jsx(A.make, {...p, x: "x"}) + +// ignore second one +let c0 = React.jsx(A.make, {...p0, x: "x"}) + +// only spread props +let c1 = React.jsx(A.make, p) + +// reversed order +let c2 = React.jsx(A.make, {...p, x: "x"}) diff --git a/tests/ppx/react/spreadProps.res b/tests/ppx/react/spreadProps.res new file mode 100644 index 00000000..3e83a205 --- /dev/null +++ b/tests/ppx/react/spreadProps.res @@ -0,0 +1,10 @@ +let c0 = + +// ignore second one +let c0 = + +// only spread props +let c1 = + +// reversed order +let c2 = From 1d552a136f59cb8f83188c279a13b2d95c2e6288 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Tue, 20 Sep 2022 18:17:04 +0900 Subject: [PATCH 2/5] clean up and comment --- cli/reactjs_jsx_v4.ml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index 63ccfdaa..40d479ac 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -216,12 +216,8 @@ let recordFromProps ~loc ~removeKey callArguments = pexp_loc = loc; pexp_attributes = []; } - | _, [spreadProps] -> - { - pexp_desc = Pexp_record (fields, Some spreadProps); - pexp_loc = loc; - pexp_attributes = []; - } + | _, [spreadProps] + (* take the first spreadProps only *) | _, spreadProps :: _ -> { pexp_desc = Pexp_record (fields, Some spreadProps); From 11edad2a86e31156d7a5ddf107e4c0ac34566ef0 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Tue, 20 Sep 2022 18:19:27 +0900 Subject: [PATCH 3/5] add test --- tests/ppx/react/expected/spreadProps.res.txt | 13 +++++++++++++ tests/ppx/react/spreadProps.res | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/tests/ppx/react/expected/spreadProps.res.txt b/tests/ppx/react/expected/spreadProps.res.txt index 1dbf2e16..bc5d0ffd 100644 --- a/tests/ppx/react/expected/spreadProps.res.txt +++ b/tests/ppx/react/expected/spreadProps.res.txt @@ -1,3 +1,16 @@ +@@jsxConfig({version: 4, mode: "classic"}) +let c0 = React.createElement(A.make, {...p, x: "x"}) + +// ignore second one +let c0 = React.createElement(A.make, {...p0, x: "x"}) + +// only spread props +let c1 = React.createElement(A.make, p) + +// reversed order +let c2 = React.createElement(A.make, {...p, x: "x"}) + +@@jsxConfig({version: 4, mode: "automatic"}) let c0 = React.jsx(A.make, {...p, x: "x"}) // ignore second one diff --git a/tests/ppx/react/spreadProps.res b/tests/ppx/react/spreadProps.res index 3e83a205..5e5bae5d 100644 --- a/tests/ppx/react/spreadProps.res +++ b/tests/ppx/react/spreadProps.res @@ -1,3 +1,16 @@ +@@jsxConfig({version:4, mode: "classic"}) +let c0 = + +// ignore second one +let c0 = + +// only spread props +let c1 = + +// reversed order +let c2 = + +@@jsxConfig({version:4, mode: "automatic"}) let c0 = // ignore second one From afdd64b00fd8412856fa065c57d7052020093ae8 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Thu, 22 Sep 2022 18:47:42 +0900 Subject: [PATCH 4/5] spread props errors - first in order - multiple use not allowed --- cli/reactjs_jsx_v4.ml | 11 ++++++++++- tests/ppx/react/expected/spreadProps.res.txt | 14 ++++++++------ tests/ppx/react/spreadProps.res | 14 ++++++++------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index 40d479ac..e7337d8f 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -183,7 +183,16 @@ let recordFromProps ~loc ~removeKey callArguments = | (Nolabel, {pexp_loc}) :: _rest -> React_jsx_common.raiseError ~loc:pexp_loc "JSX: found non-labelled argument before the last position" - | prop :: rest -> removeLastPositionUnitAux rest (prop :: acc) + | ((Labelled txt, {pexp_loc}) as prop) :: rest + | ((Optional txt, {pexp_loc}) as prop) :: rest -> + if txt = "_spreadProps" then + match acc with + | [] -> removeLastPositionUnitAux rest (prop :: acc) + | _ -> + React_jsx_common.raiseError ~loc:pexp_loc + "JSX: spread props should be first in order than other props\n\ + \ and multiple spread props are not allowed." + else removeLastPositionUnitAux rest (prop :: acc) in let props, propsToSpread = removeLastPositionUnitAux callArguments [] diff --git a/tests/ppx/react/expected/spreadProps.res.txt b/tests/ppx/react/expected/spreadProps.res.txt index bc5d0ffd..d43f50db 100644 --- a/tests/ppx/react/expected/spreadProps.res.txt +++ b/tests/ppx/react/expected/spreadProps.res.txt @@ -1,8 +1,9 @@ @@jsxConfig({version: 4, mode: "classic"}) -let c0 = React.createElement(A.make, {...p, x: "x"}) +// Error: spreadProps should be first in order than other props +// let c0 = -// ignore second one -let c0 = React.createElement(A.make, {...p0, x: "x"}) +// Error: multiple spreadProps not allowed +// let c0 = // only spread props let c1 = React.createElement(A.make, p) @@ -11,10 +12,11 @@ let c1 = React.createElement(A.make, p) let c2 = React.createElement(A.make, {...p, x: "x"}) @@jsxConfig({version: 4, mode: "automatic"}) -let c0 = React.jsx(A.make, {...p, x: "x"}) +// Error: spreadProps should be first in order than other props +// let c0 = -// ignore second one -let c0 = React.jsx(A.make, {...p0, x: "x"}) +// Error: multiple spreadProps not allowed +// let c0 = // only spread props let c1 = React.jsx(A.make, p) diff --git a/tests/ppx/react/spreadProps.res b/tests/ppx/react/spreadProps.res index 5e5bae5d..b50f5c08 100644 --- a/tests/ppx/react/spreadProps.res +++ b/tests/ppx/react/spreadProps.res @@ -1,8 +1,9 @@ @@jsxConfig({version:4, mode: "classic"}) -let c0 = +// Error: spreadProps should be first in order than other props +// let c0 = -// ignore second one -let c0 = +// Error: multiple spreadProps not allowed +// let c0 = // only spread props let c1 = @@ -11,10 +12,11 @@ let c1 = let c2 = @@jsxConfig({version:4, mode: "automatic"}) -let c0 = +// Error: spreadProps should be first in order than other props +// let c0 = -// ignore second one -let c0 = +// Error: multiple spreadProps not allowed +// let c0 = // only spread props let c1 = From 1cd77b9e2a738868f1361301ea4a588fdbd014e5 Mon Sep 17 00:00:00 2001 From: Woonki Moon Date: Thu, 22 Sep 2022 19:23:28 +0900 Subject: [PATCH 5/5] use variable, fix error msg --- cli/reactjs_jsx_v4.ml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index e7337d8f..edb19ba9 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -175,6 +175,7 @@ let makeModuleName fileName nestedModules fnName = (* make record from props and spread props if exists *) let recordFromProps ~loc ~removeKey callArguments = + let spreadPropsLabel = "_spreadProps" in let rec removeLastPositionUnitAux props acc = match props with | [] -> acc @@ -185,13 +186,13 @@ let recordFromProps ~loc ~removeKey callArguments = "JSX: found non-labelled argument before the last position" | ((Labelled txt, {pexp_loc}) as prop) :: rest | ((Optional txt, {pexp_loc}) as prop) :: rest -> - if txt = "_spreadProps" then + if txt = spreadPropsLabel then match acc with | [] -> removeLastPositionUnitAux rest (prop :: acc) | _ -> React_jsx_common.raiseError ~loc:pexp_loc - "JSX: spread props should be first in order than other props\n\ - \ and multiple spread props are not allowed." + "JSX: use {...p} {x: v} not {x: v} {...p} \n\ + \ multiple spreads {...p} {...p} not allowed." else removeLastPositionUnitAux rest (prop :: acc) in let props, propsToSpread =