From 5c27c6373d20c0c41ffdfe7bbc51cbc9f082506e Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 21 Oct 2026 13:19:40 +0000 Subject: [PATCH] Accept a payload on @react.componentWithProps with a deprecation warning #8884 made a payload on @react.componentWithProps an error, breaking code that compiled before; the payload was always ignored. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GPwM4m3vhYgbdzgXvmQ7Te Signed-off-by: Christoph Knittel --- .../component-with-props-payload.polish.md | 1 + compiler/syntax/src/jsx_common.ml | 51 +++++++++++++++---- compiler/syntax/src/jsx_v4.ml | 17 ++++++- ...nentWithProps_invalid_payload.res.expected | 11 ---- ...ct_componentWithProps_payload.res.expected | 11 ++++ ...act_componentWithProps_invalid_payload.res | 6 --- .../react_componentWithProps_payload.res | 16 ++++++ .../expected/sharedPropsWithProps.res.txt | 24 +++++++++ .../data/ppx/react/sharedPropsWithProps.res | 8 +++ 9 files changed, 115 insertions(+), 30 deletions(-) create mode 100644 changes/component-with-props-payload.polish.md delete mode 100644 tests/build_tests/super_errors/expected/react_componentWithProps_invalid_payload.res.expected create mode 100644 tests/build_tests/super_errors/expected/react_componentWithProps_payload.res.expected delete mode 100644 tests/build_tests/super_errors/fixtures/react_componentWithProps_invalid_payload.res create mode 100644 tests/build_tests/super_errors/fixtures/react_componentWithProps_payload.res diff --git a/changes/component-with-props-payload.polish.md b/changes/component-with-props-payload.polish.md new file mode 100644 index 00000000000..bb3ace2a3e8 --- /dev/null +++ b/changes/component-with-props-payload.polish.md @@ -0,0 +1 @@ +- Accept a payload on `@react.componentWithProps` again, as in 12.x, with a deprecation warning: it never had an effect. diff --git a/compiler/syntax/src/jsx_common.ml b/compiler/syntax/src/jsx_common.ml index 7d6dd48450c..320efb02d40 100644 --- a/compiler/syntax/src/jsx_common.ml +++ b/compiler/syntax/src/jsx_common.ml @@ -8,6 +8,36 @@ type jsx_config = { mutable has_component: bool; } +(* The JSX transform used to ignore a payload on [@react.componentWithProps], + so code may carry one, such as [(: props)] copied from [@react.component]. + The node cannot hold it, so the attribute stays for formatting to keep; take + it here as the component, with a deprecation warning, unless the definition + already has one. *) +let take_component_with_props_payload jsx_component attributes = + match jsx_component with + | Some _ -> (jsx_component, attributes) + | None -> + let rec go seen = function + | [] -> (None, attributes) + | ((({txt; loc}, _) : attribute) as attr) :: rest -> ( + let namespace = + match txt with + | "react.componentWithProps" -> Some Pjc_react + | "jsx.componentWithProps" -> Some Pjc_jsx + | _ -> None + in + match namespace with + | Some pjc_namespace + when Option.is_none (Ast_helper.Jsx_component.of_attribute attr) -> + Location.deprecated ~use:loc loc + (Printf.sprintf + "@%s takes no payload. The payload has no effect; remove it." txt); + ( Some {pjc_kind = Pjc_with_props; pjc_namespace; pjc_loc = loc}, + List.rev_append seen rest ) + | _ -> go (attr :: seen) rest) + in + go [] attributes + (* [Ast_helper] takes the first [@react.component], or failing that the first [@react.componentWithProps], into the binding's or description's component node. One still among the attributes is a second annotation, or one with a @@ -19,18 +49,17 @@ let reject_leftover_component_attrs attributes = match txt with | "react.component" | "jsx.component" | "react.componentWithProps" | "jsx.componentWithProps" -> ( - match Ast_helper.Jsx_component.of_attribute attr with - | Some _ -> + match (txt, Ast_helper.Jsx_component.of_attribute attr) with + | ("react.component" | "jsx.component"), None -> Location.raise_errorf ~loc - "A definition can have only one JSX component annotation." - | None -> ( - match txt with - | "react.component" | "jsx.component" -> - Location.raise_errorf ~loc - "@@%s takes no payload, or the props type to share, as in @@%s(: \ - props)." - txt txt - | _ -> Location.raise_errorf ~loc "@@%s takes no payload." txt)) + "@@%s takes no payload, or the props type to share, as in @@%s(: \ + props)." + txt txt + | _ -> + (* [take_component_with_props_payload] has taken a lone + [@react.componentWithProps] whatever its payload. *) + Location.raise_errorf ~loc + "A definition can have only one JSX component annotation.") | _ -> ()) attributes diff --git a/compiler/syntax/src/jsx_v4.ml b/compiler/syntax/src/jsx_v4.ml index c36d55c754e..b23f24e32a9 100644 --- a/compiler/syntax/src/jsx_v4.ml +++ b/compiler/syntax/src/jsx_v4.ml @@ -563,6 +563,11 @@ let map_binding ~config ~empty_loc ~pstr_loc ~file_name binding = {expr with pexp_desc = Pexp_sequence (first, constrain_jsx_return second)} | _ -> jsx_element_constraint expr in + let pvb_jsx_component, pvb_attributes = + Jsx_common.take_component_with_props_payload binding.pvb_jsx_component + binding.pvb_attributes + in + let binding = {binding with pvb_jsx_component; pvb_attributes} in Jsx_common.reject_leftover_component_attrs binding.pvb_attributes; match binding.pvb_jsx_component with | Some {pjc_kind = Pjc_labelled core_type_of_attr} -> @@ -948,8 +953,12 @@ let transform_structure_item ~config item = pstr_desc = Pstr_primitive ({pval_attributes; pval_type} as value_description); } as pstr -> ( + let pval_jsx_component, pval_attributes = + Jsx_common.take_component_with_props_payload + value_description.pval_jsx_component pval_attributes + in Jsx_common.reject_leftover_component_attrs pval_attributes; - match value_description.pval_jsx_component with + match pval_jsx_component with | None -> [item] | Some {pjc_kind = Pjc_with_props} -> Jsx_common.raise_error ~loc:pstr_loc @@ -1048,8 +1057,12 @@ let transform_signature_item ~config item = psig_loc; psig_desc = Psig_value ({pval_attributes; pval_type} as psig_desc); } as psig -> ( + let pval_jsx_component, pval_attributes = + Jsx_common.take_component_with_props_payload psig_desc.pval_jsx_component + pval_attributes + in Jsx_common.reject_leftover_component_attrs pval_attributes; - match psig_desc.pval_jsx_component with + match pval_jsx_component with (* [@react.componentWithProps] on a signature item has always been left alone. *) | None | Some {pjc_kind = Pjc_with_props} -> [item] diff --git a/tests/build_tests/super_errors/expected/react_componentWithProps_invalid_payload.res.expected b/tests/build_tests/super_errors/expected/react_componentWithProps_invalid_payload.res.expected deleted file mode 100644 index e3d44b03fca..00000000000 --- a/tests/build_tests/super_errors/expected/react_componentWithProps_invalid_payload.res.expected +++ /dev/null @@ -1,11 +0,0 @@ - - We've found a bug for you! - /.../fixtures/react_componentWithProps_invalid_payload.res:4:3-27 - - 2 │ - 3 │ module C = { - 4 │ @react.componentWithProps(: props) - 5 │ let make = (props: props) => React.string(props.label) - 6 │ } - - @react.componentWithProps takes no payload. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/react_componentWithProps_payload.res.expected b/tests/build_tests/super_errors/expected/react_componentWithProps_payload.res.expected new file mode 100644 index 00000000000..51e48fcbea2 --- /dev/null +++ b/tests/build_tests/super_errors/expected/react_componentWithProps_payload.res.expected @@ -0,0 +1,11 @@ + + Warning number 3 + /.../fixtures/react_componentWithProps_payload.res:12:3-27 + + 10 │ + 11 │ module C = { + 12 │ @react.componentWithProps(: props) + 13 │ let make = (props: props) => React.string(props.label) + 14 │ } + + deprecated: @react.componentWithProps takes no payload. The payload has no effect; remove it. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/react_componentWithProps_invalid_payload.res b/tests/build_tests/super_errors/fixtures/react_componentWithProps_invalid_payload.res deleted file mode 100644 index 6423d6be30c..00000000000 --- a/tests/build_tests/super_errors/fixtures/react_componentWithProps_invalid_payload.res +++ /dev/null @@ -1,6 +0,0 @@ -type props = {label: string} - -module C = { - @react.componentWithProps(: props) - let make = (props: props) => React.string(props.label) -} diff --git a/tests/build_tests/super_errors/fixtures/react_componentWithProps_payload.res b/tests/build_tests/super_errors/fixtures/react_componentWithProps_payload.res new file mode 100644 index 00000000000..da103ac5f2b --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/react_componentWithProps_payload.res @@ -0,0 +1,16 @@ +module React = { + type element = Jsx.element + type componentLike<'props, 'return> = 'props => 'return + type component<'props> = Jsx.component<'props> + external component: componentLike<'props, element> => component<'props> = "%component_identity" + external string: string => element = "%identity" +} + +type props = {label: string} + +module C = { + @react.componentWithProps(: props) + let make = (props: props) => React.string(props.label) +} + +let c = C.make diff --git a/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt b/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt index 1818ef31f27..91eea5058fc 100644 --- a/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt +++ b/tests/syntax_tests/data/ppx/react/expected/sharedPropsWithProps.res.txt @@ -1,3 +1,16 @@ + + Warning number 3 + syntax_tests/data/ppx/react/sharedPropsWithProps.res:63:3-27 + + 61 │ + 62 │ // The payload has no effect; it is accepted with a deprecation warni + │ ng. + 63 │ @react.componentWithProps(: props) + 64 │ let make = props => React.string(props.x) + 65 │ } + + deprecated: @react.componentWithProps takes no payload. The payload has no effect; remove it. + let f = a => Promise.resolve(a + a) @@jsxConfig({version: 4}) @@ -76,3 +89,14 @@ module V4A7 = { \"SharedPropsWithProps$V4A7" }) } + +module V4A8 = { + type props = {x: string} + + // The payload has no effect; it is accepted with a deprecation warning. + let make = (props): React.element => React.string(props.x) + let make = React.component({ + let \"SharedPropsWithProps$V4A8" = (props): React.element => make(props) + \"SharedPropsWithProps$V4A8" + }) +} diff --git a/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res b/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res index 1e0684b4804..fa9bfb61d20 100644 --- a/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res +++ b/tests/syntax_tests/data/ppx/react/sharedPropsWithProps.res @@ -55,3 +55,11 @@ module V4A7 = { React.int(props.count) } } + +module V4A8 = { + type props = {x: string} + + // The payload has no effect; it is accepted with a deprecation warning. + @react.componentWithProps(: props) + let make = props => React.string(props.x) +}