Skip to content

Commit

Permalink
[react] createElement for AbstractComponent
Browse files Browse the repository at this point in the history
Summary:
This implements createElement for AbstractComponent. The implementation here is a bit hacky, since we already have access to the config.
I explain this in a comment inline.

The reason I didn't just rewrite this entire function to use get_config and then compare the incoming config object with the config type is
because there is some special handling we have for children, keys, and ref that is handled in `object_kit`.

That said, this is something that I want to refactor in the future.

Reviewed By: samwgoldman

Differential Revision: D13221333

fbshipit-source-id: a41e86a4769abf21006d352909cbab4dddb05fa8
  • Loading branch information
jbrown215 authored and facebook-github-bot committed Nov 29, 2018
1 parent 65b550f commit 4528c07
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 7 deletions.
33 changes: 27 additions & 6 deletions src/typing/react_kit.ml
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,33 @@ let run cx trace ~use_op reason_op l u
(* Create a type variable for our props. *)
(* If we are cloning an existing element, the config does not need to
* provide the entire props type. *)
let props = if clone
then ShapeT (Tvar.mk_where cx reason_op props_to_tout)
else Tvar.mk_where cx reason_op tin_to_props
let props, defaults =
match l with
| DefT (_, ReactAbstractComponentT {config; _}) ->
(* This is a bit of a hack. We will be passing these props and
* default props to react_config in flow_js.ml to calculate the
* config and check the passed config against it. Since our config is
* already calculated, we can pretend the props type is the config
* type and that we have no defaultProps for identical behavior.
*
* This hack is necessary because we (by design) do not calculate
* props from Config and DefaultProps. Even if we did do that, we would
* just introduce unnecessary work here-- we would calculate the props from
* the config and defaultProps just so that we could re-calculate the config
* down the line.
*
* Additionally, this hack enables us to not have to explicitly handle
* AbstractComponent past this point. *)
(if clone
then ShapeT (config)
else config), None
| _ ->
(if clone
then ShapeT (Tvar.mk_where cx reason_op props_to_tout)
else Tvar.mk_where cx reason_op tin_to_props),
(* For class components and function components we want to lookup the
* static default props property so that we may add it to our config input. *)
get_defaults cx trace l ~reason_op ~rec_flow
in
(* Check the type of React keys in the config input.
*
Expand Down Expand Up @@ -544,9 +568,6 @@ let run cx trace ~use_op reason_op l u
rec_flow cx trace (config,
LookupT (reason_ref, kind, [], propref, action))
in
(* For class components and function components we want to lookup the
* static default props property so that we may add it to our config input. *)
let defaults = get_defaults cx trace l ~reason_op ~rec_flow 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
* this block we will flow the final config to our props type. *)
Expand Down
10 changes: 10 additions & 0 deletions tests/react_abstract_component/create_element.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//@flow

const React = require('react');

declare var C: React$AbstractComponent<{+foo?: number, +bar: number | string, +baz: number}, mixed, number>;

const _a = <C foo={3} bar="string" baz={4} />;
const _b = <C bar={3} baz={4} />;
const _c = <C baz={4} />; // Error missing bar
const _d = <C bar={3} />; // Error missing baz
44 changes: 43 additions & 1 deletion tests/react_abstract_component/react_abstract_component.exp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,48 @@ References:
^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------------------------- create_element.js:9:12

Cannot create `C` element because property `bar` is missing in props [1] but exists in object type [2].

create_element.js:9:12
9| const _c = <C baz={4} />; // Error missing bar
^^^^^^^^^^^^^ [1]

References:
create_element.js:5:40
5| declare var C: React$AbstractComponent<{+foo?: number, +bar: number | string, +baz: number}, mixed, number>;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------------------------ create_element.js:10:12

Cannot create `C` element because property `baz` is missing in props [1] but exists in object type [2].

create_element.js:10:12
10| const _d = <C bar={3} />; // Error missing baz
^^^^^^^^^^^^^ [1]

References:
create_element.js:5:40
5| declare var C: React$AbstractComponent<{+foo?: number, +bar: number | string, +baz: number}, mixed, number>;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- destructors.js:9:12

Cannot create `C` element because property `bar` is missing in props [1] but exists in object type [2].

destructors.js:9:12
9| const _b = <C baz={3} />; // Error, bar missing
^^^^^^^^^^^^^ [1]

References:
destructors.js:5:40
5| declare var C: React$AbstractComponent<{foo?: number, bar: number}, {foo: number}, void>;
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- destructors.js:14:2

Cannot cast `3` to `React.ElementRef` because number [1] is incompatible with undefined [2].
Expand Down Expand Up @@ -480,7 +522,7 @@ References:



Found 35 errors
Found 38 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches

0 comments on commit 4528c07

Please sign in to comment.