Skip to content

Commit

Permalink
connect: Update libdef, and add our own nontrivial wrapper.
Browse files Browse the repository at this point in the history
The libdef here is the updated one from `flow-typed`.  In that repo
it's marked as being for Flow v0.89 and up... but in reality it works
just as well with our current Flow v0.78.

OTOH the libdef has a couple of limitations:

 * It doesn't seem to understand React's `defaultProps` mechanism;
   so if the inner component has props that are made optional by
   having defaults, the outer component ends up requiring them at
   call sites.

 * It leaves some type variables in "input positions" that really
   ought to be inferrable from the arguments passed to `connect`.

   When such a type makes it into a module export, Flow wants an
   annotation for it, and a change in Flow v0.85 makes it much more
   consistent about insisting on such annotations:
     https://medium.com/flow-type/asking-for-required-annotations-64d4f9c1edf8

   With the libdef's type for `connect`, that means extra
   boilerplate to redundantly spell out the props that the outer
   component accepts, that `mapStateToProps` returns, etc.

So, we also add a wrapper that fixes these limitations:

 * For `defaultProps`, Flow has a quite effective built-in hack,
   called `ElementConfig`.  We just need to use it.

 * Fixing the boilerplate-annotations issue was trickier, but was
   accomplished by eliminating the "input" types (notably that of
   the outer component's "own props") as type variables, and instead
   carefully writing out how to infer them from other variables.

The result is that even with Flow v0.92 (the version associated with
React Native v0.59), no new annotation is required at most call sites!
(The one exception is the return type of `mapStateToProps`, and only
in cases where it takes its second, "props", argument.)

And, at the same time, we get more effective type-checking of
`connect` calls than we've ever had before:

 * Flow *does* give an error if the selector returns a wrong type.
   This is true even if the wrong type happens to be a supertype
   of what's in Props (e.g., `?Dimensions` for `Dimensions`).

 * Bonus: if the selector returns a *subtype* of what Props asks
   for -- i.e., it provides all the needed assumptions *and then
   some* -- there is no error.  That bit is easy to get wrong with
   $Diff; that's what the `$ObjMap<..., () => mixed>` is handling.

 * Also if a prop is missing entirely, Flow gives an error, just
   as in the status quo.  (The error is at call sites, because it
   just lets the prop through into the outer props.)

[Comments mainly written 2019-04-08; commit message 04-11.]
  • Loading branch information
gnprice committed Apr 17, 2019
1 parent 7208ed0 commit f8c9810
Show file tree
Hide file tree
Showing 2 changed files with 279 additions and 206 deletions.
Loading

0 comments on commit f8c9810

Please sign in to comment.