Remove key prop from JSX props type

cli/JSXV4.md

@@ -40,7 +40,7 @@ It produces a type definition and a new function.
 
 // is converted to
 
-type props<'x, 'y, 'z> = {x: 'x, @optional y: 'y, @optional z: 'z, @optional key: string}
+type props<'x, 'y, 'z> = {x: 'x, y?: 'y, z?: 'z}
 
 ({x, y, z}: props<_>) => {
   let y = switch props.y {
@@ -60,11 +60,11 @@ React.createElement(Comp.make, {x})
 
 <Comp x y=7 ?z>
 // is converted to
-React.createElement(Comp.make, {x, y:7, @optional z})
+React.createElement(Comp.make, {x, y:7, ?z})
 
 <Comp x key="7">
 // is converted to
-React.createElement(Comp.make, {x, key: "7"})
+React.createElement(Comp.make, Jsx.addKeyProp({x}, "7"))
 ```
 
 **New "jsx" transform**
@@ -76,16 +76,16 @@ The "jsx" transform affects component application but not the definition.
 ```rescript
 <Comp x>
 // is converted to
-Js.React.jsx(Comp.make, {x})
+React.jsx(Comp.make, {x})
 ```
 
 ```rescript
 <div name="div" />
 // is converted to
-Js.React.jsxDom("div", { name: "div" })
+ReactDOM.jsx("div", { name: "div" })
 ```
 
-The props type of dom elements, e.g. `div`, is inferred to `Js.React.domProps`.
+The props type of dom elements, e.g. `div`, is inferred to `ReactDOM.domProps`.
 
 ```rescript
 type domProps = {
@@ -102,7 +102,7 @@ type domProps = {
 
 // is converted to
 
-type props<'x, 'y, 'z> = {x: 'x, @optional y: 'y, @optional z: 'z}
+type props<'x, 'y, 'z> = {x: 'x, y?: 'y, z?: 'z}
 
 props<int, int, int> => React.element
 ```
@@ -121,7 +121,11 @@ function has the name of the enclosing module/file.
 
 // is converted to
 
+// v4
 ReactDOMRe.createElement(ReasonReact.fragment, [comp1, comp2, comp3])
+
+// v4 @ new jsx transform
+React.jsxs(React.jsxFragment, {children: [comp1, comp2, comp3]})
 ```
 
 **File-level config**

cli/reactjs_jsx_ppx.ml

@@ -1607,11 +1607,11 @@ module V4 = struct
         ~kind:(Ptype_record labelDeclList);
     ]
 
-  (* type props<'id, 'name, ...> = { @optional key: string, @optional id: 'id, ... } *)
+  (* type props<'x, 'y, ...> = { x: 'x, y?: 'y, ... } *)
   let makePropsRecordType propsName loc namedTypeList =
     Str.type_ Nonrecursive (makeTypeDecls propsName loc namedTypeList)
 
-  (* type props<'id, 'name, ...> = { @optional key: string, @optional id: 'id, ... } *)
+  (* type props<'x, 'y, ...> = { x: 'x, y?: 'y, ... } *)
   let makePropsRecordTypeSig propsName loc namedTypeList =
     Sig.type_ Nonrecursive (makeTypeDecls propsName loc namedTypeList)
 
@@ -1660,11 +1660,11 @@ module V4 = struct
       first = capped
       [@@raises Invalid_argument]
     in
-    let ident =
+    let ident ~suffix =
       match modulePath with
-      | Lident _ -> Ldot (modulePath, "make")
+      | Lident _ -> Ldot (modulePath, suffix)
       | Ldot (_modulePath, value) as fullPath when isCap value ->
-        Ldot (fullPath, "make")
+        Ldot (fullPath, suffix)
       | modulePath -> modulePath
     in
     let isEmptyRecord {pexp_desc} =
@@ -1675,16 +1675,16 @@ module V4 = struct
 
     (* handle key, ref, children *)
     (* React.createElement(Component.make, props, ...children) *)
+    let record = recordFromProps ~loc:jsxExprLoc ~removeKey:true args in
+    let props =
+      if isEmptyRecord record then emptyRecord ~loc:jsxExprLoc else record
+    in
+    let keyProp =
+      args |> List.filter (fun (arg_label, _) -> "key" = getLabel arg_label)
+    in
     match config.mode with
     (* The new jsx transform *)
     | "automatic" ->
-      let record = recordFromProps ~loc:jsxExprLoc ~removeKey:true args in
-      let props =
-        if isEmptyRecord record then emptyRecord ~loc:jsxExprLoc else record
-      in
-      let keyProp =
-        args |> List.filter (fun (arg_label, _) -> "key" = getLabel arg_label)
-      in
       let jsxExpr, key =
         match (!childrenArg, keyProp) with
         | None, (_, keyExpr) :: _ ->
@@ -1704,37 +1704,58 @@ module V4 = struct
       in
       Exp.apply ~attrs jsxExpr
         ([
-           (nolabel, Exp.ident {txt = ident; loc = callExprLoc});
+           (nolabel, Exp.ident {txt = ident ~suffix:"make"; loc = callExprLoc});
            (nolabel, props);
          ]
         @ key)
     | _ -> (
-      let record = recordFromProps ~loc:jsxExprLoc args in
-      (* check if record which goes to Foo.make({ ... } as record) empty or not
-         if empty then change it to {key: @optional None} only for upper case jsx
-           This would be redundant regarding PR progress https://github.com/rescript-lang/syntax/pull/299
-      *)
-      let props =
-        if isEmptyRecord record then emptyRecord ~loc:jsxExprLoc else record
+      let keyAddedProps ~keyExpr =
+        let propsType =
+          Typ.constr (Location.mknoloc @@ ident ~suffix:"props") [Typ.any ()]
+        in
+        Exp.apply
+          (Exp.ident
+             {loc = Location.none; txt = Ldot (Lident "Jsx", "addKeyProp")})
+          [(nolabel, Exp.constraint_ props propsType); (nolabel, keyExpr)]
       in
-      match !childrenArg with
-      | None ->
+      match (!childrenArg, keyProp) with
+      | None, (_, keyExpr) :: _ ->
+        Exp.apply ~attrs
+          (Exp.ident
+             {loc = Location.none; txt = Ldot (Lident "React", "createElement")})
+          [
+            (nolabel, Exp.ident {txt = ident ~suffix:"make"; loc = callExprLoc});
+            (nolabel, keyAddedProps ~keyExpr);
+          ]
+      | None, [] ->
         Exp.apply ~attrs
           (Exp.ident
              {loc = Location.none; txt = Ldot (Lident "React", "createElement")})
           [
-            (nolabel, Exp.ident {txt = ident; loc = callExprLoc});
+            (nolabel, Exp.ident {txt = ident ~suffix:"make"; loc = callExprLoc});
             (nolabel, props);
           ]
-      | Some children ->
+      | Some children, (_, keyExpr) :: _ ->
+        Exp.apply ~attrs
+          (Exp.ident
+             {
+               loc = Location.none;
+               txt = Ldot (Lident "React", "createElementVariadic");
+             })
+          [
+            (nolabel, Exp.ident {txt = ident ~suffix:"make"; loc = callExprLoc});
+            (nolabel, keyAddedProps ~keyExpr);
+            (nolabel, children);
+          ]
+      | Some children, [] ->
         Exp.apply ~attrs
           (Exp.ident
              {
                loc = Location.none;
                txt = Ldot (Lident "React", "createElementVariadic");
              })
           [
-            (nolabel, Exp.ident {txt = ident; loc = callExprLoc});
+            (nolabel, Exp.ident {txt = ident ~suffix:"make"; loc = callExprLoc});
             (nolabel, props);
             (nolabel, children);
           ])
@@ -2039,10 +2060,9 @@ module V4 = struct
               (Location.mkloc (Lident "props") pstr_loc)
               (makePropsTypeParams namedTypeList)
           in
-          (* type props<'id, 'name> = { @optional key: string, @optional id: 'id, ... } *)
+          (* type props<'x, 'y> = { x: 'x, y?: 'y, ... } *)
           let propsRecordType =
-            makePropsRecordType "props" Location.none
-              ((true, "key", [], keyType pstr_loc) :: namedTypeList)
+            makePropsRecordType "props" Location.none namedTypeList
           in
           (* can't be an arrow because it will defensively uncurry *)
           let newExternalType =
@@ -2274,10 +2294,9 @@ module V4 = struct
             (* type props = { ... } *)
             let propsRecordType =
               makePropsRecordType "props" emptyLoc
-                ([(true, "key", [], keyType emptyLoc)]
-                @ (if hasForwardRef then
-                   [(true, "ref", [], refType Location.none)]
-                  else [])
+                ((if hasForwardRef then
+                  [(true, "ref", [], refType Location.none)]
+                 else [])
                 @ namedTypeList)
             in
             let innerExpression =
@@ -2505,10 +2524,9 @@ module V4 = struct
         in
         let propsRecordType =
           makePropsRecordTypeSig "props" Location.none
-            ([(true, "key", [], keyType Location.none)]
-            (* If there is Nolabel arg, regard the type as ref in forwardRef *)
-            @ (if !hasForwardRef then [(true, "ref", [], refType Location.none)]
-              else [])
+            ((* If there is Nolabel arg, regard the type as ref in forwardRef *)
+             (if !hasForwardRef then [(true, "ref", [], refType Location.none)]
+             else [])
             @ namedTypeList)
         in
         (* can't be an arrow because it will defensively uncurry *)

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

@@ -1,4 +1,4 @@
-type props<'msg> = {key?: string, msg: 'msg} // test React JSX file
+type props<'msg> = {msg: 'msg} // test React JSX file
 
 @react.component
 let make = ({msg, _}: props<'msg>) => {

tests/ppx/react/expected/externalWithCustomName.res.txt

@@ -13,7 +13,7 @@ let t = React.createElement(Foo.component, Foo.componentProps(~a=1, ~b={"1"}, ()
 @@jsxConfig({version: 4, mode: "classic"})
 
 module Foo = {
-  type props<'a, 'b> = {key?: string, a: 'a, b: 'b}
+  type props<'a, 'b> = {a: 'a, b: 'b}
 
   @module("Foo")
   external component: React.componentLike<props<int, string>, React.element> = "component"
@@ -24,7 +24,7 @@ let t = React.createElement(Foo.component, {a: 1, b: "1"})
 @@jsxConfig({version: 4, mode: "automatic"})
 
 module Foo = {
-  type props<'a, 'b> = {key?: string, a: 'a, b: 'b}
+  type props<'a, 'b> = {a: 'a, b: 'b}
 
   @module("Foo")
   external component: React.componentLike<props<int, string>, React.element> = "component"

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

@@ -17,7 +17,7 @@ module V3 = {
 @@jsxConfig({version: 4, mode: "classic"})
 
 module V4C = {
-  type props<'msg> = {key?: string, msg: 'msg}
+  type props<'msg> = {msg: 'msg}
 
   @react.component
   let make = ({msg, _}: props<'msg>) => {
@@ -33,7 +33,7 @@ module V4C = {
 @@jsxConfig({version: 4, mode: "automatic"})
 
 module V4A = {
-  type props<'msg> = {key?: string, msg: 'msg}
+  type props<'msg> = {msg: 'msg}
 
   @react.component
   let make = ({msg, _}: props<'msg>) => {

tests/ppx/react/expected/forwardRef.res.txt

@@ -67,7 +67,6 @@ module V3 = {
 module V4C = {
   module FancyInput = {
     type props<'className, 'children> = {
-      key?: string,
       ref?: ReactDOM.Ref.currentDomRef,
       className?: 'className,
       children: 'children,
@@ -97,7 +96,7 @@ module V4C = {
       \"ForwardRef$V4C$FancyInput"
     })
   }
-  type props = {key?: string}
+  type props = {}
 
   @react.component
   let make = (_: props) => {
@@ -125,7 +124,6 @@ module V4C = {
 module V4A = {
   module FancyInput = {
     type props<'className, 'children> = {
-      key?: string,
       ref?: ReactDOM.Ref.currentDomRef,
       className?: 'className,
       children: 'children,
@@ -155,7 +153,7 @@ module V4A = {
       \"ForwardRef$V4A$FancyInput"
     })
   }
-  type props = {key?: string}
+  type props = {}
 
   @react.component
   let make = (_: props) => {

tests/ppx/react/expected/interface.resi.txt

@@ -0,0 +1,2 @@
+type props<'x> = {x: 'x}
+let make: React.componentLike<props<string>, React.element>

tests/ppx/react/expected/newtype.res.txt

@@ -24,7 +24,7 @@ module V3 = {
 @@jsxConfig({version: 4, mode: "classic"})
 
 module V4C = {
-  type props<'a, 'b, 'c> = {key?: string, a: 'a, b: 'b, c: 'c}
+  type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}
 
   @react.component
   let make = ({a, b, c, _}: props<'\"type-a", array<option<[#Foo('\"type-a")]>>, 'a>) =>
@@ -39,7 +39,7 @@ module V4C = {
 @@jsxConfig({version: 4, mode: "automatic"})
 
 module V4A = {
-  type props<'a, 'b, 'c> = {key?: string, a: 'a, b: 'b, c: 'c}
+  type props<'a, 'b, 'c> = {a: 'a, b: 'b, c: 'c}
 
   @react.component
   let make = ({a, b, c, _}: props<'\"type-a", array<option<[#Foo('\"type-a")]>>, 'a>) =>

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

@@ -0,0 +1,58 @@
+@@jsxConfig({version: 4, mode: "classic"})
+
+module Foo = {
+  type props<'x, 'y> = {x: 'x, y: 'y}
+
+  @react.component let make = ({x, y, _}: props<'x, 'y>) => React.string(x ++ y)
+  let make = {
+    let \"RemovedKeyProp$Foo" = (props: props<_>) => make(props)
+
+    \"RemovedKeyProp$Foo"
+  }
+}
+
+module HasChildren = {
+  type props<'children> = {children: 'children}
+
+  @react.component
+  let make = ({children, _}: props<'children>) =>
+    ReactDOMRe.createElement(ReasonReact.fragment, [children])
+  let make = {
+    let \"RemovedKeyProp$HasChildren" = (props: props<_>) => make(props)
+
+    \"RemovedKeyProp$HasChildren"
+  }
+}
+type props = {}
+
+@react.component
+let make = (_: props) =>
+  ReactDOMRe.createElement(
+    ReasonReact.fragment,
+    [
+      React.createElement(Foo.make, Jsx.addKeyProp(({x: "x", y: "y"}: Foo.props<_>), "k")),
+      React.createElement(Foo.make, {x: "x", y: "y"}),
+      React.createElement(
+        HasChildren.make,
+        Jsx.addKeyProp(
+          (
+            {
+              children: React.createElement(Foo.make, {x: "x", y: "y"}),
+            }: HasChildren.props<_>
+          ),
+          "k",
+        ),
+      ),
+      React.createElement(
+        HasChildren.make,
+        {
+          children: React.createElement(Foo.make, {x: "x", y: "y"}),
+        },
+      ),
+    ],
+  )
+let make = {
+  let \"RemovedKeyProp" = props => make(props)
+
+  \"RemovedKeyProp"
+}