Skip to content

Commit

Permalink
[flow][cleanup] Remove the ability to disable ref-as-prop support
Browse files Browse the repository at this point in the history
Summary:
I still kept the flag around since we still need to implement full support (where React.ElementConfig<...> would also include the ref prop) someday.

Changelog: [errors] The ability to configure `react.ref_as_prop=disabled` is removed.

Reviewed By: panagosg7

Differential Revision: D67478993

fbshipit-source-id: 904f4b6f3572b44fa20a415a2916319a7076b3a9
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Dec 21, 2024
1 parent 09003fe commit f7a104f
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 154 deletions.
6 changes: 1 addition & 5 deletions src/commands/config/flowConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1094,11 +1094,7 @@ module Opts = struct
);
( "react.ref_as_prop",
enum
[
("disabled", Options.ReactRefAsProp.Disabled);
("experimental.partial_support", Options.ReactRefAsProp.PartialSupport);
("partial_support", Options.ReactRefAsProp.PartialSupport);
]
[("partial_support", Options.ReactRefAsProp.PartialSupport)]
(fun opts react_ref_as_prop -> Ok { opts with react_ref_as_prop })
);
("react.runtime", react_runtime_parser);
Expand Down
1 change: 0 additions & 1 deletion src/common/options.ml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ type saved_state_fetcher =

module ReactRefAsProp = struct
type t =
| Disabled (** Pre React 19 behavior *)
| PartialSupport
(** Only implement React 19 behavior for function components.
* Utility types will still return ref and props separately. *)
Expand Down
2 changes: 1 addition & 1 deletion src/flow_dot_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ let stub_metadata ~root ~checked =
natural_inference_exports_primitive_const = false;
no_unchecked_indexed_access = false;
react_custom_jsx_typing = false;
react_ref_as_prop = Options.ReactRefAsProp.Disabled;
react_ref_as_prop = Options.ReactRefAsProp.PartialSupport;
react_runtime = Options.ReactRuntimeAutomatic;
recursion_limit = 10000;
relay_integration_esmodules = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ let stub_metadata ~root ~checked =
natural_inference_exports_primitive_const = false;
no_unchecked_indexed_access = false;
react_custom_jsx_typing = false;
react_ref_as_prop = Options.ReactRefAsProp.Disabled;
react_ref_as_prop = Options.ReactRefAsProp.PartialSupport;
react_runtime = Options.ReactRuntimeClassic;
recursion_limit = 10000;
relay_integration_esmodules = false;
Expand Down
2 changes: 1 addition & 1 deletion src/typing/__tests__/type_hint_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ let metadata =
natural_inference_exports_primitive_const = false;
no_unchecked_indexed_access = false;
react_custom_jsx_typing = false;
react_ref_as_prop = Options.ReactRefAsProp.Disabled;
react_ref_as_prop = Options.ReactRefAsProp.PartialSupport;
react_runtime = Options.ReactRuntimeClassic;
recursion_limit = 10000;
relay_integration_esmodules = false;
Expand Down
2 changes: 1 addition & 1 deletion src/typing/__tests__/typed_ast_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ let metadata =
natural_inference_exports_primitive_const = false;
no_unchecked_indexed_access = false;
react_custom_jsx_typing = false;
react_ref_as_prop = Options.ReactRefAsProp.Disabled;
react_ref_as_prop = Options.ReactRefAsProp.PartialSupport;
react_runtime = Options.ReactRuntimeClassic;
recursion_limit = 10000;
relay_integration_esmodules = false;
Expand Down
164 changes: 70 additions & 94 deletions src/typing/react_kit.ml
Original file line number Diff line number Diff line change
Expand Up @@ -241,28 +241,9 @@ module Kit (Flow : Flow_common.S) : REACT = struct
[MixedT.why r]
|> Option.some
| DefT (_, FunT _)
| DefT (_, ObjT _) ->
(match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.Disabled ->
get_builtin_typeapp
cx
(update_desc_new_reason (fun desc -> RTypeAppImplicit desc) reason_ref)
"React$RefSetter"
[VoidT.why reason_ref]
|> Option.some
| Options.ReactRefAsProp.PartialSupport -> None)
| DefT (_, ReactAbstractComponentT { instance = ComponentInstanceOmitted _; _ })
when match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.PartialSupport -> true
| Options.ReactRefAsProp.Disabled -> false ->
None
| DefT (_, ObjT _)
| DefT (_, ReactAbstractComponentT { instance = ComponentInstanceOmitted _; _ }) ->
get_builtin_typeapp
cx
(update_desc_new_reason (fun desc -> RTypeAppImplicit desc) reason_ref)
"React$RefSetter"
[VoidT.why reason_ref]
|> Option.some
None
| DefT (_, StrT_UNSOUND (_, name)) ->
let instance =
Tvar_resolver.mk_tvar_and_fully_resolve_where cx reason_ref (fun tout ->
Expand Down Expand Up @@ -536,84 +517,78 @@ module Kit (Flow : Flow_common.S) : REACT = struct
)
in
let ref_manipulation =
match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.Disabled ->
(* When ref-as-prop is disabled, we keep the old behavior
* where the ref prop is always filtered away. *)
Object.ReactConfig.FilterRef
| Options.ReactRefAsProp.PartialSupport ->
(match props_of_fn_component l with
| None -> Object.ReactConfig.FilterRef
| Some props ->
(match instance with
| None -> Object.ReactConfig.KeepRef
| Some (ComponentInstanceOmitted _) ->
(* e.g. `component(foo: string)`, which doesn't have ref prop,
* so we just do `{foo: string} ~> function component props` *)
Object.ReactConfig.KeepRef
| Some (ComponentInstanceAvailableAsRefSetterProp ref_t) ->
let r = reason_of_t ref_t in
if definitely_has_ref_in_props cx r props then (
( if Context.in_implicit_instantiation cx then
(* Why do we need to do this when ref_t is added below anyways?
* In implicit instantiation, we might have `fn_component ~> component(ref: infer I, ...infer Props)`
* The ref type will be underconstrained in implicit instantiation,
* so we need the extra flow to constrain it. *)
let fn_component_ref =
EvalT
( props,
TypeDestructorT (use_op, r, PropertyType { name = OrdinaryName "ref" }),
Eval.generate_id ()
)
in
rec_flow_t
cx
trace
~use_op:(Frame (ReactConfigCheck, use_op))
(ref_t, fn_component_ref)
);
(* If we see that the function component has a ref prop,
* then given `component(ref: R, ...Props)`,
* and `(fn_props_has_ref) => React.Node`
* we will do something equivalent to
* {...Props, ref: R} ~> fn_props_has_ref *)
Object.ReactConfig.AddRef ref_t
) else
(* If function component doesn't have a ref prop, technically
* we should do the same thing as above, but it will fail
* ({}) => React.Node ~> component(ref: React.RefSetter<mixed>)
* which previously passes. Therefore, during the transition phase,
* we add this logic to keep the old behavior, where a function
* component is treated as `component(ref: React.RefSetter<void>)` *)
match props_of_fn_component l with
| None -> Object.ReactConfig.FilterRef
| Some props ->
(match instance with
| None -> Object.ReactConfig.KeepRef
| Some (ComponentInstanceOmitted _) ->
(* e.g. `component(foo: string)`, which doesn't have ref prop,
* so we just do `{foo: string} ~> function component props` *)
Object.ReactConfig.KeepRef
| Some (ComponentInstanceAvailableAsRefSetterProp ref_t) ->
let r = reason_of_t ref_t in
if definitely_has_ref_in_props cx r props then (
( if Context.in_implicit_instantiation cx then
(* Why do we need to do this when ref_t is added below anyways?
* In implicit instantiation, we might have `fn_component ~> component(ref: infer I, ...infer Props)`
* The ref type will be underconstrained in implicit instantiation,
* so we need the extra flow to constrain it. *)
let fn_component_ref =
get_builtin_typeapp
cx
(update_desc_new_reason (fun desc -> RTypeAppImplicit desc) r)
"React$RefSetter"
[VoidT.why r]
EvalT
( props,
TypeDestructorT (use_op, r, PropertyType { name = OrdinaryName "ref" }),
Eval.generate_id ()
)
in
rec_flow_t
cx
trace
~use_op:(Frame (ReactConfigCheck, use_op))
(ref_t, fn_component_ref);
Object.ReactConfig.FilterRef
| Some (ComponentInstanceTopType r) ->
(* This is mostly a special case for the above case,
* where ref prop has type `React.RefSetter<mixed>` *)
if definitely_has_ref_in_props cx r props then
Object.ReactConfig.AddRef
(get_builtin_typeapp
cx
(update_desc_new_reason (fun desc -> RTypeAppImplicit desc) r)
"React$RefSetter"
[MixedT.why r]
)
else
(* e.g. The top instance type should accept all ref props,
* including the absense of one, so we can skip the check and just
* filter away the ref prop. *)
Object.ReactConfig.FilterRef))
(ref_t, fn_component_ref)
);
(* If we see that the function component has a ref prop,
* then given `component(ref: R, ...Props)`,
* and `(fn_props_has_ref) => React.Node`
* we will do something equivalent to
* {...Props, ref: R} ~> fn_props_has_ref *)
Object.ReactConfig.AddRef ref_t
) else
(* If function component doesn't have a ref prop, technically
* we should do the same thing as above, but it will fail
* ({}) => React.Node ~> component(ref: React.RefSetter<mixed>)
* which previously passes. Therefore, during the transition phase,
* we add this logic to keep the old behavior, where a function
* component is treated as `component(ref: React.RefSetter<void>)` *)
let fn_component_ref =
get_builtin_typeapp
cx
(update_desc_new_reason (fun desc -> RTypeAppImplicit desc) r)
"React$RefSetter"
[VoidT.why r]
in
rec_flow_t
cx
trace
~use_op:(Frame (ReactConfigCheck, use_op))
(ref_t, fn_component_ref);
Object.ReactConfig.FilterRef
| Some (ComponentInstanceTopType r) ->
(* This is mostly a special case for the above case,
* where ref prop has type `React.RefSetter<mixed>` *)
if definitely_has_ref_in_props cx r props then
Object.ReactConfig.AddRef
(get_builtin_typeapp
cx
(update_desc_new_reason (fun desc -> RTypeAppImplicit desc) r)
"React$RefSetter"
[MixedT.why r]
)
else
(* e.g. The top instance type should accept all ref props,
* including the absense of one, so we can skip the check and just
* filter away the ref prop. *)
Object.ReactConfig.FilterRef)
in
(* Use object spread to add children to config (if we have children)
* and remove key and ref since we already checked key and ref. Finally in
Expand Down Expand Up @@ -934,7 +909,8 @@ module Kit (Flow : Flow_common.S) : REACT = struct
inferred_targs
specialized_component
tout
| ConfigCheck { props = jsx_props; instance } -> config_check use_op ~instance ~jsx_props
| ConfigCheck { props = jsx_props; instance } ->
config_check use_op ~instance:(Some instance) ~jsx_props
| GetProps tout -> props_to_tout tout
| GetConfig tout -> get_config tout
| GetRef tout -> get_instance tout
Expand Down
52 changes: 4 additions & 48 deletions src/typing/subtyping_kit.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1570,32 +1570,10 @@ module Make (Flow : INPUT) : OUTPUT = struct
rec_flow
cx
trace
( l,
ReactKitT
( use_op,
reasonl,
React.ConfigCheck
{
props = config;
instance =
(match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.Disabled -> None
| Options.ReactRefAsProp.PartialSupport -> Some instance);
}
)
);
(l, ReactKitT (use_op, reasonl, React.ConfigCheck { props = config; instance }));

(* check rendered elements are covariant *)
rec_flow_t cx trace ~use_op (return_t, renders);

(match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.Disabled ->
flow_react_component_instance_to_instance
cx
trace
use_op
(ComponentInstanceOmitted (replace_desc_new_reason RVoid reasonl), instance)
| Options.ReactRefAsProp.PartialSupport -> ())
rec_flow_t cx trace ~use_op (return_t, renders)
(* Object Component ~> AbstractComponent *)
| ( DefT (reasonl, ObjT { call_t = Some id; _ }),
DefT
Expand All @@ -1606,20 +1584,7 @@ module Make (Flow : INPUT) : OUTPUT = struct
rec_flow
cx
trace
( l,
ReactKitT
( use_op,
reasonl,
React.ConfigCheck
{
props = config;
instance =
(match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.Disabled -> None
| Options.ReactRefAsProp.PartialSupport -> Some instance);
}
)
);
(l, ReactKitT (use_op, reasonl, React.ConfigCheck { props = config; instance }));

(* Ensure the callable signature's return type is compatible with the rendered element (renders). We
* do this by flowing it to (...empty): renders *)
Expand All @@ -1634,16 +1599,7 @@ module Make (Flow : INPUT) : OUTPUT = struct
renders
in
let mixed = MixedT.why reasonu in
rec_flow_t ~use_op cx trace (Context.find_call cx id, DefT (reasonu, FunT (mixed, funtype)));

(match Context.react_ref_as_prop cx with
| Options.ReactRefAsProp.Disabled ->
flow_react_component_instance_to_instance
cx
trace
use_op
(ComponentInstanceOmitted (replace_desc_new_reason RVoid reasonl), instance)
| Options.ReactRefAsProp.PartialSupport -> ())
rec_flow_t ~use_op cx trace (Context.find_call cx id, DefT (reasonu, FunT (mixed, funtype)))
(* AbstractComponent ~> AbstractComponent *)
| ( DefT
( reasonl,
Expand Down
2 changes: 1 addition & 1 deletion src/typing/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3079,7 +3079,7 @@ and React : sig
}
| ConfigCheck of {
props: TypeTerm.t;
instance: TypeTerm.component_instance option;
instance: TypeTerm.component_instance;
}
| GetProps of TypeTerm.t_out
| GetConfig of TypeTerm.t_out
Expand Down
1 change: 0 additions & 1 deletion tests/react_ref_as_prop/.flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
all=true
no_flowlib=false
react.runtime=automatic
react.ref_as_prop=partial_support

0 comments on commit f7a104f

Please sign in to comment.