Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Commit eb059d4

Browse files
mununkimununki
authored
Fix react comp key type (#693)
* fix react comp key type * add tests * call renamed jsx binding * add test * add more examples * fix key type of lowercase jsx runtime * add more examples * update jsx v4 spec about key type * fix JSX v4 spec * update changelog * add jsx key type tests
1 parent d64839e commit eb059d4

10 files changed

+228
-33
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
- Fix several printing issues with `async` including an infinite loop https://github.com/rescript-lang/syntax/pull/680
4444
- Fix issue where certain JSX expressions would be formatted differenctly in compiler 10.1.0-rc.1 https://github.com/rescript-lang/syntax/issues/675
4545
- Fix issue where printing nested pipe discards await https://github.com/rescript-lang/syntax/issues/687
46+
- Fix issue where the JSX key type is not an optional string https://github.com/rescript-lang/syntax/pull/693
4647

4748
#### :eyeglasses: Spec Compliance
4849

‎cli/JSXV4.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ To build an entire project in V4 mode, including all its dependencies, use the n
1111
"jsx": { "version": 4 }
1212
```
1313

14-
> Note that JSX V4 requires the rescript compiler 10.1 or higher, and `rescript-react` version `0.11` or higher. In addition, `react` version `18.2` is required.
14+
> Note that JSX V4 requires the rescript compiler 10.1 or higher, and `rescript-react` version `0.11` or higher. In addition, `react` version `18.0` is required.
1515
1616
## Configuration And Upgrade
1717

@@ -286,7 +286,11 @@ React.createElement(Comp.make, {x, y: 7, ?z})
286286
287287
<Comp x key="7">
288288
// is transformed to
289-
React.createElement(Comp.make, Jsx.addKeyProp({x: x}, "7"))
289+
React.createElement(Comp.make, React.addKeyProp(~key="7", {x: x}))
290+
291+
<Comp x key=?Some("7")>
292+
// is transformed to
293+
React.createElement(Comp.make, React.addKeyProp(~key=?Some("7"), {x: x}))
290294
```
291295

292296
### New experimental automatic mode

‎cli/reactjs_jsx_v4.ml

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -387,49 +387,42 @@ let transformUppercaseCall3 ~config modulePath mapper jsxExprLoc callExprLoc
387387
| "automatic" ->
388388
let jsxExpr, key =
389389
match (!childrenArg, keyProp) with
390-
| None, (_, keyExpr) :: _ ->
391-
( Exp.ident
392-
{loc = Location.none; txt = Ldot (Lident "React", "jsxKeyed")},
393-
[(nolabel, keyExpr)] )
390+
| None, key :: _ ->
391+
( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsx")},
392+
[key] )
394393
| None, [] ->
395394
(Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsx")}, [])
396-
| Some _, (_, keyExpr) :: _ ->
397-
( Exp.ident
398-
{loc = Location.none; txt = Ldot (Lident "React", "jsxsKeyed")},
399-
[(nolabel, keyExpr)] )
395+
| Some _, key :: _ ->
396+
( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsxs")},
397+
[key] )
400398
| Some _, [] ->
401399
( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsxs")},
402400
[] )
403401
in
404-
Exp.apply ~attrs jsxExpr ([(nolabel, makeID); (nolabel, props)] @ key)
402+
Exp.apply ~attrs jsxExpr (key @ [(nolabel, makeID); (nolabel, props)])
405403
| _ -> (
406404
match (!childrenArg, keyProp) with
407-
| None, (_, keyExpr) :: _ ->
405+
| None, key :: _ ->
408406
Exp.apply ~attrs
409407
(Exp.ident
410408
{
411409
loc = Location.none;
412410
txt = Ldot (Lident "React", "createElementWithKey");
413411
})
414-
[(nolabel, makeID); (nolabel, props); (nolabel, keyExpr)]
412+
[key; (nolabel, makeID); (nolabel, props)]
415413
| None, [] ->
416414
Exp.apply ~attrs
417415
(Exp.ident
418416
{loc = Location.none; txt = Ldot (Lident "React", "createElement")})
419417
[(nolabel, makeID); (nolabel, props)]
420-
| Some children, (_, keyExpr) :: _ ->
418+
| Some children, key :: _ ->
421419
Exp.apply ~attrs
422420
(Exp.ident
423421
{
424422
loc = Location.none;
425423
txt = Ldot (Lident "React", "createElementVariadicWithKey");
426424
})
427-
[
428-
(nolabel, makeID);
429-
(nolabel, props);
430-
(nolabel, children);
431-
(nolabel, keyExpr);
432-
]
425+
[key; (nolabel, makeID); (nolabel, props); (nolabel, children)]
433426
| Some children, [] ->
434427
Exp.apply ~attrs
435428
(Exp.ident
@@ -497,23 +490,21 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs
497490
in
498491
let jsxExpr, key =
499492
match (!childrenArg, keyProp) with
500-
| None, (_, keyExpr) :: _ ->
501-
( Exp.ident
502-
{loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxKeyed")},
503-
[(nolabel, keyExpr)] )
493+
| None, key :: _ ->
494+
( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsx")},
495+
[key] )
504496
| None, [] ->
505497
( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsx")},
506498
[] )
507-
| Some _, (_, keyExpr) :: _ ->
508-
( Exp.ident
509-
{loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxsKeyed")},
510-
[(nolabel, keyExpr)] )
499+
| Some _, key :: _ ->
500+
( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxs")},
501+
[key] )
511502
| Some _, [] ->
512503
( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxs")},
513504
[] )
514505
in
515506
Exp.apply ~attrs jsxExpr
516-
([(nolabel, componentNameExpr); (nolabel, props)] @ key)
507+
(key @ [(nolabel, componentNameExpr); (nolabel, props)])
517508
| _ ->
518509
let children, nonChildrenProps =
519510
extractChildren ~loc:jsxExprLoc callArguments

‎tests/ppx/react/expected/noPropsWithKey.res.txt

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ module V4C = {
2626
ReactDOM.createElement(
2727
React.fragment,
2828
[
29-
React.createElementWithKey(V4CA.make, {}, "k"),
30-
React.createElementWithKey(V4CB.make, {}, "k"),
29+
React.createElementWithKey(~key="k", V4CA.make, {}),
30+
React.createElementWithKey(~key="k", V4CB.make, {}),
3131
],
3232
)
3333
let make = {
@@ -36,3 +36,39 @@ module V4C = {
3636
\"NoPropsWithKey$V4C"
3737
}
3838
}
39+
40+
@@jsxConfig({version: 4, mode: "automatic"})
41+
42+
module V4CA = {
43+
type props = {}
44+
45+
@react.component let make = (_: props) => ReactDOM.jsx("div", {})
46+
let make = {
47+
let \"NoPropsWithKey$V4CA" = props => make(props)
48+
49+
\"NoPropsWithKey$V4CA"
50+
}
51+
}
52+
53+
module V4CB = {
54+
type props = {}
55+
56+
@module("c")
57+
external make: React.componentLike<props, React.element> = "component"
58+
}
59+
60+
module V4C = {
61+
type props = {}
62+
63+
@react.component
64+
let make = (_: props) =>
65+
React.jsxs(
66+
React.jsxFragment,
67+
{children: [React.jsx(~key="k", V4CA.make, {}), React.jsx(~key="k", V4CB.make, {})]},
68+
)
69+
let make = {
70+
let \"NoPropsWithKey$V4C" = props => make(props)
71+
72+
\"NoPropsWithKey$V4C"
73+
}
74+
}

‎tests/ppx/react/expected/optionalKeyType.res.txt

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
let key = None
2+
3+
@@jsxConfig({version: 3})
4+
5+
let _ = React.createElement(C.make, C.makeProps(~key="k", ()))
6+
let _ = React.createElement(C.make, C.makeProps(~key=?Some("k"), ()))
7+
let _ = React.createElement(C.make, C.makeProps(~key?, ()))
8+
let _ = ReactDOMRe.createDOMElementVariadic("div", ~props=ReactDOMRe.domProps(~key="k", ()), [])
9+
let _ = ReactDOMRe.createDOMElementVariadic(
10+
"div",
11+
~props=ReactDOMRe.domProps(~key=?Some("k"), ()),
12+
[],
13+
)
14+
let _ = ReactDOMRe.createDOMElementVariadic("div", ~props=ReactDOMRe.domProps(~key?, ()), [])
15+
let _ = ReactDOMRe.createDOMElementVariadic(
16+
"div",
17+
~props=ReactDOMRe.domProps(~key="k", ()),
18+
[ReactDOMRe.createDOMElementVariadic("br", []), ReactDOMRe.createDOMElementVariadic("br", [])],
19+
)
20+
let _ = ReactDOMRe.createDOMElementVariadic(
21+
"div",
22+
~props=ReactDOMRe.domProps(~key=?Some("k"), ()),
23+
[ReactDOMRe.createDOMElementVariadic("br", []), ReactDOMRe.createDOMElementVariadic("br", [])],
24+
)
25+
let _ = ReactDOMRe.createDOMElementVariadic(
26+
"div",
27+
~props=ReactDOMRe.domProps(~key?, ()),
28+
[ReactDOMRe.createDOMElementVariadic("br", []), ReactDOMRe.createDOMElementVariadic("br", [])],
29+
)
30+
31+
@@jsxConfig({version: 4, mode: "classic"})
32+
33+
let _ = React.createElementWithKey(~key="k", C.make, {})
34+
let _ = React.createElementWithKey(~key=?Some("k"), C.make, {})
35+
let _ = React.createElementWithKey(~key?, C.make, {})
36+
let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: "k"}, [])
37+
let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: ?Some("k")}, [])
38+
let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: ?key}, [])
39+
let _ = ReactDOM.createDOMElementVariadic(
40+
"div",
41+
~props={key: "k"},
42+
[ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])],
43+
)
44+
let _ = ReactDOM.createDOMElementVariadic(
45+
"div",
46+
~props={key: ?Some("k")},
47+
[ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])],
48+
)
49+
let _ = ReactDOM.createDOMElementVariadic(
50+
"div",
51+
~props={key: ?key},
52+
[ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])],
53+
)
54+
55+
@@jsxConfig({version: 4, mode: "automatic"})
56+
57+
let _ = React.jsx(~key="k", C.make, {})
58+
let _ = React.jsx(~key=?Some("k"), C.make, {})
59+
let _ = React.jsx(~key?, C.make, {})
60+
let _ = ReactDOM.jsx(~key="k", "div", {})
61+
let _ = ReactDOM.jsx(~key=?Some("k"), "div", {})
62+
let _ = ReactDOM.jsx(~key?, "div", {})
63+
let _ = ReactDOM.jsxs(
64+
~key="k",
65+
"div",
66+
{children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])},
67+
)
68+
let _ = ReactDOM.jsxs(
69+
~key=?Some("k"),
70+
"div",
71+
{children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])},
72+
)
73+
let _ = ReactDOM.jsxs(
74+
~key?,
75+
"div",
76+
{children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])},
77+
)

‎tests/ppx/react/expected/removedKeyProp.res.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ let make = (_: props) =>
3434
ReactDOM.createElement(
3535
React.fragment,
3636
[
37-
React.createElementWithKey(Foo.make, {x: "x", y: "y"}, "k"),
37+
React.createElementWithKey(~key="k", Foo.make, {x: "x", y: "y"}),
3838
React.createElement(Foo.make, {x: "x", y: "y"}),
3939
React.createElementWithKey(
40+
~key="k",
4041
HasChildren.make,
4142
{
4243
children: React.createElement(Foo.make, {x: "x", y: "y"}),
4344
},
44-
"k",
4545
),
4646
React.createElement(
4747
HasChildren.make,

‎tests/ppx/react/expected/spreadProps.res.txt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,16 @@ let c1 = React.createElement(A.make, p)
1111
// reversed order
1212
let c2 = React.createElement(A.make, {...p, x: "x"})
1313

14+
let c3 = ReactDOM.createDOMElementVariadic("div", ~props=p, [])
15+
16+
let c4 = ReactDOM.createDOMElementVariadic("div", ~props={...p, x: "x", key: "k"}, [])
17+
18+
let c4 = ReactDOM.createDOMElementVariadic(
19+
"div",
20+
~props={...p, key: "k"},
21+
[ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])],
22+
)
23+
1424
@@jsxConfig({version: 4, mode: "automatic"})
1525
// Error: spreadProps should be first in order than other props
1626
// let c0 = <A x="x" {...p} />
@@ -23,3 +33,13 @@ let c1 = React.jsx(A.make, p)
2333

2434
// reversed order
2535
let c2 = React.jsx(A.make, {...p, x: "x"})
36+
37+
let c3 = ReactDOM.jsx("div", p)
38+
39+
let c4 = ReactDOM.jsx(~key="k", "div", {...p, x: "x"})
40+
41+
let c5 = ReactDOM.jsxs(
42+
~key="k",
43+
"div",
44+
{...p, children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])},
45+
)

‎tests/ppx/react/noPropsWithKey.res

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,23 @@ module V4CB = {
1010
external make: unit => React.element = "component"
1111
}
1212

13+
module V4C = {
14+
@react.component
15+
let make = () => <><V4CA key="k" /> <V4CB key="k" /></>
16+
}
17+
18+
@@jsxConfig({version: 4, mode: "automatic"})
19+
20+
module V4CA = {
21+
@react.component
22+
let make = () => <div />
23+
}
24+
25+
module V4CB = {
26+
@module("c") @react.component
27+
external make: unit => React.element = "component"
28+
}
29+
1330
module V4C = {
1431
@react.component
1532
let make = () => <><V4CA key="k" /> <V4CB key="k" /></>

‎tests/ppx/react/optionalKeyType.res

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
let key = None
2+
3+
@@jsxConfig({version:3})
4+
5+
let _ = <C key="k" />
6+
let _ = <C key=?Some("k") />
7+
let _ = <C ?key />
8+
let _ = <div key="k" />
9+
let _ = <div key=?Some("k") />
10+
let _ = <div ?key />
11+
let _ = <div key="k"> <br /><br /> </div>
12+
let _ = <div key=?Some("k")> <br /><br /> </div>
13+
let _ = <div ?key> <br /><br /> </div>
14+
15+
@@jsxConfig({version:4, mode: "classic"})
16+
17+
let _ = <C key="k" />
18+
let _ = <C key=?Some("k") />
19+
let _ = <C ?key />
20+
let _ = <div key="k" />
21+
let _ = <div key=?Some("k") />
22+
let _ = <div ?key />
23+
let _ = <div key="k"> <br /><br /> </div>
24+
let _ = <div key=?Some("k")> <br /><br /> </div>
25+
let _ = <div ?key> <br /><br /> </div>
26+
27+
@@jsxConfig({version:4, mode: "automatic"})
28+
29+
let _ = <C key="k" />
30+
let _ = <C key=?Some("k") />
31+
let _ = <C ?key />
32+
let _ = <div key="k" />
33+
let _ = <div key=?Some("k") />
34+
let _ = <div ?key />
35+
let _ = <div key="k"> <br /><br /> </div>
36+
let _ = <div key=?Some("k")> <br /><br /> </div>
37+
let _ = <div ?key> <br /><br /> </div>

‎tests/ppx/react/spreadProps.res

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ let c1 = <A {...p} />
1111
// reversed order
1212
let c2 = <A {...p} x="x" />
1313

14+
let c3 = <div {...p} />
15+
16+
let c4 = <div {...p} x="x" key="k" />
17+
18+
let c4 = <div {...p} key="k"><br /><br /></div>
19+
1420
@@jsxConfig({version:4, mode: "automatic"})
1521
// Error: spreadProps should be first in order than other props
1622
// let c0 = <A x="x" {...p} />
@@ -23,3 +29,9 @@ let c1 = <A {...p} />
2329

2430
// reversed order
2531
let c2 = <A {...p} x="x" />
32+
33+
let c3 = <div {...p} />
34+
35+
let c4 = <div {...p} x="x" key="k" />
36+
37+
let c5 = <div {...p} key="k"><br /><br /></div>

0 commit comments

Comments
 (0)