diff --git a/CHANGELOG.md b/CHANGELOG.md index 2717505f..e1a901c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,8 @@ - Fix issue where uncurried functions were incorrectly converting the type of a prop given as a default value to curried https://github.com/rescript-lang/syntax/pull/731 - Fix issue with printing async functions with locally abstract types https://github.com/rescript-lang/syntax/pull/732 - Fix support for recursive components in JSX V4 https://github.com/rescript-lang/syntax/pull/733 +- Fix issue with overlapping labelled argument with default value https://github.com/rescript-lang/syntax/pull/734 +- Fix issue with using alias and default value together https://github.com/rescript-lang/syntax/pull/734 #### :eyeglasses: Spec Compliance diff --git a/cli/reactjs_jsx_v4.ml b/cli/reactjs_jsx_v4.ml index b4c9ed61..c53a467d 100644 --- a/cli/reactjs_jsx_v4.ml +++ b/cli/reactjs_jsx_v4.ml @@ -729,10 +729,10 @@ let argToType ~newtypes ~(typeConstraints : core_type option) types :: types | _ -> types -let argWithDefaultValue (name, default, _, _, _, _) = - match default with - | Some default when isOptional name -> Some (getLabel name, default) - | _ -> None +let hasDefaultValue nameArgList = + nameArgList + |> List.exists (fun (name, default, _, _, _, _) -> + Option.is_some default && isOptional name) let argToConcreteType types (name, attrs, loc, type_) = match name with @@ -1016,26 +1016,43 @@ let transformStructureItem ~config mapper item = (argToType ~newtypes ~typeConstraints) [] namedArgList in - let namedArgWithDefaultValueList = - List.filter_map argWithDefaultValue namedArgList + let vbMatch (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)) in - let vbMatch (label, default) = - Vb.mk - (Pat.var (Location.mknoloc label)) - (Exp.match_ - (Exp.ident {txt = Lident label; 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; - ]) + 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) + in + aux (List.rev namedArgList) in - let vbMatchList = List.map vbMatch namedArgWithDefaultValueList in (* type props = { ... } *) let propsRecordType = makePropsRecordType ~coreTypeOfAttr ~typVarsOfCoreType "props" @@ -1154,8 +1171,9 @@ let transformStructureItem ~config mapper item = in (* add pattern matching for optional prop value *) let expression = - if List.length vbMatchList = 0 then expression - else Exp.let_ Nonrecursive vbMatchList expression + if hasDefaultValue namedArgList then + vbMatchExpr namedArgList expression + else expression in (* (ref) => expr *) let expression = @@ -1163,11 +1181,17 @@ let transformStructureItem ~config mapper item = (fun expr (_, pattern) -> Exp.fun_ Nolabel None pattern expr) expression patternsWithNolabel in + (* ({a, b, _}: props<'a, 'b>) *) let recordPattern = match patternsWithLabel with | [] -> 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/tests/ppx/react/aliasProps.res b/tests/ppx/react/aliasProps.res index 71734f50..c6951703 100644 --- a/tests/ppx/react/aliasProps.res +++ b/tests/ppx/react/aliasProps.res @@ -9,3 +9,8 @@ module C1 = { @react.component let make = (~priority as p, ~text="Test") => React.string(p ++ text) } + +module C2 = { + @react.component + let make = (~foo as bar="") => React.string(bar) +} diff --git a/tests/ppx/react/defaultValueProp.res b/tests/ppx/react/defaultValueProp.res new file mode 100644 index 00000000..7764dd8a --- /dev/null +++ b/tests/ppx/react/defaultValueProp.res @@ -0,0 +1,9 @@ +module C0 = { + @react.component + let make = (~a=2, ~b=a*2) => React.int(a + b) +} + +module C1 = { + @react.component + let make = (~a=2, ~b) => React.int(a + b) +} diff --git a/tests/ppx/react/expected/aliasProps.res.txt b/tests/ppx/react/expected/aliasProps.res.txt index 721846d1..ece2f660 100644 --- a/tests/ppx/react/expected/aliasProps.res.txt +++ b/tests/ppx/react/expected/aliasProps.res.txt @@ -4,8 +4,9 @@ module C0 = { type props<'priority, 'text> = {priority: 'priority, text?: 'text} @react.component - let make = ({priority: _, ?text, _}: props<'priority, 'text>) => { - let text = switch text { + let make = (props: props<'priority, 'text>) => { + let _ = props.priority + let text = switch props.text { | Some(text) => text | None => "Test" } @@ -23,8 +24,9 @@ module C1 = { type props<'priority, 'text> = {priority: 'priority, text?: 'text} @react.component - let make = ({priority: p, ?text, _}: props<'priority, 'text>) => { - let text = switch text { + let make = (props: props<'priority, 'text>) => { + let p = props.priority + let text = switch props.text { | Some(text) => text | None => "Test" } @@ -37,3 +39,22 @@ module C1 = { \"AliasProps$C1" } } + +module C2 = { + type props<'foo> = {foo?: 'foo} + + @react.component + let make = (props: props<'foo>) => { + let bar = switch props.foo { + | Some(foo) => foo + | None => "" + } + + React.string(bar) + } + let make = { + let \"AliasProps$C2" = (props: props<_>) => make(props) + + \"AliasProps$C2" + } +} diff --git a/tests/ppx/react/expected/defaultValueProp.res.txt b/tests/ppx/react/expected/defaultValueProp.res.txt new file mode 100644 index 00000000..dc8f002c --- /dev/null +++ b/tests/ppx/react/expected/defaultValueProp.res.txt @@ -0,0 +1,40 @@ +module C0 = { + type props<'a, 'b> = {a?: 'a, b?: 'b} + @react.component + let make = (props: props<'a, 'b>) => { + let a = switch props.a { + | Some(a) => a + | None => 2 + } + let b = switch props.b { + | Some(b) => b + | None => a * 2 + } + + React.int(a + b) + } + let make = { + let \"DefaultValueProp$C0" = (props: props<_>) => make(props) + \"DefaultValueProp$C0" + } +} + +module C1 = { + type props<'a, 'b> = {a?: 'a, b: 'b} + + @react.component + let make = (props: props<'a, 'b>) => { + let a = switch props.a { + | Some(a) => a + | None => 2 + } + let b = props.b + + React.int(a + b) + } + let make = { + let \"DefaultValueProp$C1" = (props: props<_>) => make(props) + + \"DefaultValueProp$C1" + } +} diff --git a/tests/ppx/react/expected/uncurriedProps.res.txt b/tests/ppx/react/expected/uncurriedProps.res.txt index cf50e101..2d00031d 100644 --- a/tests/ppx/react/expected/uncurriedProps.res.txt +++ b/tests/ppx/react/expected/uncurriedProps.res.txt @@ -2,8 +2,8 @@ type props<'a> = {a?: 'a} @react.component -let make = ({?a, _}: props<(. unit) => unit>) => { - let a = switch a { +let make = (props: props<(. unit) => unit>) => { + let a = switch props.a { | Some(a) => a | None => (. ()) => () } @@ -30,8 +30,8 @@ module Foo = { type props<'callback> = {callback?: 'callback} @react.component - let make = ({?callback, _}: props<(. string, bool, bool) => unit>) => { - let callback = switch callback { + let make = (props: props<(. string, bool, bool) => unit>) => { + let callback = switch props.callback { | Some(callback) => callback | None => (. _, _, _) => () }