Skip to content

Commit bca7852

Browse files
committed
fix ignoring react component arguments when aliasing props type
1 parent c1f9e01 commit bca7852

23 files changed

+348
-75
lines changed

compiler/syntax/src/jsx_v4.ml

+60-3
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,61 @@ let vb_match_expr named_arg_list expr =
920920
in
921921
aux (List.rev named_arg_list)
922922

923+
let vb_type_annotation ~expr (name, default, pattern, alias, loc, core_type) =
924+
let label = get_label name in
925+
let pattern_name =
926+
match default with
927+
| Some _ -> "__" ^ alias
928+
| None -> alias
929+
in
930+
let value_binding =
931+
Vb.mk ~loc
932+
(match pattern with
933+
| {
934+
ppat_desc =
935+
Ppat_constraint (pattern, ({ptyp_desc = Ptyp_package _} as type_));
936+
} ->
937+
(* Handle pattern: ~comp as module(Comp: Comp) *)
938+
Pat.record
939+
[
940+
( {txt = Lident label; loc = Location.none},
941+
Pat.constraint_ pattern type_ );
942+
]
943+
Closed
944+
| _ -> (
945+
(* For other cases, use regular variable pattern with type constraint *)
946+
let type_ =
947+
match pattern with
948+
| {ppat_desc = Ppat_constraint (_, type_)} -> Some type_
949+
| _ -> core_type
950+
in
951+
match type_ with
952+
| Some type_ ->
953+
Pat.constraint_ (Pat.var (Location.mkloc pattern_name loc)) type_
954+
| None -> Pat.var (Location.mkloc pattern_name loc)))
955+
(match pattern with
956+
| {ppat_desc = Ppat_constraint (_, {ptyp_desc = Ptyp_package _})} ->
957+
(* For module types, use props directly *)
958+
Exp.ident {txt = Lident "props"; loc = Location.none}
959+
| _ ->
960+
(* For other cases, use props.x form *)
961+
Exp.field
962+
(Exp.ident {txt = Lident "props"; loc = Location.none})
963+
{txt = Lident label; loc = Location.none})
964+
in
965+
Exp.let_ Nonrecursive [value_binding] expr
966+
967+
let vb_type_annotations_expr named_arg_list expr =
968+
let rec aux named_arg_list =
969+
match named_arg_list with
970+
| [] -> expr
971+
| ((name, _, _, _, _, _) as named_arg) :: rest ->
972+
let label = get_label name in
973+
if label = "ref" then aux rest
974+
else vb_type_annotation named_arg ~expr:(aux rest)
975+
in
976+
aux (List.rev named_arg_list)
977+
923978
let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
924979
if Jsx_common.has_attr_on_binding binding then (
925980
check_multiple_components ~config ~loc:pstr_loc;
@@ -1105,6 +1160,8 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
11051160
vb_match_expr named_arg_list expression
11061161
else expression
11071162
in
1163+
(* add pattern matching for optional prop value and type annotations *)
1164+
let expression = vb_type_annotations_expr named_arg_list expression in
11081165
(* (ref) => expr *)
11091166
let expression =
11101167
List.fold_left
@@ -1118,11 +1175,11 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
11181175
Exp.fun_ Nolabel None pattern expr)
11191176
expression patterns_with_nolabel
11201177
in
1121-
(* ({a, b, _}: props<'a, 'b>) *)
1178+
(* (props: props<'a, 'b>) *)
11221179
let record_pattern =
11231180
match patterns_with_label with
1124-
| [] -> Pat.any ()
1125-
| _ -> Pat.record (List.rev patterns_with_label) Open
1181+
| [] -> Pat.any () (* (_: props<'a, 'b>)*)
1182+
| _ -> Pat.var @@ Location.mknoloc "props"
11261183
in
11271184
let expression =
11281185
Exp.fun_ Nolabel None

tests/build_tests/react_ppx/src/recursive_component_test.bs.js

+2-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/syntax_tests/data/ppx/react/expected/aliasProps.res.txt

+23-7
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ module C0 = {
44
@res.jsxComponentProps
55
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
66

7-
let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
7+
let make = (props: props<_, _>) => {
8+
let _ = props.priority
9+
let __text = props.text
810
let text = switch __text {
911
| Some(text) => text
1012
| None => "Test"
@@ -23,7 +25,9 @@ module C1 = {
2325
@res.jsxComponentProps
2426
type props<'priority, 'text> = {priority: 'priority, text?: 'text}
2527

26-
let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
28+
let make = (props: props<_, _>) => {
29+
let p = props.priority
30+
let __text = props.text
2731
let text = switch __text {
2832
| Some(text) => text
2933
| None => "Test"
@@ -42,7 +46,8 @@ module C2 = {
4246
@res.jsxComponentProps
4347
type props<'foo> = {foo?: 'foo}
4448

45-
let make = ({foo: ?__bar, _}: props<_>) => {
49+
let make = (props: props<_>) => {
50+
let __bar = props.foo
4651
let bar = switch __bar {
4752
| Some(foo) => foo
4853
| None => ""
@@ -61,7 +66,10 @@ module C3 = {
6166
@res.jsxComponentProps
6267
type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
6368

64-
let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
69+
let make = (props: props<_, _, _>) => {
70+
let __bar = props.foo
71+
let __a = props.a
72+
let b = props.b
6573
let bar = switch __bar {
6674
| Some(foo) => foo
6775
| None => ""
@@ -86,7 +94,9 @@ module C4 = {
8694
@res.jsxComponentProps
8795
type props<'a, 'x> = {a: 'a, x?: 'x}
8896

89-
let make = ({a: b, x: ?__x, _}: props<_, _>) => {
97+
let make = (props: props<_, _>) => {
98+
let b = props.a
99+
let __x = props.x
90100
let x = switch __x {
91101
| Some(x) => x
92102
| None => true
@@ -105,7 +115,9 @@ module C5 = {
105115
@res.jsxComponentProps
106116
type props<'a, 'z> = {a: 'a, z?: 'z}
107117

108-
let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
118+
let make = (props: props<_, _>) => {
119+
let a = props.a
120+
let __z = props.z
109121
let z = switch __z {
110122
| Some(z) => z
111123
| None => 3
@@ -130,7 +142,11 @@ module C6 = {
130142
@res.jsxComponentProps
131143
type props<'comp, 'x> = {comp: 'comp, x: 'x}
132144

133-
let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>) => React.jsx(Comp.make, {})
145+
let make = (props: props<_, _>) => {
146+
let {comp: module(Comp: Comp)} = props
147+
let x = props.x
148+
React.jsx(Comp.make, {})
149+
}
134150
let make = {
135151
let \"AliasProps$C6" = (props: props<_>) => make(props)
136152

tests/syntax_tests/data/ppx/react/expected/asyncAwait.res.txt

+9-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ module C0 = {
44
@res.jsxComponentProps
55
type props<'a> = {a: 'a}
66

7-
let make = async ({a, _}: props<_>) => {
7+
let make = async (props: props<_>) => {
8+
let a = props.a
89
let a = await f(a)
910
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({React.int(a)})})
1011
}
@@ -19,10 +20,13 @@ module C1 = {
1920
@res.jsxComponentProps
2021
type props<'status> = {status: 'status}
2122

22-
let make = async ({status, _}: props<_>) => {
23-
switch status {
24-
| #on => React.string("on")
25-
| #off => React.string("off")
23+
let make = async (props: props<_>) => {
24+
let status = props.status
25+
{
26+
switch status {
27+
| #on => React.string("on")
28+
| #off => React.string("off")
29+
}
2630
}
2731
}
2832
let make = {

tests/syntax_tests/data/ppx/react/expected/commentAtTop.res.txt

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
@res.jsxComponentProps
22
type props<'msg> = {msg: 'msg} // test React JSX file
33

4-
let make = ({msg, _}: props<_>) => {
5-
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
4+
let make = (props: props<_>) => {
5+
let msg = props.msg
6+
{
7+
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
8+
}
69
}
710
let make = {
811
let \"CommentAtTop" = (props: props<_>) => make(props)

tests/syntax_tests/data/ppx/react/expected/defaultValueProp.res.txt

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
module C0 = {
22
@res.jsxComponentProps
33
type props<'a, 'b> = {a?: 'a, b?: 'b}
4-
let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
4+
let make = (props: props<_, _>) => {
5+
let __a = props.a
6+
let __b = props.b
57
let a = switch __a {
68
| Some(a) => a
79
| None => 2
@@ -23,7 +25,9 @@ module C1 = {
2325
@res.jsxComponentProps
2426
type props<'a, 'b> = {a?: 'a, b: 'b}
2527

26-
let make = ({a: ?__a, b, _}: props<_, _>) => {
28+
let make = (props: props<_, _>) => {
29+
let __a = props.a
30+
let b = props.b
2731
let a = switch __a {
2832
| Some(a) => a
2933
| None => 2
@@ -43,7 +47,8 @@ module C2 = {
4347
@res.jsxComponentProps
4448
type props<'a> = {a?: 'a}
4549

46-
let make = ({a: ?__a, _}: props<_>) => {
50+
let make = (props: props<_>) => {
51+
let __a = props.a
4752
let a = switch __a {
4853
| Some(a) => a
4954
| None => a
@@ -62,7 +67,8 @@ module C3 = {
6267
@res.jsxComponentProps
6368
type props<'disabled> = {disabled?: 'disabled}
6469

65-
let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
70+
let make = (props: props<bool>) => {
71+
let __everythingDisabled: bool = props.disabled
6672
let everythingDisabled = switch __everythingDisabled {
6773
| Some(disabled) => disabled
6874
| None => false

tests/syntax_tests/data/ppx/react/expected/fileLevelConfig.res.txt

+10-4
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ module V4C = {
44
@res.jsxComponentProps
55
type props<'msg> = {msg: 'msg}
66

7-
let make = ({msg, _}: props<_>) => {
8-
ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
7+
let make = (props: props<_>) => {
8+
let msg = props.msg
9+
{
10+
ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
11+
}
912
}
1013
let make = {
1114
let \"FileLevelConfig$V4C" = (props: props<_>) => make(props)
@@ -20,8 +23,11 @@ module V4A = {
2023
@res.jsxComponentProps
2124
type props<'msg> = {msg: 'msg}
2225

23-
let make = ({msg, _}: props<_>) => {
24-
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
26+
let make = (props: props<_>) => {
27+
let msg = props.msg
28+
{
29+
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
30+
}
2531
}
2632
let make = {
2733
let \"FileLevelConfig$V4A" = (props: props<_>) => make(props)

tests/syntax_tests/data/ppx/react/expected/firstClassModules.res.txt

+29-9
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,12 @@ module Select = {
1313
items: 'items,
1414
}
1515

16-
let make = (
17-
type a key,
18-
{model: module(T: T with type t = a and type key = key), selected, onChange, items, _}: props<
19-
_,
20-
option<key>,
21-
option<key> => unit,
22-
array<a>,
23-
>,
24-
) => {
16+
let make = (type a key, props: props<_, option<key>, option<key> => unit, array<a>>) => {
17+
let {model: module(T: T with type t = a and type key = key)} = props
18+
let selected: option<key> = props.selected
19+
let onChange: option<key> => unit = props.onChange
20+
let items: array<a> = props.items
21+
2522
let _ = (model, selected, onChange, items)
2623
ReactDOM.createDOMElementVariadic("div", [])
2724
}
@@ -32,6 +29,29 @@ module Select = {
3229
}
3330
}
3431

32+
module C6 = {
33+
module type Comp = {
34+
let xx: int
35+
@res.jsxComponentProps
36+
type props = {}
37+
38+
let make: React.componentLike<props, React.element>
39+
}
40+
@res.jsxComponentProps
41+
type props<'comp, 'x> = {comp: 'comp, x: 'x}
42+
43+
let make = (props: props<_, _>) => {
44+
let {comp: module(Comp: Comp)} = props
45+
let x = props.x
46+
Comp.xx
47+
}
48+
let make = {
49+
let \"FirstClassModules$C6" = (props: props<_>) => make(props)
50+
51+
\"FirstClassModules$C6"
52+
}
53+
}
54+
3555
module External = {
3656
module type T = {
3757
type key

0 commit comments

Comments
 (0)