Skip to content

Commit

Permalink
[flow][ref-as-prop] Make React.ComponentType's ref prop behave like…
Browse files Browse the repository at this point in the history 
… `ref?: empty`

Summary:
This diff continues to make progress to move towards full support for ref-as-prop instead of staying in the current half-done state where the ref prop is still too special.

In this diff, we aim to make `React.ComponentType<{}>` behave like `component(ref?: empty)` for subtyping. `React.ComponentType<...>` is always supposed to be the top type in terms of component instance. Previously before ref-as-prop, it behaves like `component(ref?: React.RefSetter<mixed>)` because the ref prop is special and it's always a ref-setter, but with ref-as-prop, it should be the same as all other props, similar to how  `React.ComponentType<empty>` is the top type of all components.

I was able to make the change for most of the subtyping rule, except for the `React.ComponentType<{}> ~> component(ref: ref_prop)` one because the error diff is too big (see D69706346) and we cannot fix the codebase without this diff as a pre-req. This is relatively harmless, because ref-prop is still kinda special. But once a new Flow version with this diff is deployed, we will be able to start replacing most of the generic `component(ref: React.RefSetter<GenericInstance>, ...Props)` with `component(ref: GenericRefProp, ...Props)` so that we can continue the rollout.

Changelog: [internal]

Reviewed By: jbrown215

Differential Revision: D69706347

fbshipit-source-id: 8c8f2ebad492f9ba8e8e2dead6c8b253a8e2ba84
  • Loading branch information
@SamChou19815 @facebook-github-bot
SamChou19815 authored and facebook-github-bot committed Feb 23, 2025
1 parent 6eac3bb commit 9d417a3
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 132 deletions.
32 changes: 13 additions & 19 deletions src/typing/subtyping_kit.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,22 +593,20 @@ module Make (Flow : INPUT) : OUTPUT = struct
(* ref prop is contravariantly typed. We need to flip the flow. *)
rec_flow_t cx trace ~use_op (r, l)
(* component() (treated as component(ref?: React.RefSetter<void>))
* ~> React.ComponentType<{}> (equivalent to component(ref?: React.RefSetter<mixed>))
* ~> React.ComponentType<{}> (equivalent to component(ref?: empty))
*
* This is allowed since void ~> mixed, but only temporarily, because `component()` being treated
* as component(ref?: React.RefSetter<void>) is a result of intermediate ref-as-prop migration.
* In the future, it should be banned, in the same way that
* `{+ref?: React.RefSetter<mixed>} ~> {}` is banned. *)
* Technically `{+ref?: empty} ~> {}` should banned, but it's relatively benign,
* so we allow it for now. *)
| (ComponentInstanceOmitted _, ComponentInstanceTopType _) -> ()
(* React.ComponentType<{}> (equivalent to component(ref?: React.RefSetter<mixed>)) ~> component()
(* React.ComponentType<{}> (equivalent to component(ref?: empty)) ~> component()
*
* If `component()` is treated as component(ref?: React.RefSetter<void>), it will fail due to
* `mixed ~> void`. However, this treatement is temporary while we are moving to full ref-as-prop
* model, in the ref as prop model, the subtyping behavior should be
*
* React.ComponentType<{}> (equivalent to component(ref?: React.RefSetter<mixed>)) ~> component()
* -> component(ref?: React.RefSetter<mixed>) ~> component()
* -> {} ~> {+ref?: React.RefSetter<mixed>}
* React.ComponentType<{}> (equivalent to component(ref?: empty)) ~> component()
* -> component(ref?: empty) ~> component()
* -> {} ~> {+ref?: empty}
* -> OK!
* *)
| (ComponentInstanceTopType _, ComponentInstanceOmitted _) -> ()
Expand All @@ -621,14 +619,8 @@ module Make (Flow : INPUT) : OUTPUT = struct
* Allowed since ``{} ~> {+ref?: ref_prop} *)
| (ComponentInstanceAvailableAsRefSetterProp _, ComponentInstanceOmitted _) -> ()
(* component(ref: ref_prop)
* ~> React.ComponentType<{}> (equivalent to component(ref?: React.RefSetter<mixed>)) *)
| (ComponentInstanceAvailableAsRefSetterProp ref_prop, ComponentInstanceTopType r_mixed) ->
let instance = MixedT.why r_mixed in
(match try_extract_instance_of_ref_setter cx trace ref_prop with
| Some t -> rec_flow_t cx trace ~use_op (t, instance)
| None ->
(* RefSetter is contravariantly typed. We need to flip the flow. *)
rec_flow_t cx trace ~use_op (react_ref_setter_of cx instance, ref_prop))
* ~> React.ComponentType<{}> (equivalent to component(ref?: empty)) *)
| (ComponentInstanceAvailableAsRefSetterProp _, ComponentInstanceTopType _) -> ()
(* component() (equivalent to component(ref?: React.RefSetter<void>))
* ~> component(ref: ref_prop) *)
| (ComponentInstanceOmitted r, ComponentInstanceAvailableAsRefSetterProp ref_prop) ->
Expand All @@ -637,8 +629,10 @@ module Make (Flow : INPUT) : OUTPUT = struct
| None ->
(* RefSetter is contravariantly typed. We need to flip the flow. *)
rec_flow_t cx trace ~use_op (ref_prop, react_ref_setter_of cx (DefT (r, VoidT))))
(* React.ComponentType<{}> (equivalent to component(ref?: React.RefSetter<mixed>))
* ~> component(ref: ref_prop) *)
(* React.ComponentType<{}> (equivalent to component(ref?: empty)) ~> component(ref: ref_prop)
*
* Should be banned since ref_prop ~> empty will almost always fail, but we allow it for now
* since it's needed for incremental migration and it's relatively harmless. *)
| (ComponentInstanceTopType r_mixed, ComponentInstanceAvailableAsRefSetterProp ref_prop) ->
let instance = MixedT.why r_mixed in
(match try_extract_instance_of_ref_setter cx trace ref_prop with
Expand Down
11 changes: 1 addition & 10 deletions src/typing/ty_normalizer.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1250,16 +1250,7 @@ module Make (I : INPUT) : S = struct
match instance with
| Type.ComponentInstanceOmitted _ -> return None
| Type.ComponentInstanceAvailableAsRefSetterProp t -> type__ ~env t >>| Base.Option.some
| Type.ComponentInstanceTopType _ ->
return
(Some
(Ty.Generic
( Ty_symbol.builtin_symbol (Reason.OrdinaryName "React.RefSetter"),
Ty.TypeAliasKind,
Some [Ty.Top]
)
)
)
| Type.ComponentInstanceTopType _ -> return (Some Ty.(Bot EmptyType))
in
let%bind renders =
match renders with
Expand Down
2 changes: 1 addition & 1 deletion src/typing/type_sig_merge.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ and merge_annot env file = function
reason
in
let instance =
Type.ComponentInstanceTopType (mk_default_type_argument_reason_at_position Reason.RMixed 2)
Type.ComponentInstanceTopType (mk_default_type_argument_reason_at_position Reason.REmpty 2)
in
let renders =
let reason =
Expand Down
102 changes: 1 addition & 101 deletions tests/component_syntax/component_syntax.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,31 +178,6 @@ References:
^^^^^^^^^^^^^^^^^^^ [3]


Error ----------------------------------------------------------------------------------------------- annotation.js:49:2

Cannot instantiate `React.Element` because in type argument `ElementType`: [incompatible-type-arg]
- Either component [1] is incompatible with string [2].
- Or object type [3] is incompatible with number [4].

annotation.js:49:2
49| <InlineRef ref={1} />; // error: string and number refs are still not allowed
^^^^^^^^^

References:
annotation.js:48:24
48| declare var InlineRef: component(ref: number); // error
^^^^^^^^^^^^^^^^^^^^^^ [1]
<BUILTINS>/react.js:131:5
131| | string
^^^^^^ [2]
<BUILTINS>/react.js:192:5
192| | { -current: T | null, ... }
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [3]
annotation.js:48:39
48| declare var InlineRef: component(ref: number); // error
^^^^^^ [4]


Error ---------------------------------------------------------------------------------------------- annotation.js:51:40

Components do not support ref properties [1] within spreads [2] [invalid-component-prop]
Expand Down Expand Up @@ -1209,31 +1184,6 @@ References:
^^^^^^^^^^^^^^^^^^^ [3]


Error ------------------------------------------------------------------------------------------------- declared.js:51:2

Cannot instantiate `InlineRef` element because in type argument `ElementType`: [incompatible-type-arg]
- Either component InlineRef [1] is incompatible with string [2].
- Or object type [3] is incompatible with number [4].

declared.js:51:2
51| <InlineRef ref={1} />; // error: string and number refs are still not allowed
^^^^^^^^^

References:
declared.js:50:1
50| declare component InlineRef(ref: number); // error
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
<BUILTINS>/react.js:131:5
131| | string
^^^^^^ [2]
<BUILTINS>/react.js:192:5
192| | { -current: T | null, ... }
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [3]
declared.js:50:34
50| declare component InlineRef(ref: number); // error
^^^^^^ [4]


Error ------------------------------------------------------------------------------------------------ declared.js:53:35

Components do not support ref properties [1] within spreads [2] [invalid-component-prop]
Expand Down Expand Up @@ -1901,31 +1851,6 @@ References:
^^^^^^ [2]


Error --------------------------------------------------------------------------------------------------- import.js:37:2

Cannot instantiate `NoRef` element because in type argument `ElementType`: [incompatible-type-arg]
- Either component NoRef [1] is incompatible with string [2].
- Or object type [3] is incompatible with string [4].

import.js:37:2
37| <NoRef /> // error again because ref in NoRef is bad
^^^^^

References:
names.js:22:8
22| export component NoRef(ref: string) { return <div /> }; // Error
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
<BUILTINS>/react.js:131:5
131| | string
^^^^^^ [2]
<BUILTINS>/react.js:192:5
192| | { -current: T | null, ... }
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [3]
names.js:22:29
22| export component NoRef(ref: string) { return <div /> }; // Error
^^^^^^ [4]


Error --------------------------------------------------------------------------------------------------- import.js:42:2

Cannot create `ExportType` element because property `x` is missing in props [1] but exists in props of component [2].
Expand Down Expand Up @@ -2240,31 +2165,6 @@ References:
^^^^^^^^^^^^^^^^^^^ [3]


Error ---------------------------------------------------------------------------------------------------- names.js:23:2

Cannot instantiate `NoRef` element because in type argument `ElementType`: [incompatible-type-arg]
- Either component NoRef [1] is incompatible with string [2].
- Or object type [3] is incompatible with string [4].

names.js:23:2
23| <NoRef /> // error again due to bad ref
^^^^^

References:
names.js:22:8
22| export component NoRef(ref: string) { return <div /> }; // Error
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]
<BUILTINS>/react.js:131:5
131| | string
^^^^^^ [2]
<BUILTINS>/react.js:192:5
192| | { -current: T | null, ... }
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [3]
names.js:22:29
22| export component NoRef(ref: string) { return <div /> }; // Error
^^^^^^ [4]


Error --------------------------------------------------------------------------------------------------- names.js:25:35

Components do not support ref properties [1] within spreads [2] [invalid-component-prop]
Expand Down Expand Up @@ -3807,7 +3707,7 @@ Strict mode function may not have duplicate parameter names



Found 243 errors
Found 239 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches
2 changes: 1 addition & 1 deletion tests/type_at_pos_react/type_at_pos_react.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ React$CreateElement

react.js:4:7,4:19

react_abstract_component.js:3:15 = component(ref: React.RefSetter<mixed>, ...any)
react_abstract_component.js:3:15 = component(ref: empty, ...any)
react_abstract_component.js:3:15,3:15
react_abstract_component.js:8:15 = type RendersFoo = renders Foo

Expand Down

0 comments on commit 9d417a3

Please sign in to comment.