diff --git a/CHANGELOG.md b/CHANGELOG.md index 66137f2a09..c4737a5055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,8 @@ These are only breaking changes for unformatted code. - Fix issue with JSX V4 and newtype https://github.com/rescript-lang/rescript-compiler/pull/6029 - Fix issue with JSX V4 when components are nested https://github.com/rescript-lang/rescript-compiler/pull/6031 - Fix issue where generic compare on `float` values would be different from the compare for type `float` https://github.com/rescript-lang/rescript-compiler/pull/6042 +- Improve code generated for default arguments in JSX V4 https://github.com/rescript-lang/rescript-compiler/pull/6041 +- Fix issue with JSX V4 props of the form `~p as module(...)` https://github.com/rescript-lang/rescript-compiler/pull/6041 #### :nail_care: Polish diff --git a/docs/JSXV4.md b/docs/JSXV4.md index 8ed81a5c74..fafae113fc 100644 --- a/docs/JSXV4.md +++ b/docs/JSXV4.md @@ -264,7 +264,7 @@ 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 @@ -272,8 +272,8 @@ 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 y = switch y { | None => 3 + x | Some(y) => y } diff --git a/jscomp/test/alias_default_value_test.js b/jscomp/test/alias_default_value_test.js index 88cd75893c..41bb57733f 100644 --- a/jscomp/test/alias_default_value_test.js +++ b/jscomp/test/alias_default_value_test.js @@ -2,9 +2,9 @@ function Alias_default_value_test$C0(props) { + var b = props.b; var a = props.a; var a$1 = a !== undefined ? a : 2; - var b = props.b; var b$1 = b !== undefined ? b : (a$1 << 1); return a$1 + b$1 | 0; } @@ -14,9 +14,9 @@ var C0 = { }; function Alias_default_value_test$C1(props) { - var foo = props.foo; - if (foo !== undefined) { - return foo; + var bar = props.foo; + if (bar !== undefined) { + return bar; } else { return ""; } @@ -26,6 +26,51 @@ var C1 = { make: Alias_default_value_test$C1 }; +function Alias_default_value_test$C2(props) { + var a = props.a; + var bar = props.foo; + var bar$1 = bar !== undefined ? bar : ""; + var a$1 = a !== undefined ? a : bar$1; + return bar$1 + a$1 + props.b; +} + +var C2 = { + make: Alias_default_value_test$C2 +}; + +function Alias_default_value_test$C3(props) { + var text = props.text; + if (text !== undefined) { + return text; + } else { + return "Test"; + } +} + +var C3 = { + make: Alias_default_value_test$C3 +}; + +function Alias_default_value_test$C4(props) { + return props.a; +} + +var C4 = { + make: Alias_default_value_test$C4 +}; + +function Alias_default_value_test$C6(props) { + return props.comp.xx; +} + +var C6 = { + make: Alias_default_value_test$C6 +}; + exports.C0 = C0; exports.C1 = C1; +exports.C2 = C2; +exports.C3 = C3; +exports.C4 = C4; +exports.C6 = C6; /* No side effect */ diff --git a/jscomp/test/alias_default_value_test.res b/jscomp/test/alias_default_value_test.res index e4594f0562..68d6a8b5f8 100644 --- a/jscomp/test/alias_default_value_test.res +++ b/jscomp/test/alias_default_value_test.res @@ -15,3 +15,31 @@ module C1 = { React.string(bar) } } + +module C2 = { + @react.component + let make = (~foo as bar="", ~a=bar, ~b) => { + React.string(bar ++ a ++ b) + } +} + +module C3 = { + @react.component + let make = (~priority as _, ~text="Test") => React.string(text) +} + +module C4 = { + @react.component + let make = (~a as b, ~x=true) => b +} + +module C6 = { + module type Comp = { + let xx : int + @react.component + let make: unit => React.element + } + + @react.component + let make = (~comp as module(Comp: Comp), ~x as (a, b)) => Comp.xx +} diff --git a/res_syntax/src/reactjs_jsx_v4.ml b/res_syntax/src/reactjs_jsx_v4.ml index 65447e854d..e2afe36d6d 100644 --- a/res_syntax/src/reactjs_jsx_v4.ml +++ b/res_syntax/src/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 diff --git a/res_syntax/tests/ppx/react/aliasProps.res b/res_syntax/tests/ppx/react/aliasProps.res index c695170353..8c183b9415 100644 --- a/res_syntax/tests/ppx/react/aliasProps.res +++ b/res_syntax/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) =>
b
+} + +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)) => +} diff --git a/res_syntax/tests/ppx/react/expected/aliasProps.res.txt b/res_syntax/tests/ppx/react/expected/aliasProps.res.txt index 0e310eab42..03afab4223 100644 --- a/res_syntax/tests/ppx/react/expected/aliasProps.res.txt +++ b/res_syntax/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 + } + 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" + } +} diff --git a/res_syntax/tests/ppx/react/expected/defaultValueProp.res.txt b/res_syntax/tests/ppx/react/expected/defaultValueProp.res.txt index 705b8e7ad6..1ef8dd01f1 100644 --- a/res_syntax/tests/ppx/react/expected/defaultValueProp.res.txt +++ b/res_syntax/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) } diff --git a/res_syntax/tests/ppx/react/expected/firstClassModules.res.txt b/res_syntax/tests/ppx/react/expected/firstClassModules.res.txt index 2ce2da2676..dd14bf349f 100644 --- a/res_syntax/tests/ppx/react/expected/firstClassModules.res.txt +++ b/res_syntax/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, option => unit, array, diff --git a/res_syntax/tests/ppx/react/expected/newtype.res.txt b/res_syntax/tests/ppx/react/expected/newtype.res.txt index b8900c508b..7d09fa4528 100644 --- a/res_syntax/tests/ppx/react/expected/newtype.res.txt +++ b/res_syntax/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) => { + let make = (type a, {foo: (foo: module(T with type t = a)), _}: props<_>) => { module T = unpack(foo) ReactDOM.jsx("div", {}) } @@ -91,3 +91,11 @@ module V4A3 = { \"Newtype$V4A3" } } +type props<'x, 'q> = {x: 'x, q: 'q} + +let make = ({x, q, _}: props<('a, 'b), 'a>) => [fst(x), q] +let make = { + let \"Newtype" = (props: props<_>) => make(props) + + \"Newtype" +} diff --git a/res_syntax/tests/ppx/react/expected/uncurriedProps.res.txt b/res_syntax/tests/ppx/react/expected/uncurriedProps.res.txt index e09209752e..459ddf98d2 100644 --- a/res_syntax/tests/ppx/react/expected/uncurriedProps.res.txt +++ b/res_syntax/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 => (. _, _, _) => () } diff --git a/res_syntax/tests/ppx/react/newtype.res b/res_syntax/tests/ppx/react/newtype.res index c77634ed88..aab1935456 100644 --- a/res_syntax/tests/ppx/react/newtype.res +++ b/res_syntax/tests/ppx/react/newtype.res @@ -43,3 +43,6 @@ module V4A3 = { foo } } + +@react.component +let make =(~x : ('a,'b), ~q:'a ) => [fst(x), q] \ No newline at end of file