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

Fix react comp key type #693

Merged
merged 11 commits into from
Oct 23, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 16 additions & 18 deletions cli/reactjs_jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -387,49 +387,47 @@ let transformUppercaseCall3 ~config modulePath mapper jsxExprLoc callExprLoc
| "automatic" ->
let jsxExpr, key =
match (!childrenArg, keyProp) with
| None, (_, keyExpr) :: _ ->
| None, key :: _ ->
( Exp.ident
{loc = Location.none; txt = Ldot (Lident "React", "jsxKeyed")},
[(nolabel, keyExpr)] )
{loc = Location.none; txt = Ldot (Lident "React", "jsxWithKey")},
[key] )
| None, [] ->
(Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsx")}, [])
| Some _, (_, keyExpr) :: _ ->
( Exp.ident
{loc = Location.none; txt = Ldot (Lident "React", "jsxsKeyed")},
[(nolabel, keyExpr)] )
{loc = Location.none; txt = Ldot (Lident "React", "jsxWithKey")},
[] )
| Some _, key :: _ ->
( Exp.ident
{loc = Location.none; txt = Ldot (Lident "React", "jsxsWithKey")},
[key] )
| Some _, [] ->
( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsxs")},
( Exp.ident
{loc = Location.none; txt = Ldot (Lident "React", "jsxsWithKey")},
[] )
in
Exp.apply ~attrs jsxExpr ([(nolabel, makeID); (nolabel, props)] @ key)
Exp.apply ~attrs jsxExpr (key @ [(nolabel, makeID); (nolabel, props)])
| _ -> (
match (!childrenArg, keyProp) with
| None, (_, keyExpr) :: _ ->
| None, key :: _ ->
Exp.apply ~attrs
(Exp.ident
{
loc = Location.none;
txt = Ldot (Lident "React", "createElementWithKey");
})
[(nolabel, makeID); (nolabel, props); (nolabel, keyExpr)]
[key; (nolabel, makeID); (nolabel, props)]
| None, [] ->
Exp.apply ~attrs
(Exp.ident
{loc = Location.none; txt = Ldot (Lident "React", "createElement")})
[(nolabel, makeID); (nolabel, props)]
| Some children, (_, keyExpr) :: _ ->
| Some children, key :: _ ->
Exp.apply ~attrs
(Exp.ident
{
loc = Location.none;
txt = Ldot (Lident "React", "createElementVariadicWithKey");
})
[
(nolabel, makeID);
(nolabel, props);
(nolabel, children);
(nolabel, keyExpr);
]
[key; (nolabel, makeID); (nolabel, props); (nolabel, children)]
| Some children, [] ->
Exp.apply ~attrs
(Exp.ident
Expand Down
2 changes: 1 addition & 1 deletion tests/ppx/react/expected/externalWithCustomName.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ module Foo = {
external component: React.componentLike<props<int, string>, React.element> = "component"
}

let t = React.jsx(Foo.component, {a: 1, b: "1"})
let t = React.jsxWithKey(Foo.component, {a: 1, b: "1"})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why jsxWithKey if the key is not passed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 changes: 4 additions & 1 deletion tests/ppx/react/expected/forwardRef.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ module V4A = {
"div",
{
children: ?ReactDOM.someElement(
React.jsx(FancyInput.make, {ref: input, children: {React.string("Click to focus")}}),
React.jsxWithKey(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why jsxWithKey if the key is not passed?

FancyInput.make,
{ref: input, children: {React.string("Click to focus")}},
),
),
},
)
Expand Down
4 changes: 2 additions & 2 deletions tests/ppx/react/expected/noPropsWithKey.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ module V4C = {
ReactDOM.createElement(
React.fragment,
[
React.createElementWithKey(V4CA.make, {}, "k"),
React.createElementWithKey(V4CB.make, {}, "k"),
React.createElementWithKey(~key="k", V4CA.make, {}),
React.createElementWithKey(~key="k", V4CB.make, {}),
],
)
let make = {
Expand Down
4 changes: 2 additions & 2 deletions tests/ppx/react/expected/removedKeyProp.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ let make = (_: props) =>
ReactDOM.createElement(
React.fragment,
[
React.createElementWithKey(Foo.make, {x: "x", y: "y"}, "k"),
React.createElementWithKey(~key="k", Foo.make, {x: "x", y: "y"}),
React.createElement(Foo.make, {x: "x", y: "y"}),
React.createElementWithKey(
~key="k",
HasChildren.make,
{
children: React.createElement(Foo.make, {x: "x", y: "y"}),
},
"k",
),
React.createElement(
HasChildren.make,
Expand Down
4 changes: 2 additions & 2 deletions tests/ppx/react/expected/spreadProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ let c2 = React.createElement(A.make, {...p, x: "x"})
// let c0 = <A x="x" {...p0} {...p1} />

// only spread props
let c1 = React.jsx(A.make, p)
let c1 = React.jsxWithKey(A.make, p)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why jsxWithKey is the key is not passed?


// reversed order
let c2 = React.jsx(A.make, {...p, x: "x"})
let c2 = React.jsxWithKey(A.make, {...p, x: "x"})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why jsxWithKey is the key is not passed?