Port of rescript-lang/rescript#6041

Author: cristianoc

CHANGELOG.md

@@ -66,6 +66,8 @@
 - Fix formatting of props spread for multiline JSX expression https://github.com/rescript-lang/syntax/pull/736
 - Fix issue with JSX V4 and newtype https://github.com/rescript-lang/syntax/pull/737
 - Fix issue with JSX V4 when components are nested https://github.com/rescript-lang/syntax/pull/738
+- Improve code generated for default arguments in JSX V4 https://github.com/rescript-lang/syntax/pull/739
+- Fix issue with JSX V4 props of the form `~p as module(...)` https://github.com/rescript-lang/syntax/pull/739
 
 #### :eyeglasses: Spec Compliance
 

cli/JSXV4.md

@@ -18,7 +18,7 @@ To build an entire project in V4 mode, including all its dependencies, use the n
 ### Dependency-level config
 
 Dependencies inherit the `jsx` configuration of the root project. So if the root project uses V4 then the dependencies are built using V4, and the same for V3.
-To build certain dependencies in V3 compatibility mode, whatever the version used in the root project, use `"v3-dependencies"`: the listed dependencies will be built in V3 mode, and in addition `-open ReatcV3` is added to the compiler options.
+To build certain dependencies in V3 compatibility mode, whatever the version used in the root project, use `"v3-dependencies"`: the listed dependencies will be built in V3 mode, and in addition `-open ReactV3` is added to the compiler options.
 
 For example, suppose a V3 project uses rescript-react 0.11, which requires compatibility mode if compiled with V3, and that 2 dependencies `"rescript-react-native", "rescript-react-navigation"` only build with compatibility mode. Then the setting will be:
 
@@ -264,19 +264,21 @@ let make = React.forwardRef({
 ### Transformation for Component Definition
 
 ```rescript
-@react.component (~x, ~y=3+x, ?z) => body
+@react.component (~x, ~y=3+x, ~z=?) => body
 ```
 
 is transformed to
 
 ```rescript
 type props<'x, 'y, 'z> = {x: 'x, y?: 'y, z?: 'z}
 
-({x, y, z}: props<_>) => {
-  let y = switch props.y {
+({x, ?y, ?z}: props<_, _, _>) => {
+  let x = x
+  let y = switch y {
   | None => 3 + x
   | Some(y) => y
   }
+  let z = z
   body
 }
 ```

cli/reactjs_jsx_v4.ml

@@ -640,6 +640,7 @@ let rec recursivelyTransformNamedArgsForMake expr args newtypes coreType =
     in
     let type_ =
       match pattern with
+      | {ppat_desc = Ppat_constraint (_, {ptyp_desc = Ptyp_package _})} -> None
       | {ppat_desc = Ppat_constraint (_, type_)} -> Some type_
       | _ -> None
     in
@@ -843,39 +844,34 @@ let modifiedBinding ~bindingLoc ~bindingPatLoc ~fnName binding =
   in
   (wrapExpressionWithBinding wrapExpression, hasForwardRef, expression)
 
-let vbMatch (name, default, _, alias, loc, _) =
+let vbMatch ~expr (name, default, _, alias, loc, _) =
   let label = getLabel name in
   match default with
   | Some default ->
-    Vb.mk
-      (Pat.var (Location.mkloc alias loc))
-      (Exp.match_
-         (Exp.field
-            (Exp.ident {txt = Lident "props"; loc = Location.none})
-            (Location.mknoloc @@ Lident label))
-         [
-           Exp.case
-             (Pat.construct
-                (Location.mknoloc @@ Lident "Some")
-                (Some (Pat.var (Location.mknoloc label))))
-             (Exp.ident (Location.mknoloc @@ Lident label));
-           Exp.case
-             (Pat.construct (Location.mknoloc @@ Lident "None") None)
-             default;
-         ])
-  | None ->
-    Vb.mk
-      (Pat.var (Location.mkloc alias loc))
-      (Exp.field
-         (Exp.ident {txt = Lident "props"; loc = Location.none})
-         (Location.mknoloc @@ Lident label))
+    let value_binding =
+      Vb.mk
+        (Pat.var (Location.mkloc alias loc))
+        (Exp.match_
+           (Exp.ident {txt = Lident alias; loc = Location.none})
+           [
+             Exp.case
+               (Pat.construct
+                  (Location.mknoloc @@ Lident "Some")
+                  (Some (Pat.var (Location.mknoloc label))))
+               (Exp.ident (Location.mknoloc @@ Lident label));
+             Exp.case
+               (Pat.construct (Location.mknoloc @@ Lident "None") None)
+               default;
+           ])
+    in
+    Exp.let_ Nonrecursive [value_binding] expr
+  | None -> expr
 
 let vbMatchExpr namedArgList expr =
   let rec aux namedArgList =
     match namedArgList with
     | [] -> expr
-    | [namedArg] -> Exp.let_ Nonrecursive [vbMatch namedArg] expr
-    | namedArg :: rest -> Exp.let_ Nonrecursive [vbMatch namedArg] (aux rest)
+    | namedArg :: rest -> vbMatch namedArg ~expr:(aux rest)
   in
   aux (List.rev namedArgList)
 
@@ -970,11 +966,10 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
     in
     let rec stripConstraintUnpack ~label pattern =
       match pattern with
+      | {ppat_desc = Ppat_constraint (_, {ptyp_desc = Ptyp_package _})} ->
+        pattern
       | {ppat_desc = Ppat_constraint (pattern, _)} ->
         stripConstraintUnpack ~label pattern
-      | {ppat_desc = Ppat_unpack _; ppat_loc} ->
-        (* remove unpack e.g. model: module(T) *)
-        Pat.var ~loc:ppat_loc {txt = label; loc = ppat_loc}
       | _ -> pattern
     in
     let rec returnedExpression patternsWithLabel patternsWithNolabel
@@ -1043,11 +1038,6 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding =
       | [] -> Pat.any ()
       | _ -> Pat.record (List.rev patternsWithLabel) Open
     in
-    let recordPattern =
-      if hasDefaultValue namedArgList then
-        Pat.var {txt = "props"; loc = emptyLoc}
-      else recordPattern
-    in
     let expression =
       Exp.fun_ Nolabel None
         (Pat.constraint_ recordPattern

tests/ppx/react/aliasProps.res

@@ -14,3 +14,30 @@ module C2 = {
   @react.component
   let make = (~foo as bar="") => React.string(bar)
 }
+
+module C3 = {
+  @react.component
+  let make = (~foo as bar="", ~a=bar, ~b) => {
+    React.string(bar ++ a ++ b)
+  }
+}
+
+module C4 = {
+  @react.component
+  let make = (~a as b, ~x=true) => <div> b </div>
+}
+
+module C5 = {
+  @react.component
+  let make = (~a as (x, y), ~z=3) => x + y + z
+}
+
+module C6 = {
+  module type Comp = {
+    @react.component
+    let make: unit => React.element
+  }
+
+  @react.component
+  let make = (~comp as module(Comp: Comp), ~x as (a, b)) => <Comp />
+}

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

@@ -3,9 +3,8 @@
 module C0 = {
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
-  let make = (props: props<_, _>) => {
-    let _ = props.priority
-    let text = switch props.text {
+  let make = ({priority: _, ?text, _}: props<_, _>) => {
+    let text = switch text {
     | Some(text) => text
     | None => "Test"
     }
@@ -22,9 +21,8 @@ module C0 = {
 module C1 = {
   type props<'priority, 'text> = {priority: 'priority, text?: 'text}
 
-  let make = (props: props<_, _>) => {
-    let p = props.priority
-    let text = switch props.text {
+  let make = ({priority: p, ?text, _}: props<_, _>) => {
+    let text = switch text {
     | Some(text) => text
     | None => "Test"
     }
@@ -41,8 +39,8 @@ module C1 = {
 module C2 = {
   type props<'foo> = {foo?: 'foo}
 
-  let make = (props: props<_>) => {
-    let bar = switch props.foo {
+  let make = ({foo: ?bar, _}: props<_>) => {
+    let bar = switch bar {
     | Some(foo) => foo
     | None => ""
     }
@@ -55,3 +53,79 @@ module C2 = {
     \"AliasProps$C2"
   }
 }
+
+module C3 = {
+  type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}
+
+  let make = ({foo: ?bar, ?a, b, _}: props<_, _, _>) => {
+    let bar = switch bar {
+    | Some(foo) => foo
+    | None => ""
+    }
+    let a = switch a {
+    | Some(a) => a
+    | None => bar
+    }
+
+    {
+      React.string(bar ++ a ++ b)
+    }
+  }
+  let make = {
+    let \"AliasProps$C3" = (props: props<_>) => make(props)
+
+    \"AliasProps$C3"
+  }
+}
+
+module C4 = {
+  type props<'a, 'x> = {a: 'a, x?: 'x}
+
+  let make = ({a: b, ?x, _}: props<_, _>) => {
+    let x = switch x {
+    | Some(x) => x
+    | None => true
+    }
+
+    ReactDOM.jsx("div", {children: ?ReactDOM.someElement(b)})
+  }
+  let make = {
+    let \"AliasProps$C4" = (props: props<_>) => make(props)
+
+    \"AliasProps$C4"
+  }
+}
+
+module C5 = {
+  type props<'a, 'z> = {a: 'a, z?: 'z}
+
+  let make = ({a: (x, y), ?z, _}: props<_, _>) => {
+    let z = switch z {
+    | Some(z) => z
+    | None => 3
+    }
+
+    x + y + z
+  }
+  let make = {
+    let \"AliasProps$C5" = (props: props<_>) => make(props)
+
+    \"AliasProps$C5"
+  }
+}
+
+module C6 = {
+  module type Comp = {
+    type props = {}
+
+    let make: React.componentLike<props, React.element>
+  }
+  type props<'comp, 'x> = {comp: 'comp, x: 'x}
+
+  let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>) => React.jsx(Comp.make, {})
+  let make = {
+    let \"AliasProps$C6" = (props: props<_>) => make(props)
+
+    \"AliasProps$C6"
+  }
+}

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

@@ -1,11 +1,11 @@
 module C0 = {
   type props<'a, 'b> = {a?: 'a, b?: 'b}
-  let make = (props: props<_, _>) => {
-    let a = switch props.a {
+  let make = ({?a, ?b, _}: props<_, _>) => {
+    let a = switch a {
     | Some(a) => a
     | None => 2
     }
-    let b = switch props.b {
+    let b = switch b {
     | Some(b) => b
     | None => a * 2
     }
@@ -21,12 +21,11 @@ module C0 = {
 module C1 = {
   type props<'a, 'b> = {a?: 'a, b: 'b}
 
-  let make = (props: props<_, _>) => {
-    let a = switch props.a {
+  let make = ({?a, b, _}: props<_, _>) => {
+    let a = switch a {
     | Some(a) => a
     | None => 2
     }
-    let b = props.b
 
     React.int(a + b)
   }

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

@@ -66,8 +66,8 @@ module Select = {
 
   let make = (
     type a key,
-    {model, selected, onChange, items, _}: props<
-      module(T with type t = a and type key = key),
+    {model: module(T: T with type t = a and type key = key), selected, onChange, items, _}: props<
+      _,
       option<key>,
       option<key> => unit,
       array<a>,

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

@@ -67,7 +67,7 @@ module type T = {
 module V4A2 = {
   type props<'foo> = {foo: 'foo}
 
-  let make = (type a, {foo, _}: props<module(T with type t = a)>) => {
+  let make = (type a, {foo: (foo: module(T with type t = a)), _}: props<_>) => {
     module T = unpack(foo)
     ReactDOM.jsx("div", {})
   }

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

@@ -1,8 +1,8 @@
 @@jsxConfig({version: 4})
 type props<'a> = {a?: 'a}
 
-let make = (props: props<(. unit) => unit>) => {
-  let a = switch props.a {
+let make = ({?a, _}: props<(. unit) => unit>) => {
+  let a = switch a {
   | Some(a) => a
   | None => (. ()) => ()
   }
@@ -28,8 +28,8 @@ func(~callback=(. str, a, b) => {
 module Foo = {
   type props<'callback> = {callback?: 'callback}
 
-  let make = (props: props<(. string, bool, bool) => unit>) => {
-    let callback = switch props.callback {
+  let make = ({?callback, _}: props<(. string, bool, bool) => unit>) => {
+    let callback = switch callback {
     | Some(callback) => callback
     | None => (. _, _, _) => ()
     }