Fix unpacking first-class module in default argument of react component (#8296)

Author: cknitt

CHANGELOG.md

@@ -28,6 +28,7 @@
 - Reanalyze server: invalidate cache and recompute results when config changes in `rescript.json`. https://github.com/rescript-lang/rescript/pull/8262
 - Fix `null` and array values incorrectly matching the `Object` branch when pattern matching on `JSON.t` (or other untagged variants with an `Object` case) in statement position. https://github.com/rescript-lang/rescript/pull/8279
 - Fix rewatch panic when `package.json` has no `name` field. https://github.com/rescript-lang/rescript/pull/8291
+- Fix unpacking first-class module in default argument of react component. https://github.com/rescript-lang/rescript/pull/8296
 
 #### :memo: Documentation
 

compiler/syntax/src/jsx_v4.ml

@@ -498,15 +498,23 @@ let modified_binding ~binding_loc ~binding_pat_loc ~fn_name binding =
   in
   (wrap_expression_with_binding wrap_expression, has_forward_ref, expression)
 
-let vb_match ~expr (name, default, _, alias, loc, _) =
+let rec strip_constraint_unpack pattern =
+  match pattern with
+  | {ppat_desc = Ppat_constraint (_, {ptyp_desc = Ptyp_package _})} -> pattern
+  | {ppat_desc = Ppat_constraint (pattern, _)} ->
+    strip_constraint_unpack pattern
+  | _ -> pattern
+
+let vb_match ~expr (name, default, pattern, _alias, loc, _) =
   let label = get_label name in
   match default with
   | Some default ->
+    let resolved_name = "__" ^ label ^ "_value" in
     let value_binding =
       Vb.mk
-        (Pat.var (Location.mkloc alias loc))
+        (Pat.var (Location.mkloc resolved_name loc))
         (Exp.match_
-           (Exp.ident {txt = Lident ("__" ^ alias); loc = Location.none})
+           (Exp.ident {txt = Lident ("__" ^ label); loc = Location.none})
            [
              Exp.case
                (Pat.construct
@@ -518,7 +526,10 @@ let vb_match ~expr (name, default, _, alias, loc, _) =
                default;
            ])
     in
-    Exp.let_ Nonrecursive [value_binding] expr
+    Exp.let_ Nonrecursive [value_binding]
+      (Exp.let_ Nonrecursive
+         [Vb.mk pattern (Exp.ident (Location.mknoloc @@ Lident resolved_name))]
+         expr)
   | None -> expr
 
 let vb_match_expr named_arg_list expr =
@@ -652,22 +663,6 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
           ]
           (Exp.ident ~loc:pstr_loc {loc = empty_loc; txt = Lident txt})
     in
-    let rec strip_constraint_unpack ~label pattern =
-      match pattern with
-      | {ppat_desc = Ppat_constraint (_, {ptyp_desc = Ptyp_package _})} ->
-        pattern
-      | {ppat_desc = Ppat_constraint (pattern, _)} ->
-        strip_constraint_unpack ~label pattern
-      | _ -> pattern
-    in
-    let safe_pattern_label pattern =
-      match pattern with
-      | {ppat_desc = Ppat_var {txt; loc}} ->
-        {pattern with ppat_desc = Ppat_var {txt = "__" ^ txt; loc}}
-      | {ppat_desc = Ppat_alias (p, {txt; loc})} ->
-        {pattern with ppat_desc = Ppat_alias (p, {txt = "__" ^ txt; loc})}
-      | _ -> pattern
-    in
     let rec returned_expression patterns_with_label patterns_with_nolabel
         ({pexp_desc} as expr) =
       match pexp_desc with
@@ -688,17 +683,20 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
             lhs = {ppat_loc; ppat_desc} as pattern;
             rhs = expr;
           } -> (
-        let pattern_without_constraint =
-          strip_constraint_unpack ~label:(get_label arg_label) pattern
-        in
+        let pattern_without_constraint = strip_constraint_unpack pattern in
         (*
            If prop has the default value as Ident, it will get a build error
            when the referenced Ident value and the prop have the same name.
-           So we add a "__" to label to resolve the build error.
+           So we bind a temp "__<label>" in the props pattern and destructure
+           the original pattern later after resolving the default.
         *)
         let pattern_with_safe_label =
           match default with
-          | Some _ -> safe_pattern_label pattern_without_constraint
+          | Some _ ->
+            Pat.var
+              (Location.mkloc
+                 ("__" ^ get_label arg_label)
+                 pattern_without_constraint.ppat_loc)
           | _ -> pattern_without_constraint
         in
         if is_labelled arg_label || is_optional arg_label then

tests/build_tests/react_ppx/src/issue_7917_test.res

@@ -0,0 +1,9 @@
+module M = {
+  @react.component
+  let make = () => React.null
+}
+
+module type S = module type of M
+
+@react.component
+let make = (~component as module(C: S)=module(M: S)) => <C />

tests/build_tests/react_ppx/src/issue_7917_test.res.js

@@ -0,0 +1,13 @@
+@@jsxConfig({version: 4})
+
+module C0 = {
+  module M = {
+    @react.component
+    let make = () => React.null
+  }
+
+  module type S = module type of M
+
+  @react.component
+  let make = (~component as module(C: S)=module(M: S)) => <C />
+}

tests/syntax_tests/data/ppx/react/defaultPatternProp.res

@@ -8,10 +8,11 @@ module C0 = {
   }
 
   let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
-    let text = switch __text {
+    let __text_value = switch __text {
     | Some(text) => text
     | None => "Test"
     }
+    let text = __text_value
     (React.string(text): React.element)
   }
   let make = {
@@ -29,10 +30,11 @@ module C1 = {
   }
 
   let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
-    let text = switch __text {
+    let __text_value = switch __text {
     | Some(text) => text
     | None => "Test"
     }
+    let text = __text_value
     (React.string(p ++ text): React.element)
   }
   let make = {
@@ -48,11 +50,12 @@ module C2 = {
     foo?: 'foo,
   }
 
-  let make = ({foo: ?__bar, _}: props<_>) => {
-    let bar = switch __bar {
+  let make = ({foo: ?__foo, _}: props<_>) => {
+    let __foo_value = switch __foo {
     | Some(foo) => foo
     | None => ""
     }
+    let bar = __foo_value
     (React.string(bar): React.element)
   }
   let make = {
@@ -70,15 +73,17 @@ module C3 = {
     b: 'b,
   }
 
-  let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
-    let bar = switch __bar {
+  let make = ({foo: ?__foo, a: ?__a, b, _}: props<_, _, _>) => {
+    let __foo_value = switch __foo {
     | Some(foo) => foo
     | None => ""
     }
-    let a = switch __a {
+    let bar = __foo_value
+    let __a_value = switch __a {
     | Some(a) => a
     | None => bar
     }
+    let a = __a_value
     (
       {
         React.string(bar ++ a ++ b)
@@ -100,10 +105,11 @@ module C4 = {
   }
 
   let make = ({a: b, x: ?__x, _}: props<_, _>) => {
-    let x = switch __x {
+    let __x_value = switch __x {
     | Some(x) => x
     | None => true
     }
+    let x = __x_value
     (ReactDOM.jsx("div", {children: ?ReactDOM.someElement(b)}): React.element)
   }
   let make = {
@@ -121,10 +127,11 @@ module C5 = {
   }
 
   let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
-    let z = switch __z {
+    let __z_value = switch __z {
     | Some(z) => z
     | None => 3
     }
+    let z = __z_value
     (x + y + z: React.element)
   }
   let make = {

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

@@ -0,0 +1,35 @@
+@@jsxConfig({version: 4})
+
+module C0 = {
+  module M = {
+    @res.jsxComponentProps
+    type props = {}
+
+    let make = (_: props): React.element => React.null
+    let make = {
+      let \"DefaultPatternProp$C0$M" = props => make(props)
+
+      \"DefaultPatternProp$C0$M"
+    }
+  }
+
+  module type S = module type of M
+  @res.jsxComponentProps
+  type props<'component> = {
+    component?: 'component,
+  }
+
+  let make = ({component: ?__component, _}: props<_>) => {
+    let __component_value = switch __component {
+    | Some(component) => component
+    | None => module(M: S)
+    }
+    let module(C: S) = __component_value
+    (React.jsx(C.make, {}): React.element)
+  }
+  let make = {
+    let \"DefaultPatternProp$C0" = (props: props<_>) => make(props)
+
+    \"DefaultPatternProp$C0"
+  }
+}

tests/syntax_tests/data/ppx/react/expected/defaultPatternProp.res.txt

@@ -5,14 +5,16 @@ module C0 = {
     b?: 'b,
   }
   let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
-    let a = switch __a {
+    let __a_value = switch __a {
     | Some(a) => a
     | None => 2
     }
-    let b = switch __b {
+    let a = __a_value
+    let __b_value = switch __b {
     | Some(b) => b
     | None => a * 2
     }
+    let b = __b_value
     (React.int(a + b): React.element)
   }
   let make = {
@@ -29,10 +31,11 @@ module C1 = {
   }
 
   let make = ({a: ?__a, b, _}: props<_, _>) => {
-    let a = switch __a {
+    let __a_value = switch __a {
     | Some(a) => a
     | None => 2
     }
+    let a = __a_value
     (React.int(a + b): React.element)
   }
   let make = {
@@ -50,10 +53,11 @@ module C2 = {
   }
 
   let make = ({a: ?__a, _}: props<_>) => {
-    let a = switch __a {
+    let __a_value = switch __a {
     | Some(a) => a
     | None => a
     }
+    let a = __a_value
     (React.string(a): React.element)
   }
   let make = {
@@ -69,11 +73,12 @@ module C3 = {
     disabled?: 'disabled,
   }
 
-  let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
-    let everythingDisabled = switch __everythingDisabled {
+  let make = ({disabled: ?__disabled, _}: props<bool>) => {
+    let __disabled_value = switch __disabled {
     | Some(disabled) => disabled
     | None => false
     }
+    let everythingDisabled: bool = __disabled_value
     (
       {
         React.string(everythingDisabled ? "true" : "false")

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

@@ -5,10 +5,11 @@ type props<'a> = {
 }
 
 let make = ({a: ?__a, _}: props<unit => unit>) => {
-  let a = switch __a {
+  let __a_value = switch __a {
   | Some(a) => a
   | None => () => ()
   }
+  let a: unit => unit = __a_value
   (React.null: React.element)
 }
 let make = {
@@ -34,10 +35,11 @@ module Foo = {
   }
 
   let make = ({callback: ?__callback, _}: props<(string, bool, bool) => unit>) => {
-    let callback = switch __callback {
+    let __callback_value = switch __callback {
     | Some(callback) => callback
     | None => (_, _, _) => ()
     }
+    let callback: (string, bool, bool) => unit = __callback_value
     (
       {
         React.null

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

@@ -4,19 +4,19 @@
 function Alias_default_value_test$C0(props) {
   let __b = props.b;
   let __a = props.a;
-  let a = __a !== undefined ? __a : 2;
-  let b = __b !== undefined ? __b : (a << 1);
-  return a + b | 0;
+  let __a_value = __a !== undefined ? __a : 2;
+  let __b_value = __b !== undefined ? __b : (__a_value << 1);
+  return __a_value + __b_value | 0;
 }
 
 let C0 = {
   make: Alias_default_value_test$C0
 };
 
 function Alias_default_value_test$C1(props) {
-  let __bar = props.foo;
-  if (__bar !== undefined) {
-    return __bar;
+  let __foo = props.foo;
+  if (__foo !== undefined) {
+    return __foo;
   } else {
     return "";
   }
@@ -28,10 +28,10 @@ let C1 = {
 
 function Alias_default_value_test$C2(props) {
   let __a = props.a;
-  let __bar = props.foo;
-  let bar = __bar !== undefined ? __bar : "";
-  let a = __a !== undefined ? __a : bar;
-  return bar + a + props.b;
+  let __foo = props.foo;
+  let __foo_value = __foo !== undefined ? __foo : "";
+  let __a_value = __a !== undefined ? __a : __foo_value;
+  return __foo_value + __a_value + props.b;
 }
 
 let C2 = {