Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ignoring react component arguments when aliasing props type #7194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
63 changes: 60 additions & 3 deletions compiler/syntax/src/jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,61 @@ let vb_match_expr named_arg_list expr =
in
aux (List.rev named_arg_list)

let vb_type_annotation ~expr (name, default, pattern, alias, loc, core_type) =
let label = get_label name in
let pattern_name =
match default with
| Some _ -> "__" ^ alias
| None -> alias
in
let value_binding =
Vb.mk ~loc
(match pattern with
| {
ppat_desc =
Ppat_constraint (pattern, ({ptyp_desc = Ptyp_package _} as type_));
} ->
(* Handle pattern: ~comp as module(Comp: Comp) *)
Pat.record
[
( {txt = Lident label; loc = Location.none},
Pat.constraint_ pattern type_ );
]
Closed
| _ -> (
(* For other cases, use regular variable pattern with type constraint *)
let type_ =
match pattern with
| {ppat_desc = Ppat_constraint (_, type_)} -> Some type_
| _ -> core_type
in
match type_ with
| Some type_ ->
Pat.constraint_ (Pat.var (Location.mkloc pattern_name loc)) type_
| None -> Pat.var (Location.mkloc pattern_name loc)))
(match pattern with
| {ppat_desc = Ppat_constraint (_, {ptyp_desc = Ptyp_package _})} ->
(* For module types, use props directly *)
Exp.ident {txt = Lident "props"; loc = Location.none}
| _ ->
(* For other cases, use props.x form *)
Exp.field
(Exp.ident {txt = Lident "props"; loc = Location.none})
{txt = Lident label; loc = Location.none})
in
Exp.let_ Nonrecursive [value_binding] expr

let vb_type_annotations_expr named_arg_list expr =
let rec aux named_arg_list =
match named_arg_list with
| [] -> expr
| ((name, _, _, _, _, _) as named_arg) :: rest ->
let label = get_label name in
if label = "ref" then aux rest
else vb_type_annotation named_arg ~expr:(aux rest)
in
aux (List.rev named_arg_list)

let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
if Jsx_common.has_attr_on_binding binding then (
check_multiple_components ~config ~loc:pstr_loc;
Expand Down Expand Up @@ -1105,6 +1160,8 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
vb_match_expr named_arg_list expression
else expression
in
(* add pattern matching for optional prop value and type annotations *)
let expression = vb_type_annotations_expr named_arg_list expression in
(* (ref) => expr *)
let expression =
List.fold_left
Expand All @@ -1118,11 +1175,11 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
Exp.fun_ Nolabel None pattern expr)
expression patterns_with_nolabel
in
(* ({a, b, _}: props<'a, 'b>) *)
(* (props: props<'a, 'b>) *)
let record_pattern =
match patterns_with_label with
| [] -> Pat.any ()
| _ -> Pat.record (List.rev patterns_with_label) Open
| [] -> Pat.any () (* (_: props<'a, 'b>)*)
| _ -> Pat.var @@ Location.mknoloc "props"
in
let expression =
Exp.fun_ Nolabel None
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 23 additions & 7 deletions tests/syntax_tests/data/ppx/react/expected/aliasProps.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ module C0 = {
@res.jsxComponentProps
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

let make = ({priority: _, text: ?__text, _}: props<_, _>) => {
let make = (props: props<_, _>) => {
let _ = props.priority
let __text = props.text
let text = switch __text {
| Some(text) => text
| None => "Test"
Expand All @@ -23,7 +25,9 @@ module C1 = {
@res.jsxComponentProps
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

let make = ({priority: p, text: ?__text, _}: props<_, _>) => {
let make = (props: props<_, _>) => {
let p = props.priority
let __text = props.text
let text = switch __text {
| Some(text) => text
| None => "Test"
Expand All @@ -42,7 +46,8 @@ module C2 = {
@res.jsxComponentProps
type props<'foo> = {foo?: 'foo}

let make = ({foo: ?__bar, _}: props<_>) => {
let make = (props: props<_>) => {
let __bar = props.foo
let bar = switch __bar {
| Some(foo) => foo
| None => ""
Expand All @@ -61,7 +66,10 @@ module C3 = {
@res.jsxComponentProps
type props<'foo, 'a, 'b> = {foo?: 'foo, a?: 'a, b: 'b}

let make = ({foo: ?__bar, a: ?__a, b, _}: props<_, _, _>) => {
let make = (props: props<_, _, _>) => {
let __bar = props.foo
let __a = props.a
let b = props.b
let bar = switch __bar {
| Some(foo) => foo
| None => ""
Expand All @@ -86,7 +94,9 @@ module C4 = {
@res.jsxComponentProps
type props<'a, 'x> = {a: 'a, x?: 'x}

let make = ({a: b, x: ?__x, _}: props<_, _>) => {
let make = (props: props<_, _>) => {
let b = props.a
let __x = props.x
let x = switch __x {
| Some(x) => x
| None => true
Expand All @@ -105,7 +115,9 @@ module C5 = {
@res.jsxComponentProps
type props<'a, 'z> = {a: 'a, z?: 'z}

let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
let make = (props: props<_, _>) => {
let a = props.a
let __z = props.z
let z = switch __z {
| Some(z) => z
| None => 3
Expand All @@ -130,7 +142,11 @@ module C6 = {
@res.jsxComponentProps
type props<'comp, 'x> = {comp: 'comp, x: 'x}

let make = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>) => React.jsx(Comp.make, {})
let make = (props: props<_, _>) => {
let {comp: module(Comp: Comp)} = props
let x = props.x
React.jsx(Comp.make, {})
}
let make = {
let \"AliasProps$C6" = (props: props<_>) => make(props)

Expand Down
14 changes: 9 additions & 5 deletions tests/syntax_tests/data/ppx/react/expected/asyncAwait.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ module C0 = {
@res.jsxComponentProps
type props<'a> = {a: 'a}

let make = async ({a, _}: props<_>) => {
let make = async (props: props<_>) => {
let a = props.a
let a = await f(a)
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({React.int(a)})})
}
Expand All @@ -19,10 +20,13 @@ module C1 = {
@res.jsxComponentProps
type props<'status> = {status: 'status}

let make = async ({status, _}: props<_>) => {
switch status {
| #on => React.string("on")
| #off => React.string("off")
let make = async (props: props<_>) => {
let status = props.status
{
switch status {
| #on => React.string("on")
| #off => React.string("off")
}
}
}
let make = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
@res.jsxComponentProps
type props<'msg> = {msg: 'msg} // test React JSX file

let make = ({msg, _}: props<_>) => {
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
let make = (props: props<_>) => {
let msg = props.msg
{
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
}
}
let make = {
let \"CommentAtTop" = (props: props<_>) => make(props)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
module C0 = {
@res.jsxComponentProps
type props<'a, 'b> = {a?: 'a, b?: 'b}
let make = ({a: ?__a, b: ?__b, _}: props<_, _>) => {
let make = (props: props<_, _>) => {
let __a = props.a
let __b = props.b
let a = switch __a {
| Some(a) => a
| None => 2
Expand All @@ -23,7 +25,9 @@ module C1 = {
@res.jsxComponentProps
type props<'a, 'b> = {a?: 'a, b: 'b}

let make = ({a: ?__a, b, _}: props<_, _>) => {
let make = (props: props<_, _>) => {
let __a = props.a
let b = props.b
let a = switch __a {
| Some(a) => a
| None => 2
Expand All @@ -43,7 +47,8 @@ module C2 = {
@res.jsxComponentProps
type props<'a> = {a?: 'a}

let make = ({a: ?__a, _}: props<_>) => {
let make = (props: props<_>) => {
let __a = props.a
let a = switch __a {
| Some(a) => a
| None => a
Expand All @@ -62,7 +67,8 @@ module C3 = {
@res.jsxComponentProps
type props<'disabled> = {disabled?: 'disabled}

let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
let make = (props: props<bool>) => {
let __everythingDisabled: bool = props.disabled
let everythingDisabled = switch __everythingDisabled {
| Some(disabled) => disabled
| None => false
Expand Down
14 changes: 10 additions & 4 deletions tests/syntax_tests/data/ppx/react/expected/fileLevelConfig.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ module V4C = {
@res.jsxComponentProps
type props<'msg> = {msg: 'msg}

let make = ({msg, _}: props<_>) => {
ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
let make = (props: props<_>) => {
let msg = props.msg
{
ReactDOM.createDOMElementVariadic("div", [{msg->React.string}])
}
}
let make = {
let \"FileLevelConfig$V4C" = (props: props<_>) => make(props)
Expand All @@ -20,8 +23,11 @@ module V4A = {
@res.jsxComponentProps
type props<'msg> = {msg: 'msg}

let make = ({msg, _}: props<_>) => {
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
let make = (props: props<_>) => {
let msg = props.msg
{
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
}
}
let make = {
let \"FileLevelConfig$V4A" = (props: props<_>) => make(props)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,12 @@ module Select = {
items: 'items,
}

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

let _ = (model, selected, onChange, items)
ReactDOM.createDOMElementVariadic("div", [])
}
Expand All @@ -32,6 +29,29 @@ module Select = {
}
}

module C6 = {
module type Comp = {
let xx: int
@res.jsxComponentProps
type props = {}

let make: React.componentLike<props, React.element>
}
@res.jsxComponentProps
type props<'comp, 'x> = {comp: 'comp, x: 'x}

let make = (props: props<_, _>) => {
let {comp: module(Comp: Comp)} = props
let x = props.x
Comp.xx
}
let make = {
let \"FirstClassModules$C6" = (props: props<_>) => make(props)

\"FirstClassModules$C6"
}
}

module External = {
module type T = {
type key
Expand Down
Loading
Loading