Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Fix issue overlapping argument with default value and alias with default value #734

Merged
merged 8 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 36 additions & 22 deletions cli/reactjs_jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1016,26 +1016,34 @@ let transformStructureItem ~config mapper item =
(argToType ~newtypes ~typeConstraints)
[] namedArgList
in
let namedArgWithDefaultValueList =
List.filter_map argWithDefaultValue namedArgList
let vbMatch (name, default, _, _, _, _) =
let label = getLabel name in
match default with
| Some default ->
Vb.mk
(Pat.var (Location.mknoloc label))
(Exp.match_
(Exp.ident
{txt = Ldot (Lident "props", 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;
])
| None ->
Vb.mk
(Pat.var (Location.mknoloc label))
(Exp.ident @@ Location.mknoloc @@ Ldot (Lident "props", 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 vbMatchList =
if hasDefaultValue namedArgList then List.map vbMatch namedArgList
else []
in
let vbMatchList = List.map vbMatch namedArgWithDefaultValueList in
(* type props = { ... } *)
let propsRecordType =
makePropsRecordType ~coreTypeOfAttr ~typVarsOfCoreType "props"
Expand Down Expand Up @@ -1163,11 +1171,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
Expand Down
9 changes: 9 additions & 0 deletions tests/ppx/react/defaultValueProp.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module C0 = {
@react.component
let make = (~a=2, ~b=a*2) => <div />
}

module C1 = {
@react.component
let make = (~a=2, ~b) => <div />
}
10 changes: 6 additions & 4 deletions tests/ppx/react/expected/aliasProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ 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>) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using props instead of destructuring to avoid the conflict from overlapped argument name with default value.

let text = switch props.text {
| Some(text) => text
| None => "Test"
}
and priority = props.priority

React.string(text)
}
Expand All @@ -23,11 +24,12 @@ 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 text = switch props.text {
| Some(text) => text
| None => "Test"
}
and priority = props.priority

React.string(p ++ text)
}
Expand Down
40 changes: 40 additions & 0 deletions tests/ppx/react/expected/defaultValueProp.res.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
module C0 = {
type props<'a, 'b> = {a?: 'a, b?: 'b}
@react.component
let make = (props: props<'a, 'b>) => {
let b = switch props.b {
| Some(b) => b
| None => a * 2
}
and a = switch props.a {
| Some(a) => a
| None => 2
}

ReactDOM.jsx("div", {})
}
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 b = props.b
and a = switch props.a {
| Some(a) => a
| None => 2
}

ReactDOM.jsx("div", {})
}
let make = {
let \"DefaultValueProp$C1" = (props: props<_>) => make(props)

\"DefaultValueProp$C1"
}
}
8 changes: 4 additions & 4 deletions tests/ppx/react/expected/uncurriedProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 => (. ()) => ()
}
Expand All @@ -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 => (. _, _, _) => ()
}
Expand Down