diff --git a/CHANGELOG.md b/CHANGELOG.md index 87a36ca1..5848015d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ - Fix several printing issues with `async` including an infinite loop https://github.com/rescript-lang/syntax/pull/680 - 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 - Fix issue where printing nested pipe discards await https://github.com/rescript-lang/syntax/issues/687 +- Fix issue where the JSX key type is not an optional string https://github.com/rescript-lang/syntax/pull/693 #### :eyeglasses: Spec Compliance diff --git a/cli/JSXV4.md b/cli/JSXV4.md index bffe0181..87876018 100644 --- a/cli/JSXV4.md +++ b/cli/JSXV4.md @@ -11,7 +11,7 @@ To build an entire project in V4 mode, including all its dependencies, use the n "jsx": { "version": 4 } ``` -> 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. +> 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. ## Configuration And Upgrade @@ -286,7 +286,11 @@ React.createElement(Comp.make, {x, y: 7, ?z}) // is transformed to -React.createElement(Comp.make, Jsx.addKeyProp({x: x}, "7")) +React.createElement(Comp.make, React.addKeyProp(~key="7", {x: x})) + + +// is transformed to +React.createElement(Comp.make, React.addKeyProp(~key=?Some("7"), {x: x})) ``` ### New experimental automatic mode diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index e6876391..48483093 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -387,49 +387,42 @@ let transformUppercaseCall3 ~config modulePath mapper jsxExprLoc callExprLoc | "automatic" -> let jsxExpr, key = match (!childrenArg, keyProp) with - | None, (_, keyExpr) :: _ -> - ( Exp.ident - {loc = Location.none; txt = Ldot (Lident "React", "jsxKeyed")}, - [(nolabel, keyExpr)] ) + | None, key :: _ -> + ( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsx")}, + [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)] ) + | Some _, key :: _ -> + ( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsxs")}, + [key] ) | Some _, [] -> ( Exp.ident {loc = Location.none; txt = Ldot (Lident "React", "jsxs")}, [] ) 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 @@ -497,23 +490,21 @@ let transformLowercaseCall3 ~config mapper jsxExprLoc callExprLoc attrs in let jsxExpr, key = match (!childrenArg, keyProp) with - | None, (_, keyExpr) :: _ -> - ( Exp.ident - {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxKeyed")}, - [(nolabel, keyExpr)] ) + | None, key :: _ -> + ( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsx")}, + [key] ) | None, [] -> ( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsx")}, [] ) - | Some _, (_, keyExpr) :: _ -> - ( Exp.ident - {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxsKeyed")}, - [(nolabel, keyExpr)] ) + | Some _, key :: _ -> + ( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxs")}, + [key] ) | Some _, [] -> ( Exp.ident {loc = Location.none; txt = Ldot (Lident "ReactDOM", "jsxs")}, [] ) in Exp.apply ~attrs jsxExpr - ([(nolabel, componentNameExpr); (nolabel, props)] @ key) + (key @ [(nolabel, componentNameExpr); (nolabel, props)]) | _ -> let children, nonChildrenProps = extractChildren ~loc:jsxExprLoc callArguments diff --git a/tests/ppx/react/expected/noPropsWithKey.res.txt b/tests/ppx/react/expected/noPropsWithKey.res.txt index cacabd78..3aebb52d 100644 --- a/tests/ppx/react/expected/noPropsWithKey.res.txt +++ b/tests/ppx/react/expected/noPropsWithKey.res.txt @@ -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 = { @@ -36,3 +36,39 @@ module V4C = { \"NoPropsWithKey$V4C" } } + +@@jsxConfig({version: 4, mode: "automatic"}) + +module V4CA = { + type props = {} + + @react.component let make = (_: props) => ReactDOM.jsx("div", {}) + let make = { + let \"NoPropsWithKey$V4CA" = props => make(props) + + \"NoPropsWithKey$V4CA" + } +} + +module V4CB = { + type props = {} + + @module("c") + external make: React.componentLike = "component" +} + +module V4C = { + type props = {} + + @react.component + let make = (_: props) => + React.jsxs( + React.jsxFragment, + {children: [React.jsx(~key="k", V4CA.make, {}), React.jsx(~key="k", V4CB.make, {})]}, + ) + let make = { + let \"NoPropsWithKey$V4C" = props => make(props) + + \"NoPropsWithKey$V4C" + } +} diff --git a/tests/ppx/react/expected/optionalKeyType.res.txt b/tests/ppx/react/expected/optionalKeyType.res.txt new file mode 100644 index 00000000..137570b7 --- /dev/null +++ b/tests/ppx/react/expected/optionalKeyType.res.txt @@ -0,0 +1,77 @@ +let key = None + +@@jsxConfig({version: 3}) + +let _ = React.createElement(C.make, C.makeProps(~key="k", ())) +let _ = React.createElement(C.make, C.makeProps(~key=?Some("k"), ())) +let _ = React.createElement(C.make, C.makeProps(~key?, ())) +let _ = ReactDOMRe.createDOMElementVariadic("div", ~props=ReactDOMRe.domProps(~key="k", ()), []) +let _ = ReactDOMRe.createDOMElementVariadic( + "div", + ~props=ReactDOMRe.domProps(~key=?Some("k"), ()), + [], +) +let _ = ReactDOMRe.createDOMElementVariadic("div", ~props=ReactDOMRe.domProps(~key?, ()), []) +let _ = ReactDOMRe.createDOMElementVariadic( + "div", + ~props=ReactDOMRe.domProps(~key="k", ()), + [ReactDOMRe.createDOMElementVariadic("br", []), ReactDOMRe.createDOMElementVariadic("br", [])], +) +let _ = ReactDOMRe.createDOMElementVariadic( + "div", + ~props=ReactDOMRe.domProps(~key=?Some("k"), ()), + [ReactDOMRe.createDOMElementVariadic("br", []), ReactDOMRe.createDOMElementVariadic("br", [])], +) +let _ = ReactDOMRe.createDOMElementVariadic( + "div", + ~props=ReactDOMRe.domProps(~key?, ()), + [ReactDOMRe.createDOMElementVariadic("br", []), ReactDOMRe.createDOMElementVariadic("br", [])], +) + +@@jsxConfig({version: 4, mode: "classic"}) + +let _ = React.createElementWithKey(~key="k", C.make, {}) +let _ = React.createElementWithKey(~key=?Some("k"), C.make, {}) +let _ = React.createElementWithKey(~key?, C.make, {}) +let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: "k"}, []) +let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: ?Some("k")}, []) +let _ = ReactDOM.createDOMElementVariadic("div", ~props={key: ?key}, []) +let _ = ReactDOM.createDOMElementVariadic( + "div", + ~props={key: "k"}, + [ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])], +) +let _ = ReactDOM.createDOMElementVariadic( + "div", + ~props={key: ?Some("k")}, + [ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])], +) +let _ = ReactDOM.createDOMElementVariadic( + "div", + ~props={key: ?key}, + [ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])], +) + +@@jsxConfig({version: 4, mode: "automatic"}) + +let _ = React.jsx(~key="k", C.make, {}) +let _ = React.jsx(~key=?Some("k"), C.make, {}) +let _ = React.jsx(~key?, C.make, {}) +let _ = ReactDOM.jsx(~key="k", "div", {}) +let _ = ReactDOM.jsx(~key=?Some("k"), "div", {}) +let _ = ReactDOM.jsx(~key?, "div", {}) +let _ = ReactDOM.jsxs( + ~key="k", + "div", + {children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])}, +) +let _ = ReactDOM.jsxs( + ~key=?Some("k"), + "div", + {children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])}, +) +let _ = ReactDOM.jsxs( + ~key?, + "div", + {children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])}, +) diff --git a/tests/ppx/react/expected/removedKeyProp.res.txt b/tests/ppx/react/expected/removedKeyProp.res.txt index 6f44b028..3b51b231 100644 --- a/tests/ppx/react/expected/removedKeyProp.res.txt +++ b/tests/ppx/react/expected/removedKeyProp.res.txt @@ -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, diff --git a/tests/ppx/react/expected/spreadProps.res.txt b/tests/ppx/react/expected/spreadProps.res.txt index d43f50db..765264ea 100644 --- a/tests/ppx/react/expected/spreadProps.res.txt +++ b/tests/ppx/react/expected/spreadProps.res.txt @@ -11,6 +11,16 @@ let c1 = React.createElement(A.make, p) // reversed order let c2 = React.createElement(A.make, {...p, x: "x"}) +let c3 = ReactDOM.createDOMElementVariadic("div", ~props=p, []) + +let c4 = ReactDOM.createDOMElementVariadic("div", ~props={...p, x: "x", key: "k"}, []) + +let c4 = ReactDOM.createDOMElementVariadic( + "div", + ~props={...p, key: "k"}, + [ReactDOM.createDOMElementVariadic("br", []), ReactDOM.createDOMElementVariadic("br", [])], +) + @@jsxConfig({version: 4, mode: "automatic"}) // Error: spreadProps should be first in order than other props // let c0 = @@ -23,3 +33,13 @@ let c1 = React.jsx(A.make, p) // reversed order let c2 = React.jsx(A.make, {...p, x: "x"}) + +let c3 = ReactDOM.jsx("div", p) + +let c4 = ReactDOM.jsx(~key="k", "div", {...p, x: "x"}) + +let c5 = ReactDOM.jsxs( + ~key="k", + "div", + {...p, children: React.array([ReactDOM.jsx("br", {}), ReactDOM.jsx("br", {})])}, +) diff --git a/tests/ppx/react/noPropsWithKey.res b/tests/ppx/react/noPropsWithKey.res index 779309cc..9a986307 100644 --- a/tests/ppx/react/noPropsWithKey.res +++ b/tests/ppx/react/noPropsWithKey.res @@ -10,6 +10,23 @@ module V4CB = { external make: unit => React.element = "component" } +module V4C = { + @react.component + let make = () => <> +} + +@@jsxConfig({version: 4, mode: "automatic"}) + +module V4CA = { + @react.component + let make = () =>
+} + +module V4CB = { + @module("c") @react.component + external make: unit => React.element = "component" +} + module V4C = { @react.component let make = () => <> diff --git a/tests/ppx/react/optionalKeyType.res b/tests/ppx/react/optionalKeyType.res new file mode 100644 index 00000000..bec2add5 --- /dev/null +++ b/tests/ppx/react/optionalKeyType.res @@ -0,0 +1,37 @@ +let key = None + +@@jsxConfig({version:3}) + +let _ = +let _ = +let _ = +let _ =
+let _ =
+let _ =
+let _ =


+let _ =


+let _ =


+ +@@jsxConfig({version:4, mode: "classic"}) + +let _ = +let _ = +let _ = +let _ =