Skip to content

Commit 1df3b23

Browse files
mununkimununki
authored
Fix the incorrect type annotation for ref in JSX4 (#6718)
* jsx4: fix incorrect type annotation for ref * update change log * add test for interface * remove unnecessary handling logic of ref for interface
1 parent 41ec1b0 commit 1df3b23

File tree

5 files changed

+231
-17
lines changed

5 files changed

+231
-17
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
- Improve error when using '@deriving(accessors)' on a variant with record arguments. https://github.com/rescript-lang/rescript-compiler/pull/6712
2222
- Stop escaping JSX prop names with hyphens. https://github.com/rescript-lang/rescript-compiler/pull/6705
2323
- Fix trailing undefined for optional parameters not omitted with `@send` and `@new`. https://github.com/rescript-lang/rescript-compiler/pull/6716
24+
- Fix JSX4 adding the incorrect type annotation for the prop `ref` in React.forwardRef component https://github.com/rescript-lang/rescript-compiler/pull/6718
2425

2526
# 11.1.0-rc.7
2627

‎jscomp/syntax/src/jsx_v4.ml

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,12 @@ let safeTypeFromValue valueStr =
4646
if valueStr = "" || (valueStr.[0] [@doesNotRaise]) <> '_' then valueStr
4747
else "T" ^ valueStr
4848

49+
let refTypeVar loc = Typ.var ~loc "ref"
50+
4951
let refType loc =
5052
Typ.constr ~loc
51-
{loc; txt = Ldot (Ldot (Lident "ReactDOM", "Ref"), "currentDomRef")}
52-
[]
53+
{loc; txt = Ldot (Ldot (Lident "Js", "Nullable"), "t")}
54+
[refTypeVar loc]
5355

5456
type 'a children = ListLiteral of 'a | Exact of 'a
5557

@@ -279,11 +281,11 @@ let makePropsTypeParams ?(stripExplicitOption = false)
279281
(* TODO: Worth thinking how about "ref_" or "_ref" usages *)
280282
else if label = "ref" then
281283
(*
282-
If ref has a type annotation then use it, else `ReactDOM.Ref.currentDomRef.
284+
If ref has a type annotation then use it, else 'ref.
283285
For example, if JSX ppx is used for React Native, type would be different.
284286
*)
285287
match interiorType with
286-
| {ptyp_desc = Ptyp_any} -> Some (refType loc)
288+
| {ptyp_desc = Ptyp_any} -> Some (refTypeVar loc)
287289
| _ ->
288290
(* Strip explicit Js.Nullable.t in case of forwardRef *)
289291
if stripExplicitJsNullableOfRef then stripJsNullable interiorType
@@ -1077,7 +1079,14 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
10771079
(* (ref) => expr *)
10781080
let expression =
10791081
List.fold_left
1080-
(fun expr (_, pattern) -> Exp.fun_ Nolabel None pattern expr)
1082+
(fun expr (_, pattern) ->
1083+
let pattern =
1084+
match pattern.ppat_desc with
1085+
| Ppat_var {txt} when txt = "ref" ->
1086+
Pat.constraint_ pattern (refType Location.none)
1087+
| _ -> pattern
1088+
in
1089+
Exp.fun_ Nolabel None pattern expr)
10811090
expression patternsWithNolabel
10821091
in
10831092
(* ({a, b, _}: props<'a, 'b>) *)
@@ -1249,7 +1258,6 @@ let transformSignatureItem ~config item =
12491258
let pval_type = Jsx_common.extractUncurried pval_type in
12501259
check_string_int_attribute_iter.signature_item
12511260
check_string_int_attribute_iter item;
1252-
let hasForwardRef = ref false in
12531261
let coreTypeOfAttr = Jsx_common.coreTypeOfAttrs pval_attributes in
12541262
let typVarsOfCoreType =
12551263
coreTypeOfAttr
@@ -1268,9 +1276,7 @@ let transformSignatureItem ~config item =
12681276
(Nolabel, {ptyp_desc = Ptyp_constr ({txt = Lident "unit"}, _)}, rest)
12691277
->
12701278
getPropTypes types rest
1271-
| Ptyp_arrow (Nolabel, _type, rest) ->
1272-
hasForwardRef := true;
1273-
getPropTypes types rest
1279+
| Ptyp_arrow (Nolabel, _type, rest) -> getPropTypes types rest
12741280
| Ptyp_arrow (name, ({ptyp_attributes = attrs} as type_), returnValue)
12751281
when isOptional name || isLabelled name ->
12761282
(returnValue, (name, attrs, returnValue.ptyp_loc, type_) :: types)
@@ -1290,12 +1296,7 @@ let transformSignatureItem ~config item =
12901296
in
12911297
let propsRecordType =
12921298
makePropsRecordTypeSig ~coreTypeOfAttr ~typVarsOfCoreType "props"
1293-
psig_loc
1294-
((* If there is Nolabel arg, regard the type as ref in forwardRef *)
1295-
(if !hasForwardRef then
1296-
[(true, "ref", [], Location.none, refType Location.none)]
1297-
else [])
1298-
@ namedTypeList)
1299+
psig_loc namedTypeList
12991300
in
13001301
(* can't be an arrow because it will defensively uncurry *)
13011302
let newExternalType =

‎jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ module V4A = {
187187
ref?: 'ref,
188188
}
189189

190-
let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
190+
let make = ({?className, children, _}: props<_, _, 'ref>, ref: Js.Nullable.t<'ref>) =>
191191
ReactDOM.jsxs(
192192
"div",
193193
{
@@ -239,7 +239,7 @@ module V4AUncurried = {
239239
ref?: 'ref,
240240
}
241241

242-
let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
242+
let make = ({?className, children, _}: props<_, _, 'ref>, ref: Js.Nullable.t<'ref>) =>
243243
ReactDOM.jsxs(
244244
"div",
245245
{

‎jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
@@jsxConfig({version: 3})
2+
3+
module V3: {
4+
module FancyInput: {
5+
@obj
6+
external makeProps: (
7+
~className: string=?,
8+
~children: React.element,
9+
~ref: ReactDOM.Ref.currentDomRef=?,
10+
~key: string=?,
11+
unit,
12+
) => {
13+
"className": option<string>,
14+
"children": React.element,
15+
"ref": option<ReactDOM.Ref.currentDomRef>,
16+
} = ""
17+
let make: React.componentLike<
18+
{
19+
"className": option<string>,
20+
"children": React.element,
21+
"ref": option<ReactDOM.Ref.currentDomRef>,
22+
},
23+
React.element,
24+
>
25+
}
26+
27+
module ForwardRef: {
28+
@obj
29+
external makeProps: (
30+
~ref: React.ref<Js.Nullable.t<inputRef>>=?,
31+
~key: string=?,
32+
unit,
33+
) => {"ref": option<React.ref<Js.Nullable.t<inputRef>>>} = ""
34+
let make: React.componentLike<
35+
{"ref": option<React.ref<Js.Nullable.t<inputRef>>>},
36+
React.element,
37+
>
38+
}
39+
}
40+
41+
@@jsxConfig({version: 4, mode: "classic"})
42+
43+
module V4C: {
44+
module FancyInput: {
45+
type props<'className, 'children, 'ref> = {
46+
className?: 'className,
47+
children: 'children,
48+
ref?: 'ref,
49+
}
50+
51+
let make: React.componentLike<
52+
props<string, React.element, ReactDOM.Ref.currentDomRef>,
53+
React.element,
54+
>
55+
}
56+
57+
module ForwardRef: {
58+
type props<'ref> = {ref?: 'ref}
59+
60+
let make: React.componentLike<props<React.ref<Js.Nullable.t<inputRef>>>, React.element>
61+
}
62+
}
63+
64+
module V4CUncurried: {
65+
module FancyInput: {
66+
type props<'className, 'children, 'ref> = {
67+
className?: 'className,
68+
children: 'children,
69+
ref?: 'ref,
70+
}
71+
72+
let make: React.componentLike<
73+
props<string, React.element, ReactDOM.Ref.currentDomRef>,
74+
React.element,
75+
>
76+
}
77+
78+
module ForwardRef: {
79+
type props<'ref> = {ref?: 'ref}
80+
81+
let make: React.componentLike<props<React.ref<Js.Nullable.t<inputRef>>>, React.element>
82+
}
83+
}
84+
85+
@@jsxConfig({version: 4, mode: "automatic"})
86+
87+
module V4A: {
88+
module FancyInput: {
89+
type props<'className, 'children, 'ref> = {
90+
className?: 'className,
91+
children: 'children,
92+
ref?: 'ref,
93+
}
94+
95+
let make: React.componentLike<
96+
props<string, React.element, ReactDOM.Ref.currentDomRef>,
97+
React.element,
98+
>
99+
}
100+
101+
module ForwardRef: {
102+
type props<'ref> = {ref?: 'ref}
103+
104+
let make: React.componentLike<props<React.ref<Js.Nullable.t<inputRef>>>, React.element>
105+
}
106+
}
107+
108+
module V4AUncurried: {
109+
module FancyInput: {
110+
type props<'className, 'children, 'ref> = {
111+
className?: 'className,
112+
children: 'children,
113+
ref?: 'ref,
114+
}
115+
116+
let make: React.componentLike<
117+
props<string, React.element, ReactDOM.Ref.currentDomRef>,
118+
React.element,
119+
>
120+
}
121+
122+
module ForwardRef: {
123+
type props<'ref> = {ref?: 'ref}
124+
125+
let make: React.componentLike<props<React.ref<Js.Nullable.t<inputRef>>>, React.element>
126+
}
127+
}

‎jscomp/syntax/tests/ppx/react/forwardRef.resi

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
@@jsxConfig({version: 3})
2+
3+
module V3: {
4+
module FancyInput: {
5+
@react.component
6+
let make: (
7+
~className: string=?,
8+
~children: React.element,
9+
~ref: ReactDOM.Ref.currentDomRef=?,
10+
) => React.element
11+
}
12+
13+
module ForwardRef: {
14+
@react.component
15+
let make: (~ref: React.ref<Js.Nullable.t<inputRef>>=?) => React.element
16+
}
17+
}
18+
19+
@@jsxConfig({version: 4, mode: "classic"})
20+
21+
module V4C: {
22+
module FancyInput: {
23+
@react.component
24+
let make: (
25+
~className: string=?,
26+
~children: React.element,
27+
~ref: ReactDOM.Ref.currentDomRef=?,
28+
) => React.element
29+
}
30+
31+
module ForwardRef: {
32+
@react.component
33+
let make: (~ref: React.ref<Js.Nullable.t<inputRef>>=?) => React.element
34+
}
35+
}
36+
37+
module V4CUncurried: {
38+
module FancyInput: {
39+
@react.component
40+
let make: (
41+
~className: string=?,
42+
~children: React.element,
43+
~ref: ReactDOM.Ref.currentDomRef=?,
44+
) => React.element
45+
}
46+
47+
module ForwardRef: {
48+
@react.component
49+
let make: (~ref: React.ref<Js.Nullable.t<inputRef>>=?) => React.element
50+
}
51+
}
52+
53+
@@jsxConfig({version: 4, mode: "automatic"})
54+
55+
module V4A: {
56+
module FancyInput: {
57+
@react.component
58+
let make: (
59+
~className: string=?,
60+
~children: React.element,
61+
~ref: ReactDOM.Ref.currentDomRef=?,
62+
) => React.element
63+
}
64+
65+
module ForwardRef: {
66+
@react.component
67+
let make: (~ref: React.ref<Js.Nullable.t<inputRef>>=?) => React.element
68+
}
69+
}
70+
71+
module V4AUncurried: {
72+
module FancyInput: {
73+
@react.component
74+
let make: (
75+
~className: string=?,
76+
~children: React.element,
77+
~ref: ReactDOM.Ref.currentDomRef=?,
78+
) => React.element
79+
}
80+
81+
module ForwardRef: {
82+
@react.component
83+
let make: (~ref: React.ref<Js.Nullable.t<inputRef>>=?) => React.element
84+
}
85+
}

0 commit comments

Comments
 (0)