From f2fd5e1307fa03536aa386a020bfd882d6208555 Mon Sep 17 00:00:00 2001 From: woonki Date: Wed, 18 Dec 2024 20:06:21 +0900 Subject: [PATCH] Add jsx ppx for attribute `@react.componentWithProps` (#7203) * add ppx for attribute @react.componentWithProps * clean up the attribute after transformation * use fn_name * add arity=1 * build error using @react.componentWithProps with React.forwardRef * change log --- CHANGELOG.md | 1 + compiler/syntax/src/jsx_common.ml | 9 +- compiler/syntax/src/jsx_v4.ml | 112 ++++++++++++++- .../react_component_with_props.res.expected | 15 ++ .../fixtures/react_component_with_props.res | 15 ++ .../expected/sharedPropsWithProps.res.txt | 133 ++++++++++++++++++ .../data/ppx/react/sharedPropsWithProps.res | 93 ++++++++++++ 7 files changed, 374 insertions(+), 4 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/react_component_with_props.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/react_component_with_props.res create mode 100644 tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt create mode 100644 tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res diff --git a/CHANGELOG.md b/CHANGELOG.md index f893536f57..183f036b02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ # 12.0.0-alpha.6 (Unreleased) - Fix exponential notation syntax. https://github.com/rescript-lang/rescript/pull/7174 - Add `Option.all` & `Result.all` helpers. https://github.com/rescript-lang/rescript/pull/7181 +- Add `@react.componentWithProps` that explicitly handles the props with shared props: https://github.com/rescript-lang/rescript/pull/7203 #### :bug: Bug fix - Fix bug where a ref assignment is moved ouside a conditional. https://github.com/rescript-lang/rescript/pull/7176 diff --git a/compiler/syntax/src/jsx_common.ml b/compiler/syntax/src/jsx_common.ml index fd04f58bba..1e749be686 100644 --- a/compiler/syntax/src/jsx_common.ml +++ b/compiler/syntax/src/jsx_common.ml @@ -15,9 +15,14 @@ let has_attr (loc, _) = | "react.component" | "jsx.component" -> true | _ -> false +let has_attr_with_props (loc, _) = + match loc.txt with + | "react.componentWithProps" | "jsx.componentWithProps" -> true + | _ -> false + (* Iterate over the attributes and try to find the [@react.component] attribute *) -let has_attr_on_binding {pvb_attributes} = - List.find_opt has_attr pvb_attributes <> None +let has_attr_on_binding pred {pvb_attributes} = + List.find_opt pred pvb_attributes <> None let core_type_of_attrs attributes = List.find_map diff --git a/compiler/syntax/src/jsx_v4.ml b/compiler/syntax/src/jsx_v4.ml index e8fe6defd4..872e7ac282 100644 --- a/compiler/syntax/src/jsx_v4.ml +++ b/compiler/syntax/src/jsx_v4.ml @@ -124,7 +124,9 @@ let merlin_focus = ({loc = Location.none; txt = "merlin.focus"}, PStr []) (* Helper method to filter out any attribute that isn't [@react.component] *) let other_attrs_pure (loc, _) = match loc.txt with - | "react.component" | "jsx.component" -> false + | "react.component" | "jsx.component" | "react.componentWithProps" + | "jsx.componentWithProps" -> + false | _ -> true (* Finds the name of the variable the binding is assigned to, otherwise raises Invalid_argument *) @@ -941,7 +943,7 @@ let vb_match_expr named_arg_list expr = 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 ( + if Jsx_common.has_attr_on_binding Jsx_common.has_attr binding then ( check_multiple_components ~config ~loc:pstr_loc; let binding = Jsx_common.remove_arity binding in let core_type_of_attr = @@ -1189,6 +1191,112 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name ~rec_flag binding = Some (binding_wrapper full_expression) ) in (Some props_record_type, binding, new_binding)) + else if Jsx_common.has_attr_on_binding Jsx_common.has_attr_with_props binding + then + let modified_binding = Jsx_common.remove_arity binding in + let modified_binding = + { + modified_binding with + pvb_attributes = + modified_binding.pvb_attributes |> List.filter other_attrs_pure; + } + in + let fn_name = get_fn_name modified_binding.pvb_pat in + let internal_fn_name = fn_name ^ "$Internal" in + let full_module_name = + make_module_name file_name config.nested_modules fn_name + in + + let is_async = + Ext_list.find_first modified_binding.pvb_expr.pexp_attributes + Ast_async.is_async + |> Option.is_some + in + + let make_new_binding ~loc ~full_module_name binding = + let props_pattern = + match binding.pvb_expr with + | {pexp_desc = Pexp_apply (wrapper_expr, [(Nolabel, func_expr)])} + when is_forward_ref wrapper_expr -> + (* Case when using React.forwardRef *) + let rec check_invalid_forward_ref expr = + match expr.pexp_desc with + | Pexp_fun ((Labelled _ | Optional _), _, _, _, _) -> + Location.raise_errorf ~loc:expr.pexp_loc + "Components using React.forwardRef cannot use \ + @react.componentWithProps. Please use @react.component \ + instead." + | Pexp_fun (Nolabel, _, _, body, _) -> + check_invalid_forward_ref body + | _ -> () + in + check_invalid_forward_ref func_expr; + Pat.var {txt = "props"; loc} + | { + pexp_desc = + Pexp_fun (_, _, {ppat_desc = Ppat_constraint (_, typ)}, _, _); + } -> ( + match typ with + | {ptyp_desc = Ptyp_constr ({txt = Lident "props"}, args)} -> + (* props<_> *) + if List.length args > 0 then + Pat.constraint_ + (Pat.var {txt = "props"; loc}) + (Typ.constr {txt = Lident "props"; loc} [Typ.any ()]) + (* props *) + else + Pat.constraint_ + (Pat.var {txt = "props"; loc}) + (Typ.constr {txt = Lident "props"; loc} []) + | _ -> Pat.var {txt = "props"; loc}) + | _ -> Pat.var {txt = "props"; loc} + in + + let wrapper_expr = + Exp.fun_ ~arity:None Nolabel None props_pattern + (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 wrapper_expr = + Ast_uncurried.uncurried_fun ~loc:wrapper_expr.pexp_loc ~arity:1 + wrapper_expr + in + + let internal_expression = + Exp.let_ Nonrecursive + [Vb.mk (Pat.var {txt = full_module_name; loc}) wrapper_expr] + (Exp.ident {txt = Lident full_module_name; loc}) + in + + Vb.mk ~attrs:modified_binding.pvb_attributes + (Pat.var {txt = fn_name; loc}) + internal_expression + in + + let new_binding = + match rec_flag with + | Recursive -> None + | Nonrecursive -> + Some + (make_new_binding ~loc:empty_loc ~full_module_name modified_binding) + in + ( None, + { + binding with + pvb_attributes = binding.pvb_attributes |> List.filter other_attrs_pure; + }, + new_binding ) else (None, binding, None) let transform_structure_item ~config item = diff --git a/tests/build_tests/super_errors/expected/react_component_with_props.res.expected b/tests/build_tests/super_errors/expected/react_component_with_props.res.expected new file mode 100644 index 0000000000..66083eef22 --- /dev/null +++ b/tests/build_tests/super_errors/expected/react_component_with_props.res.expected @@ -0,0 +1,15 @@ + + We've found a bug for you! + /.../fixtures/react_component_with_props.res:4:5-13:10 + + 2 │ @react.componentWithProps + 3 │ let make = React.forwardRef(( + 4 │ ~className=?, + 5 │  ~children, + . │ ... + 12 │  children + 13 │   + 14 │ ) + 15 │ } + + Components using React.forwardRef cannot use @react.componentWithProps. Please use @react.component instead. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/react_component_with_props.res b/tests/build_tests/super_errors/fixtures/react_component_with_props.res new file mode 100644 index 0000000000..29ca3b301b --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/react_component_with_props.res @@ -0,0 +1,15 @@ +module V4C7 = { + @react.componentWithProps + let make = React.forwardRef(( + ~className=?, + ~children, + ref: Js.Nullable.t, + ) => +
+ Belt.Option.map(React.Ref.domRef)} + /> + children +
+ ) +} diff --git a/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt b/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt new file mode 100644 index 0000000000..03dfa09891 --- /dev/null +++ b/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt @@ -0,0 +1,133 @@ +let f = a => Js.Promise.resolve(a + a) + +@@jsxConfig({version: 4, mode: "classic"}) + +module V4C1 = { + type props = sharedProps + let make = props => React.string(props.x ++ props.y) + let make = { + let \"SharedPropsWithProps$V4C1" = props => make(props) + \"SharedPropsWithProps$V4C1" + } +} + +module V4C2 = { + type props = sharedProps + let make = (props: props) => React.string(props.x ++ props.y) + let make = { + let \"SharedPropsWithProps$V4C2" = (props: props) => make(props) + \"SharedPropsWithProps$V4C2" + } +} + +module V4C3 = { + type props<'a> = sharedProps<'a> + let make = ({x, y}: props<_>) => React.string(x ++ y) + let make = { + let \"SharedPropsWithProps$V4C3" = (props: props<_>) => make(props) + \"SharedPropsWithProps$V4C3" + } +} + +module V4C4 = { + type props<'a> = sharedProps + let make = ({x, y}: props<_>) => React.string(x ++ y) + let make = { + let \"SharedPropsWithProps$V4C4" = (props: props<_>) => make(props) + \"SharedPropsWithProps$V4C4" + } +} + +module V4C5 = { + type props<'a> = {a: 'a} + let make = async ({a}: props<_>) => { + let a = await f(a) + ReactDOM.createDOMElementVariadic("div", [{React.int(a)}]) + } + let make = { + let \"SharedPropsWithProps$V4C5" = (props: props<_>) => + JsxPPXReactSupport.asyncComponent(make(props)) + \"SharedPropsWithProps$V4C5" + } +} + +module V4C6 = { + type props<'status> = {status: 'status} + let make = async ({status}: props<_>) => { + switch status { + | #on => React.string("on") + | #off => React.string("off") + } + } + let make = { + let \"SharedPropsWithProps$V4C6" = (props: props<_>) => + JsxPPXReactSupport.asyncComponent(make(props)) + \"SharedPropsWithProps$V4C6" + } +} + +@@jsxConfig({version: 4, mode: "automatic"}) + +module V4A1 = { + type props = sharedProps + let make = props => React.string(props.x ++ props.y) + let make = { + let \"SharedPropsWithProps$V4A1" = props => make(props) + \"SharedPropsWithProps$V4A1" + } +} + +module V4A2 = { + type props = sharedProps + let make = (props: props) => React.string(props.x ++ props.y) + let make = { + let \"SharedPropsWithProps$V4A2" = (props: props) => make(props) + \"SharedPropsWithProps$V4A2" + } +} + +module V4A3 = { + type props<'a> = sharedProps<'a> + let make = ({x, y}: props<_>) => React.string(x ++ y) + let make = { + let \"SharedPropsWithProps$V4A3" = (props: props<_>) => make(props) + \"SharedPropsWithProps$V4A3" + } +} + +module V4A4 = { + type props<'a> = sharedProps + let make = ({x, y}: props<_>) => React.string(x ++ y) + let make = { + let \"SharedPropsWithProps$V4A4" = (props: props<_>) => make(props) + \"SharedPropsWithProps$V4A4" + } +} + +module V4A5 = { + type props<'a> = {a: 'a} + let make = async ({a}: props<_>) => { + let a = await f(a) + ReactDOM.jsx("div", {children: ?ReactDOM.someElement({React.int(a)})}) + } + let make = { + let \"SharedPropsWithProps$V4A5" = (props: props<_>) => + JsxPPXReactSupport.asyncComponent(make(props)) + \"SharedPropsWithProps$V4A5" + } +} + +module V4A6 = { + type props<'status> = {status: 'status} + let make = async ({status}: props<_>) => { + switch status { + | #on => React.string("on") + | #off => React.string("off") + } + } + let make = { + let \"SharedPropsWithProps$V4A6" = (props: props<_>) => + JsxPPXReactSupport.asyncComponent(make(props)) + \"SharedPropsWithProps$V4A6" + } +} diff --git a/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res b/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res new file mode 100644 index 0000000000..00d382bc92 --- /dev/null +++ b/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res @@ -0,0 +1,93 @@ +let f = a => Js.Promise.resolve(a + a) + +@@jsxConfig({version:4, mode: "classic"}) + +module V4C1 = { + type props = sharedProps + @react.componentWithProps + let make = (props) => React.string(props.x ++ props.y) +} + +module V4C2 = { + type props = sharedProps + @react.componentWithProps + let make = (props: props) => React.string(props.x ++ props.y) +} + +module V4C3 = { + type props<'a> = sharedProps<'a> + @react.componentWithProps + let make = ({x, y}: props<_>) => React.string(x ++ y) +} + +module V4C4 = { + type props<'a> = sharedProps + @react.componentWithProps + let make = ({x, y}: props<_>) => React.string(x ++ y) +} + +module V4C5 = { + type props<'a> = {a: 'a} + @react.componentWithProps + let make = async ({a}: props<_>) => { + let a = await f(a) +
{React.int(a)}
+ } +} + +module V4C6 = { + type props<'status> = {status: 'status} + @react.componentWithProps + let make = async ({status}: props<_>) => { + switch status { + | #on => React.string("on") + | #off => React.string("off") + } + } +} + +@@jsxConfig({version:4, mode: "automatic"}) + +module V4A1 = { + type props = sharedProps + @react.componentWithProps + let make = (props) => React.string(props.x ++ props.y) +} + +module V4A2 = { + type props = sharedProps + @react.componentWithProps + let make = (props: props) => React.string(props.x ++ props.y) +} + +module V4A3 = { + type props<'a> = sharedProps<'a> + @react.componentWithProps + let make = ({x, y}: props<_>) => React.string(x ++ y) +} + +module V4A4 = { + type props<'a> = sharedProps + @react.componentWithProps + let make = ({x, y}: props<_>) => React.string(x ++ y) +} + +module V4A5 = { + type props<'a> = {a: 'a} + @react.componentWithProps + let make = async ({a}: props<_>) => { + let a = await f(a) +
{React.int(a)}
+ } +} + +module V4A6 = { + type props<'status> = {status: 'status} + @react.componentWithProps + let make = async ({status}: props<_>) => { + switch status { + | #on => React.string("on") + | #off => React.string("off") + } + } +}