Skip to content

Commit 95e5923

Browse files
cristianoccristianoc
committed
Fix issue with JSX V4 when components are nested
Fixes #5975 The issue is due to a combination of: 1) when a component let make = body is transformed, first the body is transformed, then when transformStructureItem4 is called, the body is transformed again 2) @react.component is left after the PPX transformation Because of this, in the case of nested components, the transformed inner components is transformed again by transformStructureItem4, but it should not as it's been desugared already. It would be enough to change either one, to avoid the error with nested components, but this PR handles both aspects.
1 parent bf8f1a3 commit 95e5923

24 files changed

+66
-72
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ These are only breaking changes for unformatted code.
7474
- Support `@gentype.import` as an alias to `@genType.import` in the compiler https://github.com/rescript-lang/rescript-compiler/pull/6020
7575
- Fix issue with integer overflow check https://github.com/rescript-lang/rescript-compiler/pull/6028
7676
- Fix issue with JSX V4 and newtype https://github.com/rescript-lang/rescript-compiler/pull/6029
77+
- Fix issue with JSX V4 when components are nested https://github.com/rescript-lang/rescript-compiler/pull/6031
7778

7879
#### :nail_care: Polish
7980

‎res_syntax/src/reactjs_jsx_ppx.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ let getMapper ~config =
127127
| Pstr_attribute attr -> processConfigAttribute attr config
128128
| _ -> ());
129129
let item = default_mapper.structure_item mapper item in
130-
if config.version = 3 then transformStructureItem3 mapper item
131-
else if config.version = 4 then transformStructureItem4 mapper item
130+
if config.version = 3 then transformStructureItem3 item
131+
else if config.version = 4 then transformStructureItem4 item
132132
else [item])
133133
items
134134
|> List.flatten

‎res_syntax/src/reactjs_jsx_v3.ml

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -446,8 +446,7 @@ let jsxMapper ~config =
446446
args
447447
in
448448

449-
let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes =
450-
let expr = mapper.expr mapper expr in
449+
let rec recursivelyTransformNamedArgsForMake expr args newtypes =
451450
match expr.pexp_desc with
452451
(* TODO: make this show up with a loc. *)
453452
| Pexp_fun (Labelled "key", _, _, _) | Pexp_fun (Optional "key", _, _, _) ->
@@ -494,7 +493,7 @@ let jsxMapper ~config =
494493
| _ -> None
495494
in
496495

497-
recursivelyTransformNamedArgsForMake mapper expression
496+
recursivelyTransformNamedArgsForMake expression
498497
((arg, default, pattern, alias, pattern.ppat_loc, type_) :: args)
499498
newtypes
500499
| Pexp_fun
@@ -517,10 +516,9 @@ let jsxMapper ~config =
517516
"React: react.component refs only support plain arguments and type \
518517
annotations."
519518
| Pexp_newtype (label, expression) ->
520-
recursivelyTransformNamedArgsForMake mapper expression args
521-
(label :: newtypes)
519+
recursivelyTransformNamedArgsForMake expression args (label :: newtypes)
522520
| Pexp_constraint (expression, _typ) ->
523-
recursivelyTransformNamedArgsForMake mapper expression args newtypes
521+
recursivelyTransformNamedArgsForMake expression args newtypes
524522
| _ -> (args, newtypes, None)
525523
in
526524

@@ -586,7 +584,7 @@ let jsxMapper ~config =
586584
in
587585

588586
let nestedModules = ref [] in
589-
let transformStructureItem mapper item =
587+
let transformStructureItem item =
590588
match item with
591589
(* external *)
592590
| {
@@ -825,7 +823,7 @@ let jsxMapper ~config =
825823
let props = getPropsAttr payload in
826824
(* do stuff here! *)
827825
let namedArgList, newtypes, forwardRef =
828-
recursivelyTransformNamedArgsForMake mapper
826+
recursivelyTransformNamedArgsForMake
829827
(modifiedBindingOld binding)
830828
[] []
831829
in

‎res_syntax/src/reactjs_jsx_v4.ml

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -598,9 +598,8 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs
598598
})
599599
args
600600

601-
let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
601+
let rec recursivelyTransformNamedArgsForMake expr args newtypes coreType
602602
=
603-
let expr = mapper.expr mapper expr in
604603
match expr.pexp_desc with
605604
(* TODO: make this show up with a loc. *)
606605
| Pexp_fun (Labelled "key", _, _, _) | Pexp_fun (Optional "key", _, _, _) ->
@@ -647,7 +646,7 @@ let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
647646
| _ -> None
648647
in
649648

650-
recursivelyTransformNamedArgsForMake mapper expression
649+
recursivelyTransformNamedArgsForMake expression
651650
((arg, default, pattern, alias, pattern.ppat_loc, type_) :: args)
652651
newtypes coreType
653652
| Pexp_fun
@@ -680,10 +679,10 @@ let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
680679
"React: react.component refs only support plain arguments and type \
681680
annotations."
682681
| Pexp_newtype (label, expression) ->
683-
recursivelyTransformNamedArgsForMake mapper expression args
682+
recursivelyTransformNamedArgsForMake expression args
684683
(label :: newtypes) coreType
685684
| Pexp_constraint (expression, coreType) ->
686-
recursivelyTransformNamedArgsForMake mapper expression args newtypes
685+
recursivelyTransformNamedArgsForMake expression args newtypes
687686
(Some coreType)
688687
| _ -> (args, newtypes, coreType)
689688

@@ -724,7 +723,7 @@ let check_string_int_attribute_iter =
724723

725724
{Ast_iterator.default_iterator with attribute}
726725

727-
let transformStructureItem ~config mapper item =
726+
let transformStructureItem ~config item =
728727
match item with
729728
(* external *)
730729
| {
@@ -848,6 +847,8 @@ let transformStructureItem ~config mapper item =
848847
binding with
849848
pvb_pat = {binding.pvb_pat with ppat_loc = emptyLoc};
850849
pvb_loc = emptyLoc;
850+
pvb_attributes =
851+
binding.pvb_attributes |> List.filter otherAttrsPure;
851852
}
852853
in
853854
let fnName = getFnName binding.pvb_pat in
@@ -891,8 +892,7 @@ let transformStructureItem ~config mapper item =
891892
let modifiedBinding binding =
892893
let hasApplication = ref false in
893894
let wrapExpressionWithBinding expressionFn expression =
894-
Vb.mk ~loc:bindingLoc
895-
~attrs:(List.filter otherAttrsPure binding.pvb_attributes)
895+
Vb.mk ~loc:bindingLoc ~attrs:binding.pvb_attributes
896896
(Pat.var ~loc:bindingPatLoc {loc = bindingPatLoc; txt = fnName})
897897
(expressionFn expression)
898898
in
@@ -1000,7 +1000,7 @@ let transformStructureItem ~config mapper item =
10001000
in
10011001
(* do stuff here! *)
10021002
let namedArgList, newtypes, _typeConstraints =
1003-
recursivelyTransformNamedArgsForMake mapper
1003+
recursivelyTransformNamedArgsForMake
10041004
(modifiedBindingOld binding)
10051005
[] [] None
10061006
in

‎res_syntax/tests/ppx/react/expected/aliasProps.res.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
module C0 = {
44
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
55

6-
@react.component
76
let make = (props: props<_, _>) => {
87
let _ = props.priority
98
let text = switch props.text {
@@ -23,7 +22,6 @@ module C0 = {
2322
module C1 = {
2423
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
2524

26-
@react.component
2725
let make = (props: props<_, _>) => {
2826
let p = props.priority
2927
let text = switch props.text {
@@ -43,7 +41,6 @@ module C1 = {
4341
module C2 = {
4442
type props<'foo> = {foo?: 'foo}
4543

46-
@react.component
4744
let make = (props: props<_>) => {
4845
let bar = switch props.foo {
4946
| Some(foo) => foo

‎res_syntax/tests/ppx/react/expected/commentAtTop.res.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
type props<'msg> = {msg: 'msg} // test React JSX file
22

3-
@react.component
43
let make = ({msg, _}: props<_>) => {
54
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
65
}

‎res_syntax/tests/ppx/react/expected/defaultValueProp.res.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
module C0 = {
22
type props<'a, 'b> = {a?: 'a, b?: 'b}
3-
@react.component
43
let make = (props: props<_, _>) => {
54
let a = switch props.a {
65
| Some(a) => a
@@ -22,7 +21,6 @@ module C0 = {
2221
module C1 = {
2322
type props<'a, 'b> = {a?: 'a, b: 'b}
2423

25-
@react.component
2624
let make = (props: props<_, _>) => {
2725
let a = switch props.a {
2826
| Some(a) => a

‎res_syntax/tests/ppx/react/expected/fileLevelConfig.res.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ module V3 = {
2020
module V4C = {
2121
type props<'msg> = {msg: 'msg}
2222

23-
@react.component
2423
let make = ({msg, _}: props<_>) => {
2524
ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
2625
}
@@ -36,7 +35,6 @@ module V4C = {
3635
module V4A = {
3736
type props<'msg> = {msg: 'msg}
3837

39-
@react.component
4038
let make = ({msg, _}: props<_>) => {
4139
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
4240
}

‎res_syntax/tests/ppx/react/expected/firstClassModules.res.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ module Select = {
6464
items: 'items,
6565
}
6666

67-
@react.component
6867
let make = (
6968
type a key,
7069
{model, selected, onChange, items, _}: props<

‎res_syntax/tests/ppx/react/expected/forwardRef.res.txt

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ module V4C = {
7575
ref?: 'ref,
7676
}
7777

78-
@react.component
7978
let make = (
8079
{?className, children, _}: props<_, _, ReactRef.currentDomRef>,
8180
ref: Js.Nullable.t<ReactRef.currentDomRef>,
@@ -103,7 +102,6 @@ module V4C = {
103102
}
104103
type props = {}
105104

106-
@react.component
107105
let make = (_: props) => {
108106
let input = React.useRef(Js.Nullable.null)
109107

@@ -132,7 +130,6 @@ module V4CUncurried = {
132130
ref?: 'ref,
133131
}
134132

135-
@react.component
136133
let make = (
137134
{?className, children, _}: props<_, _, ReactRef.currentDomRef>,
138135
ref: Js.Nullable.t<ReactRef.currentDomRef>,
@@ -160,7 +157,6 @@ module V4CUncurried = {
160157
}
161158
type props = {}
162159

163-
@react.component
164160
let make = (_: props) => {
165161
let input = React.useRef(Js.Nullable.null)
166162

@@ -191,7 +187,6 @@ module V4A = {
191187
ref?: 'ref,
192188
}
193189

194-
@react.component
195190
let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
196191
ReactDOM.jsxs(
197192
"div",
@@ -217,7 +212,6 @@ module V4A = {
217212
}
218213
type props = {}
219214

220-
@react.component
221215
let make = (_: props) => {
222216
let input = React.useRef(Js.Nullable.null)
223217

@@ -245,7 +239,6 @@ module V4AUncurried = {
245239
ref?: 'ref,
246240
}
247241

248-
@react.component
249242
let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
250243
ReactDOM.jsxs(
251244
"div",
@@ -271,7 +264,6 @@ module V4AUncurried = {
271264
}
272265
type props = {}
273266

274-
@react.component
275267
let make = (_: props) => {
276268
let input = React.useRef(Js.Nullable.null)
277269

‎res_syntax/tests/ppx/react/expected/interface.res.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module A = {
22
type props<'x> = {x: 'x}
3-
@react.component let make = ({x, _}: props<_>) => React.string(x)
3+
let make = ({x, _}: props<_>) => React.string(x)
44
let make = {
55
let \"Interface$A" = (props: props<_>) => make(props)
66
\"Interface$A"
@@ -10,7 +10,7 @@ module A = {
1010
module NoProps = {
1111
type props = {}
1212

13-
@react.component let make = (_: props) => ReactDOM.jsx("div", {})
13+
let make = (_: props) => ReactDOM.jsx("div", {})
1414
let make = {
1515
let \"Interface$NoProps" = props => make(props)
1616

‎res_syntax/tests/ppx/react/expected/interfaceWithRef.res.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
type props<'x, 'ref> = {x: 'x, ref?: 'ref}
2-
@react.component
32
let make = (
43
{x, _}: props<string, ReactDOM.Ref.currentDomRef>,
54
ref: Js.Nullable.t<ReactDOM.Ref.currentDomRef>,

‎res_syntax/tests/ppx/react/expected/mangleKeyword.res.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ let c31 = React.createElement(C31.make, C31.makeProps(~_open="x", ()))
2222
module C4C0 = {
2323
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}
2424

25-
@react.component
2625
let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open)
2726
let make = {
2827
let \"MangleKeyword$C4C0" = (props: props<_>) => make(props)
@@ -44,7 +43,6 @@ let c4c1 = React.createElement(C4C1.make, {_open: "x", _type: "t"})
4443
module C4A0 = {
4544
type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}
4645

47-
@react.component
4846
let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open)
4947
let make = {
5048
let \"MangleKeyword$C4A0" = (props: props<_>) => make(props)

‎res_syntax/tests/ppx/react/expected/nested.res.txt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
module Outer = {
2+
type props = {}
3+
let make = (_: props) => {
4+
module Inner = {
5+
type props = {}
6+
7+
let make = (_: props) => ReactDOM.jsx("div", {})
8+
let make = {
9+
let \"Nested$Outer" = props => make(props)
10+
11+
\"Nested$Outer"
12+
}
13+
}
14+
15+
React.jsx(Inner.make, {})
16+
}
17+
let make = {
18+
let \"Nested$Outer" = props => make(props)
19+
\"Nested$Outer"
20+
}
21+
}

‎res_syntax/tests/ppx/react/expected/newtype.res.txt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ module V3 = {
2626
module V4C = {
2727
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}
2828

29-
@react.component
3029
let make = (type a, {a, b, c, _}: props<a, array<option<[#Foo(a)]>>, 'a>) =>
3130
ReactDOM.createDOMElementVariadic("div", [])
3231
let make = {
@@ -41,7 +40,6 @@ module V4C = {
4140
module V4A = {
4241
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}
4342

44-
@react.component
4543
let make = (type a, {a, b, c, _}: props<a, array<option<[#Foo(a)]>>, 'a>) =>
4644
ReactDOM.jsx("div", {})
4745
let make = {
@@ -54,7 +52,6 @@ module V4A = {
5452
module V4A1 = {
5553
type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}
5654

57-
@react.component
5855
let make = (type x y, {a, b, c, _}: props<x, array<y>, 'a>) => ReactDOM.jsx("div", {})
5956
let make = {
6057
let \"Newtype$V4A1" = (props: props<_>) => make(props)
@@ -70,7 +67,6 @@ module type T = {
7067
module V4A2 = {
7168
type props<'foo> = {foo: 'foo}
7269

73-
@react.component
7470
let make = (type a, {foo, _}: props<module(T with type t = a)>) => {
7571
module T = unpack(foo)
7672
ReactDOM.jsx("div", {})
@@ -85,7 +81,6 @@ module V4A2 = {
8581
module V4A3 = {
8682
type props<'foo> = {foo: 'foo}
8783

88-
@react.component
8984
let make = (type a, {foo, _}: props<_>) => {
9085
module T = unpack(foo: T with type t = a)
9186
foo

0 commit comments

Comments
 (0)