Fix issue with JSX V4 and newtype

Make the following changes:
1) Don't replace type variable `a` with `"type-a`",  but leave it alone.
2) If the original `make` uses `(type a b, ...` wrap the first generated `make` with the same new type definitions
3) In the constraint `:props<'a, 'b>` where `'a` and `'b` were not constrained, use `:props<_, _>` instead. This avoids issues when e.g. `'a` is inferred to be a newtype bound in the make function, which would need to be mentioned explicitly and would give a type error.

Fixes #5978
Replaces #6000

Author: cristianoc

CHANGELOG.md

@@ -73,6 +73,7 @@ These are only breaking changes for unformatted code.
 - Fix formatting of `switch` expressions that contain brace `cases` inside https://github.com/rescript-lang/rescript-compiler/pull/6015
 - Support `@gentype.import` as an alias to `@genType.import` in the compiler https://github.com/rescript-lang/rescript-compiler/pull/6020
 - Fix issue with integer overflow check https://github.com/rescript-lang/rescript-compiler/pull/6028
+- Fix issue with JSX V4 and newtype https://github.com/rescript-lang/rescript-compiler/pull/6029
 
 #### :nail_care: Polish
 

jscomp/test/build.ninja

@@ -0,0 +1,40 @@
+'use strict';
+
+
+function Jsxv4_newtype$V4A(props) {
+  return null;
+}
+
+var V4A = {
+  make: Jsxv4_newtype$V4A
+};
+
+function Jsxv4_newtype$V4A1(props) {
+  return null;
+}
+
+var V4A1 = {
+  make: Jsxv4_newtype$V4A1
+};
+
+function Jsxv4_newtype$V4A2(props) {
+  return null;
+}
+
+var V4A2 = {
+  make: Jsxv4_newtype$V4A2
+};
+
+function Jsxv4_newtype$V4A3(props) {
+  return props.foo;
+}
+
+var V4A3 = {
+  make: Jsxv4_newtype$V4A3
+};
+
+exports.V4A = V4A;
+exports.V4A1 = V4A1;
+exports.V4A2 = V4A2;
+exports.V4A3 = V4A3;
+/* No side effect */

jscomp/test/jsxv4_newtype.js

@@ -0,0 +1,33 @@
+@@bs.config({
+   flags: ["-bs-jsx", "4"],
+ })
+
+module V4A = {
+  @react.component
+  let make = (type a, ~a: a, ~b: array<option<[#Foo(a)]>>, ~c: 'a, _) => React.null
+}
+
+module V4A1 = {
+  @react.component
+  let make = (type x y, ~a:x, ~b:array<y>, ~c:'a) => React.null
+}
+
+module type T = {
+  type t
+}
+
+module V4A2 = {
+  @react.component
+  let make = (type a, ~foo: module(T with type t = a)) => {
+    module T = unpack(foo)
+    React.null
+  }
+}
+
+module V4A3 = {
+  @react.component
+  let make = (type a, ~foo) => {
+    module T = unpack(foo: T with type t = a)
+    foo
+  }
+}

jscomp/test/jsxv4_newtype.res

@@ -277,7 +277,7 @@ let makePropsTypeParams ?(stripExplicitOption = false)
                 For example, if JSX ppx is used for React Native, type would be different.
              *)
            match interiorType with
-           | {ptyp_desc = Ptyp_var "ref"} -> Some (refType loc)
+           | {ptyp_desc = Ptyp_any} -> Some (refType loc)
            | _ ->
              (* Strip explicit Js.Nullable.t in case of forwardRef *)
              if stripExplicitJsNullableOfRef then stripJsNullable interiorType
@@ -687,46 +687,18 @@ let rec recursivelyTransformNamedArgsForMake mapper expr args newtypes coreType
       (Some coreType)
   | _ -> (args, newtypes, coreType)
 
-let newtypeToVar newtype type_ =
-  let var_desc = Ptyp_var ("type-" ^ newtype) in
-  let typ (mapper : Ast_mapper.mapper) typ =
-    match typ.ptyp_desc with
-    | Ptyp_constr ({txt = Lident name}, _) when name = newtype ->
-      {typ with ptyp_desc = var_desc}
-    | _ -> Ast_mapper.default_mapper.typ mapper typ
-  in
-  let mapper = {Ast_mapper.default_mapper with typ} in
-  mapper.typ mapper type_
-
-let argToType ~newtypes ~(typeConstraints : core_type option) types
+let argToType types
     ((name, default, {ppat_attributes = attrs}, _alias, loc, type_) :
       arg_label * expression option * pattern * label * 'loc * core_type option)
     =
-  let rec getType name coreType =
-    match coreType with
-    | {ptyp_desc = Ptyp_arrow (arg, c1, c2)} ->
-      if name = arg then Some c1 else getType name c2
-    | _ -> None
-  in
-  let typeConst = Option.bind typeConstraints (getType name) in
-  let type_ =
-    List.fold_left
-      (fun type_ newtype ->
-        match (type_, typeConst) with
-        | _, Some typ | Some typ, None -> Some (newtypeToVar newtype.txt typ)
-        | _ -> None)
-      type_ newtypes
-  in
   match (type_, name, default) with
   | Some type_, name, _ when isOptional name ->
     (true, getLabel name, attrs, loc, type_) :: types
   | Some type_, name, _ -> (false, getLabel name, attrs, loc, type_) :: types
   | None, name, _ when isOptional name ->
-    (true, getLabel name, attrs, loc, Typ.var ~loc (safeTypeFromValue name))
-    :: types
+    (true, getLabel name, attrs, loc, Typ.any ~loc ()) :: types
   | None, name, _ when isLabelled name ->
-    (false, getLabel name, attrs, loc, Typ.var ~loc (safeTypeFromValue name))
-    :: types
+    (false, getLabel name, attrs, loc, Typ.any ~loc ()) :: types
   | _ -> types
 
 let hasDefaultValue nameArgList =
@@ -1027,16 +999,12 @@ let transformStructureItem ~config mapper item =
             modifiedBinding binding
           in
           (* do stuff here! *)
-          let namedArgList, newtypes, typeConstraints =
+          let namedArgList, newtypes, _typeConstraints =
             recursivelyTransformNamedArgsForMake mapper
               (modifiedBindingOld binding)
               [] [] None
           in
-          let namedTypeList =
-            List.fold_left
-              (argToType ~newtypes ~typeConstraints)
-              [] namedArgList
-          in
+          let namedTypeList = List.fold_left argToType [] namedArgList in
           let vbMatch (name, default, _, alias, loc, _) =
             let label = getLabel name in
             match default with
@@ -1229,6 +1197,13 @@ let transformStructureItem ~config mapper item =
                       | _ -> [Typ.any ()]))))
               expression
           in
+          let expression =
+            (* Add new tupes (type a,b,c) to make's definition *)
+            newtypes
+            |> List.fold_left
+                 (fun e newtype -> Exp.newtype newtype e)
+                 expression
+          in
           (* let make = ({id, name, ...}: props<'id, 'name, ...>) => { ... } *)
           let bindings, newBinding =
             match recFlag with

res_syntax/src/reactjs_jsx_v4.ml

@@ -4,7 +4,7 @@ module C0 = {
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
   @react.component
-  let make = (props: props<'priority, 'text>) => {
+  let make = (props: props<_, _>) => {
     let _ = props.priority
     let text = switch props.text {
     | Some(text) => text
@@ -24,7 +24,7 @@ module C1 = {
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
   @react.component
-  let make = (props: props<'priority, 'text>) => {
+  let make = (props: props<_, _>) => {
     let p = props.priority
     let text = switch props.text {
     | Some(text) => text
@@ -44,7 +44,7 @@ module C2 = {
   type props<'foo> = {foo?: 'foo}
 
   @react.component
-  let make = (props: props<'foo>) => {
+  let make = (props: props<_>) => {
     let bar = switch props.foo {
     | Some(foo) => foo
     | None => ""

res_syntax/tests/ppx/react/expected/aliasProps.res.txt

@@ -1,7 +1,7 @@
 type props<'msg> = {msg: 'msg} // test React JSX file
 
 @react.component
-let make = ({msg, _}: props<'msg>) => {
+let make = ({msg, _}: props<_>) => {
   ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
 }
 let make = {

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

@@ -1,7 +1,7 @@
 module C0 = {
   type props<'a, 'b> = {a?: 'a, b?: 'b}
   @react.component
-  let make = (props: props<'a, 'b>) => {
+  let make = (props: props<_, _>) => {
     let a = switch props.a {
     | Some(a) => a
     | None => 2
@@ -23,7 +23,7 @@ module C1 = {
   type props<'a, 'b> = {a?: 'a, b: 'b}
 
   @react.component
-  let make = (props: props<'a, 'b>) => {
+  let make = (props: props<_, _>) => {
     let a = switch props.a {
     | Some(a) => a
     | None => 2

res_syntax/tests/ppx/react/expected/defaultValueProp.res.txt

@@ -21,7 +21,7 @@ module V4C = {
   type props<'msg> = {msg: 'msg}
 
   @react.component
-  let make = ({msg, _}: props<'msg>) => {
+  let make = ({msg, _}: props<_>) => {
     ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
   }
   let make = {
@@ -37,7 +37,7 @@ module V4A = {
   type props<'msg> = {msg: 'msg}
 
   @react.component
-  let make = ({msg, _}: props<'msg>) => {
+  let make = ({msg, _}: props<_>) => {
     ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
   }
   let make = {

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

@@ -66,11 +66,12 @@ module Select = {
 
   @react.component
   let make = (
+    type a key,
     {model, selected, onChange, items, _}: props<
-      module(T with type t = '\"type-a" and type key = '\"type-key"),
-      option<'\"type-key">,
-      option<'\"type-key"> => unit,
-      array<'\"type-a">,
+      module(T with type t = a and type key = key),
+      option<key>,
+      option<key> => unit,
+      array<a>,
     >,
   ) => {
     let _ = (model, selected, onChange, items)

res_syntax/tests/ppx/react/expected/firstClassModules.res.txt

@@ -77,7 +77,7 @@ module V4C = {
 
     @react.component
     let make = (
-      {?className, children, _}: props<'className, 'children, ReactRef.currentDomRef>,
+      {?className, children, _}: props<_, _, ReactRef.currentDomRef>,
       ref: Js.Nullable.t<ReactRef.currentDomRef>,
     ) =>
       ReactDOM.createDOMElementVariadic(
@@ -134,7 +134,7 @@ module V4CUncurried = {
 
     @react.component
     let make = (
-      {?className, children, _}: props<'className, 'children, ReactRef.currentDomRef>,
+      {?className, children, _}: props<_, _, ReactRef.currentDomRef>,
       ref: Js.Nullable.t<ReactRef.currentDomRef>,
     ) =>
       ReactDOM.createDOMElementVariadic(
@@ -192,10 +192,7 @@ module V4A = {
     }
 
     @react.component
-    let make = (
-      {?className, children, _}: props<'className, 'children, ReactDOM.Ref.currentDomRef>,
-      ref,
-    ) =>
+    let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
       ReactDOM.jsxs(
         "div",
         {
@@ -249,10 +246,7 @@ module V4AUncurried = {
     }
 
     @react.component
-    let make = (
-      {?className, children, _}: props<'className, 'children, ReactDOM.Ref.currentDomRef>,
-      ref,
-    ) =>
+    let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) =>
       ReactDOM.jsxs(
         "div",
         {

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

@@ -1,6 +1,6 @@
 module A = {
   type props<'x> = {x: 'x}
-  @react.component let make = ({x, _}: props<'x>) => React.string(x)
+  @react.component let make = ({x, _}: props<_>) => React.string(x)
   let make = {
     let \"Interface$A" = (props: props<_>) => make(props)
     \"Interface$A"

res_syntax/tests/ppx/react/expected/interface.res.txt

@@ -23,8 +23,7 @@ module C4C0 = {
   type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}
 
   @react.component
-  let make = ({@as("open") _open, @as("type") _type, _}: props<'T_open, string>) =>
-    React.string(_open)
+  let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open)
   let make = {
     let \"MangleKeyword$C4C0" = (props: props<_>) => make(props)
 
@@ -46,8 +45,7 @@ module C4A0 = {
   type props<'T_open, 'T_type> = {@as("open") _open: 'T_open, @as("type") _type: 'T_type}
 
   @react.component
-  let make = ({@as("open") _open, @as("type") _type, _}: props<'T_open, string>) =>
-    React.string(_open)
+  let make = ({@as("open") _open, @as("type") _type, _}: props<_, string>) => React.string(_open)
   let make = {
     let \"MangleKeyword$C4A0" = (props: props<_>) => make(props)