From 69faf406ac885bca5e52d53098cf6e63a69f8d25 Mon Sep 17 00:00:00 2001 From: mununki Date: Sun, 7 Apr 2024 18:34:57 +0900 Subject: [PATCH 1/4] jsx4: fix incorrect type annotation for ref --- jscomp/syntax/src/jsx_v4.ml | 21 +++++++++++++------ .../ppx/react/expected/forwardRef.res.txt | 4 ++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/jscomp/syntax/src/jsx_v4.ml b/jscomp/syntax/src/jsx_v4.ml index 4524ca459c..ce242960d4 100644 --- a/jscomp/syntax/src/jsx_v4.ml +++ b/jscomp/syntax/src/jsx_v4.ml @@ -46,10 +46,12 @@ let safeTypeFromValue valueStr = if valueStr = "" || (valueStr.[0] [@doesNotRaise]) <> '_' then valueStr else "T" ^ valueStr +let refTypeVar loc = Typ.var ~loc "ref" + let refType loc = Typ.constr ~loc - {loc; txt = Ldot (Ldot (Lident "ReactDOM", "Ref"), "currentDomRef")} - [] + {loc; txt = Ldot (Ldot (Lident "Js", "Nullable"), "t")} + [refTypeVar loc] type 'a children = ListLiteral of 'a | Exact of 'a @@ -279,11 +281,11 @@ let makePropsTypeParams ?(stripExplicitOption = false) (* TODO: Worth thinking how about "ref_" or "_ref" usages *) else if label = "ref" then (* - If ref has a type annotation then use it, else `ReactDOM.Ref.currentDomRef. + If ref has a type annotation then use it, else 'ref. For example, if JSX ppx is used for React Native, type would be different. *) match interiorType with - | {ptyp_desc = Ptyp_any} -> Some (refType loc) + | {ptyp_desc = Ptyp_any} -> Some (refTypeVar loc) | _ -> (* Strip explicit Js.Nullable.t in case of forwardRef *) if stripExplicitJsNullableOfRef then stripJsNullable interiorType @@ -1077,7 +1079,14 @@ let mapBinding ~config ~emptyLoc ~pstr_loc ~fileName ~recFlag binding = (* (ref) => expr *) let expression = List.fold_left - (fun expr (_, pattern) -> Exp.fun_ Nolabel None pattern expr) + (fun expr (_, pattern) -> + let pattern = + match pattern.ppat_desc with + | Ppat_var {txt} when txt = "ref" -> + Pat.constraint_ pattern (refType Location.none) + | _ -> pattern + in + Exp.fun_ Nolabel None pattern expr) expression patternsWithNolabel in (* ({a, b, _}: props<'a, 'b>) *) @@ -1293,7 +1302,7 @@ let transformSignatureItem ~config item = psig_loc ((* If there is Nolabel arg, regard the type as ref in forwardRef *) (if !hasForwardRef then - [(true, "ref", [], Location.none, refType Location.none)] + [(true, "ref", [], Location.none, refTypeVar Location.none)] else []) @ namedTypeList) in diff --git a/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt b/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt index 09555aba53..80f843e18b 100644 --- a/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt +++ b/jscomp/syntax/tests/ppx/react/expected/forwardRef.res.txt @@ -187,7 +187,7 @@ module V4A = { ref?: 'ref, } - let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) => + let make = ({?className, children, _}: props<_, _, 'ref>, ref: Js.Nullable.t<'ref>) => ReactDOM.jsxs( "div", { @@ -239,7 +239,7 @@ module V4AUncurried = { ref?: 'ref, } - let make = ({?className, children, _}: props<_, _, ReactDOM.Ref.currentDomRef>, ref) => + let make = ({?className, children, _}: props<_, _, 'ref>, ref: Js.Nullable.t<'ref>) => ReactDOM.jsxs( "div", { From 0ed0376950fbdafdeac351c3a4bfb23e980591bf Mon Sep 17 00:00:00 2001 From: mununki Date: Sun, 7 Apr 2024 19:23:14 +0900 Subject: [PATCH 2/4] update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a0834c43c..8949a6c567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Improve error when using '@deriving(accessors)' on a variant with record arguments. https://github.com/rescript-lang/rescript-compiler/pull/6712 - Stop escaping JSX prop names with hyphens. https://github.com/rescript-lang/rescript-compiler/pull/6705 - Fix trailing undefined for optional parameters not omitted with `@send` and `@new`. https://github.com/rescript-lang/rescript-compiler/pull/6716 +- Fix JSX4 adding the incorrect type annotation for the prop `ref` in React.forwardRef component https://github.com/rescript-lang/rescript-compiler/pull/6718 # 11.1.0-rc.7 From 34d17b77aa4ed0c46ac24bc627f233098c49e2f2 Mon Sep 17 00:00:00 2001 From: mununki Date: Sun, 7 Apr 2024 20:25:36 +0900 Subject: [PATCH 3/4] add test for interface --- .../ppx/react/expected/forwardRef.resi.txt | 127 ++++++++++++++++++ jscomp/syntax/tests/ppx/react/forwardRef.resi | 85 ++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt create mode 100644 jscomp/syntax/tests/ppx/react/forwardRef.resi diff --git a/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt b/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt new file mode 100644 index 0000000000..4b8c51cd83 --- /dev/null +++ b/jscomp/syntax/tests/ppx/react/expected/forwardRef.resi.txt @@ -0,0 +1,127 @@ +@@jsxConfig({version: 3}) + +module V3: { + module FancyInput: { + @obj + external makeProps: ( + ~className: string=?, + ~children: React.element, + ~ref: ReactDOM.Ref.currentDomRef=?, + ~key: string=?, + unit, + ) => { + "className": option, + "children": React.element, + "ref": option, + } = "" + let make: React.componentLike< + { + "className": option, + "children": React.element, + "ref": option, + }, + React.element, + > + } + + module ForwardRef: { + @obj + external makeProps: ( + ~ref: React.ref>=?, + ~key: string=?, + unit, + ) => {"ref": option>>} = "" + let make: React.componentLike< + {"ref": option>>}, + React.element, + > + } +} + +@@jsxConfig({version: 4, mode: "classic"}) + +module V4C: { + module FancyInput: { + type props<'className, 'children, 'ref> = { + className?: 'className, + children: 'children, + ref?: 'ref, + } + + let make: React.componentLike< + props, + React.element, + > + } + + module ForwardRef: { + type props<'ref> = {ref?: 'ref} + + let make: React.componentLike>>, React.element> + } +} + +module V4CUncurried: { + module FancyInput: { + type props<'className, 'children, 'ref> = { + className?: 'className, + children: 'children, + ref?: 'ref, + } + + let make: React.componentLike< + props, + React.element, + > + } + + module ForwardRef: { + type props<'ref> = {ref?: 'ref} + + let make: React.componentLike>>, React.element> + } +} + +@@jsxConfig({version: 4, mode: "automatic"}) + +module V4A: { + module FancyInput: { + type props<'className, 'children, 'ref> = { + className?: 'className, + children: 'children, + ref?: 'ref, + } + + let make: React.componentLike< + props, + React.element, + > + } + + module ForwardRef: { + type props<'ref> = {ref?: 'ref} + + let make: React.componentLike>>, React.element> + } +} + +module V4AUncurried: { + module FancyInput: { + type props<'className, 'children, 'ref> = { + className?: 'className, + children: 'children, + ref?: 'ref, + } + + let make: React.componentLike< + props, + React.element, + > + } + + module ForwardRef: { + type props<'ref> = {ref?: 'ref} + + let make: React.componentLike>>, React.element> + } +} diff --git a/jscomp/syntax/tests/ppx/react/forwardRef.resi b/jscomp/syntax/tests/ppx/react/forwardRef.resi new file mode 100644 index 0000000000..5dba51695e --- /dev/null +++ b/jscomp/syntax/tests/ppx/react/forwardRef.resi @@ -0,0 +1,85 @@ +@@jsxConfig({version: 3}) + +module V3: { + module FancyInput: { + @react.component + let make: ( + ~className: string=?, + ~children: React.element, + ~ref: ReactDOM.Ref.currentDomRef=?, + ) => React.element + } + + module ForwardRef: { + @react.component + let make: (~ref: React.ref>=?) => React.element + } +} + +@@jsxConfig({version: 4, mode: "classic"}) + +module V4C: { + module FancyInput: { + @react.component + let make: ( + ~className: string=?, + ~children: React.element, + ~ref: ReactDOM.Ref.currentDomRef=?, + ) => React.element + } + + module ForwardRef: { + @react.component + let make: (~ref: React.ref>=?) => React.element + } +} + +module V4CUncurried: { + module FancyInput: { + @react.component + let make: ( + ~className: string=?, + ~children: React.element, + ~ref: ReactDOM.Ref.currentDomRef=?, + ) => React.element + } + + module ForwardRef: { + @react.component + let make: (~ref: React.ref>=?) => React.element + } +} + +@@jsxConfig({version: 4, mode: "automatic"}) + +module V4A: { + module FancyInput: { + @react.component + let make: ( + ~className: string=?, + ~children: React.element, + ~ref: ReactDOM.Ref.currentDomRef=?, + ) => React.element + } + + module ForwardRef: { + @react.component + let make: (~ref: React.ref>=?) => React.element + } +} + +module V4AUncurried: { + module FancyInput: { + @react.component + let make: ( + ~className: string=?, + ~children: React.element, + ~ref: ReactDOM.Ref.currentDomRef=?, + ) => React.element + } + + module ForwardRef: { + @react.component + let make: (~ref: React.ref>=?) => React.element + } +} From 05dbe6f101372484d2b8425b8a9b2480a710f152 Mon Sep 17 00:00:00 2001 From: mununki Date: Sun, 7 Apr 2024 20:26:36 +0900 Subject: [PATCH 4/4] remove unnecessary handling logic of ref for interface --- jscomp/syntax/src/jsx_v4.ml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/jscomp/syntax/src/jsx_v4.ml b/jscomp/syntax/src/jsx_v4.ml index ce242960d4..933a63da9d 100644 --- a/jscomp/syntax/src/jsx_v4.ml +++ b/jscomp/syntax/src/jsx_v4.ml @@ -1258,7 +1258,6 @@ let transformSignatureItem ~config item = let pval_type = Jsx_common.extractUncurried pval_type in check_string_int_attribute_iter.signature_item check_string_int_attribute_iter item; - let hasForwardRef = ref false in let coreTypeOfAttr = Jsx_common.coreTypeOfAttrs pval_attributes in let typVarsOfCoreType = coreTypeOfAttr @@ -1277,9 +1276,7 @@ let transformSignatureItem ~config item = (Nolabel, {ptyp_desc = Ptyp_constr ({txt = Lident "unit"}, _)}, rest) -> getPropTypes types rest - | Ptyp_arrow (Nolabel, _type, rest) -> - hasForwardRef := true; - getPropTypes types rest + | Ptyp_arrow (Nolabel, _type, rest) -> getPropTypes types rest | Ptyp_arrow (name, ({ptyp_attributes = attrs} as type_), returnValue) when isOptional name || isLabelled name -> (returnValue, (name, attrs, returnValue.ptyp_loc, type_) :: types) @@ -1299,12 +1296,7 @@ let transformSignatureItem ~config item = in let propsRecordType = makePropsRecordTypeSig ~coreTypeOfAttr ~typVarsOfCoreType "props" - psig_loc - ((* If there is Nolabel arg, regard the type as ref in forwardRef *) - (if !hasForwardRef then - [(true, "ref", [], Location.none, refTypeVar Location.none)] - else []) - @ namedTypeList) + psig_loc namedTypeList in (* can't be an arrow because it will defensively uncurry *) let newExternalType =