Skip to content

Check @react.component function return type #7733

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
41 changes: 27 additions & 14 deletions compiler/syntax/src/jsx_v4.ml
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,16 @@ let vb_match_expr named_arg_list expr =
in
aux (List.rev named_arg_list)

(* https://github.com/rescript-lang/rescript/issues/7722 *)
let add_jsx_element_return_constraint config expression =
Exp.constraint_ expression
(Typ.constr
{txt = module_access_name config "element"; loc = expression.pexp_loc}
[])

let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
if Jsx_common.has_attr_on_binding Jsx_common.has_attr binding then (
(* @react.component *)
check_multiple_components ~config ~loc:pstr_loc;
let core_type_of_attr =
Jsx_common.core_type_of_attrs binding.pvb_attributes
Expand Down Expand Up @@ -732,6 +740,7 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
| [] -> Pat.any ()
| _ -> Pat.record (List.rev patterns_with_label) Open
in
let expression = add_jsx_element_return_constraint config expression in
let expression =
Exp.fun_ ~arity:(Some 1) ~async:is_async Nolabel None
(Pat.constraint_ record_pattern
Expand Down Expand Up @@ -779,6 +788,7 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
(Some props_record_type, binding, new_binding))
else if Jsx_common.has_attr_on_binding Jsx_common.has_attr_with_props binding
then
(* @react.componentWithProps *)
let modified_binding =
{
binding with
Expand Down Expand Up @@ -835,21 +845,24 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding =
| _ -> Pat.var {txt = "props"; loc}
in

let expression =
Jsx_common.async_component ~async:is_async
(Exp.apply
(Exp.ident
{
txt =
Lident
(match rec_flag with
| Recursive -> internal_fn_name
| Nonrecursive -> fn_name);
loc;
})
[(Nolabel, Exp.ident {txt = Lident "props"; loc})])
in
let expression = add_jsx_element_return_constraint config expression in
let wrapper_expr =
Exp.fun_ ~arity:None Nolabel None props_pattern
~attrs:binding.pvb_expr.pexp_attributes
(Jsx_common.async_component ~async:is_async
(Exp.apply
(Exp.ident
{
txt =
Lident
(match rec_flag with
| Recursive -> internal_fn_name
| Nonrecursive -> fn_name);
loc;
})
[(Nolabel, Exp.ident {txt = Lident "props"; loc})]))
~attrs:binding.pvb_expr.pexp_attributes expression
in

let wrapper_expr = Ast_uncurried.uncurried_fun ~arity:1 wrapper_expr in
Expand Down Expand Up @@ -1282,7 +1295,7 @@ let mk_react_jsx (config : Jsx_common.jsx_config) mapper loc attrs
let args = [(nolabel, elementTag); (nolabel, props_record)] @ key_and_unit in
Exp.apply ~loc ~attrs ~transformed_jsx:true jsx_expr args

(* In most situations, the component name is the make function from a module.
(* In most situations, the component name is the make function from a module.
However, if the name contains a lowercase letter, it means it probably an external component.
In this case, we use the name as is.
See tests/syntax_tests/data/ppx/react/externalWithCustomName.res
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

We've found a bug for you!
/.../fixtures/jsx_invalid_return_type.res:14:5-6

12 │ @react.component
13 │ let make = () => {
14 │ 42 // This should be an error - returning int instead of React.elem
│ ent
15 │ }
16 │ }

This has type: int
But it's expected to have type: React.element (defined as Jsx.element)

In JSX, all content must be JSX elements. You can convert int to a JSX element with React.int.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

We've found a bug for you!
/.../fixtures/jsx_invalid_return_type_component_with_props.res

This has type: int
But it's expected to have type: React.element (defined as Jsx.element)

In JSX, all content must be JSX elements. You can convert int to a JSX element with React.int.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>
}

module BadComponent = {
@react.component
let make = () => {
42 // This should be an error - returning int instead of React.element
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@@config({
flags: ["-bs-jsx", "4"],
})

module React = {
type element = Jsx.element
type componentLike<'props, 'return> = 'props => 'return
type component<'props> = Jsx.component<'props>
}

module BadComponent = {
@react.componentWithProps
let make = _props => {
42 // This should be an error - returning int instead of React.element
}
}
15 changes: 8 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,7 @@ module C0 = {
@res.jsxComponentProps
type props<'priority, 'text> = {priority: 'priority, text?: 'text}

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

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

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

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

let make = ({a: (x, y), z: ?__z, _}: props<_, _>) => {
let make = ({a: (x, y), z: ?__z, _}: props<_, _>): React.element => {
let z = switch __z {
| Some(z) => z
| None => 3
Expand All @@ -130,7 +130,8 @@ 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 = ({comp: module(Comp: Comp), x: (a, b), _}: props<_, _>): React.element =>
React.jsx(Comp.make, {})
let make = {
let \"AliasProps$C6" = (props: props<_>) => make(props)

Expand Down
4 changes: 2 additions & 2 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,7 @@ module C0 = {
@res.jsxComponentProps
type props<'a> = {a: 'a}

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

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

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

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

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

let make = ({disabled: ?__everythingDisabled, _}: props<bool>) => {
let make = ({disabled: ?__everythingDisabled, _}: props<bool>): React.element => {
let everythingDisabled = switch __everythingDisabled {
| Some(disabled) => disabled
| None => false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module V4A = {
@res.jsxComponentProps
type props<'msg> = {msg: 'msg}

let make = ({msg, _}: props<_>) => {
let make = ({msg, _}: props<_>): React.element => {
ReactDOM.jsx("div", {children: ?ReactDOM.someElement({msg->React.string})})
}
let make = {
Expand Down
74 changes: 38 additions & 36 deletions tests/syntax_tests/data/ppx/react/expected/forwardRef.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,24 @@ module V4A = {
ref?: 'ref,
}

let make = ({?className, children, _}: props<_, _, 'ref>, ref: Js.Nullable.t<'ref>) =>
ReactDOM.jsxs(
"div",
{
children: React.array([
ReactDOM.jsx(
"input",
{
type_: "text",
?className,
ref: ?{Js.Nullable.toOption(ref)->Belt.Option.map(ReactDOM.Ref.domRef)},
},
),
children,
]),
},
)
let make = ({?className, children, _}: props<_, _, 'ref>): React.element =>
(ref: Js.Nullable.t<'ref>) =>
ReactDOM.jsxs(
"div",
{
children: React.array([
ReactDOM.jsx(
"input",
{
type_: "text",
?className,
ref: ?{Js.Nullable.toOption(ref)->Belt.Option.map(ReactDOM.Ref.domRef)},
},
),
children,
]),
},
)
let make = React.forwardRef({
let \"ForwardRef$V4A$FancyInput" = (props: props<_>, ref) => make(props, ref)

Expand All @@ -35,7 +36,7 @@ module V4A = {
@res.jsxComponentProps
type props = {}

let make = (_: props) => {
let make = (_: props): React.element => {
let input = React.useRef(Js.Nullable.null)

ReactDOM.jsx(
Expand Down Expand Up @@ -63,23 +64,24 @@ module V4AUncurried = {
ref?: 'ref,
}

let make = ({?className, children, _}: props<_, _, 'ref>, ref: Js.Nullable.t<'ref>) =>
ReactDOM.jsxs(
"div",
{
children: React.array([
ReactDOM.jsx(
"input",
{
type_: "text",
?className,
ref: ?{Js.Nullable.toOption(ref)->Belt.Option.map(ReactDOM.Ref.domRef)},
},
),
children,
]),
},
)
let make = ({?className, children, _}: props<_, _, 'ref>): React.element =>
(ref: Js.Nullable.t<'ref>) =>
ReactDOM.jsxs(
"div",
{
children: React.array([
ReactDOM.jsx(
"input",
{
type_: "text",
?className,
ref: ?{Js.Nullable.toOption(ref)->Belt.Option.map(ReactDOM.Ref.domRef)},
},
),
children,
]),
},
)
let make = React.forwardRef({
let \"ForwardRef$V4AUncurried$FancyInput" = (props: props<_>, ref) => make(props, ref)

Expand All @@ -89,7 +91,7 @@ module V4AUncurried = {
@res.jsxComponentProps
type props = {}

let make = (_: props) => {
let make = (_: props): React.element => {
let input = React.useRef(Js.Nullable.null)

ReactDOM.jsx(
Expand Down
4 changes: 2 additions & 2 deletions tests/syntax_tests/data/ppx/react/expected/interface.res.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module A = {
@res.jsxComponentProps
type props<'x> = {x: 'x}
let make = ({x, _}: props<_>) => React.string(x)
let make = ({x, _}: props<_>): React.element => React.string(x)
let make = {
let \"Interface$A" = (props: props<_>) => make(props)
\"Interface$A"
Expand All @@ -12,7 +12,7 @@ module NoProps = {
@res.jsxComponentProps
type props = {}

let make = (_: props) => ReactDOM.jsx("div", {})
let make = (_: props): React.element => ReactDOM.jsx("div", {})
let make = {
let \"Interface$NoProps" = props => make(props)

Expand Down
Loading
Loading