per-account 4/n: Sort out thunk actions

[gnprice]

This is the next PR in the series after #5030, #5016, #5017, and #5023, produced from the branch described at #5006 (comment) (and after the interlude in #5043, #5053, and #5054.)

From the foreshadowing last time:

After that, we'll convert thunk actions and React components so that they get a PerAccountState rather than a GlobalState.

This PR handles thunk actions. It also causes React components to get a dispatch: Dispatch that provides a PerAccountState to the dispatched thunk actions, rather than a GlobalState.

Coming next, we'll convert React components to get a PerAccountState themselves. After that, we'll start distinguishing plain actions: just like other areas of our code before the start of this PR series, most of them implicitly refer to the active account while others don't, and we'll start tracking which is which so that we can later reinterpret the former group as referring to whichever specific account their caller had in mind.

[chrisbobbe]

Thanks! See a few comments below.

[chrisbobbe]

Hmm, this is a little tricky! 🤔 Thanks for writing it down. Maybe we can talk in the office sometime, to help me understand it better?

I'm not sure how to apply what I know about variance to understand why we have the contravariance sigils - in OwnProps<-C, -SP>.

I think that's because I'm prepared to look at that type declaration and use it to evaluate whether OwnProps<C1, SP1> <: OwnProps<C2 SP2> for a given C1, SP1, C2, and SP2—but then I ask myself "What's a use for evaluating that?", and I can't think of one, and I can't find a flow like that in the code. Is there one?

Then I guess I'm kind of stuck there. It's also not yet very intuitive to me what an "input position" or "output position" is (though I've seen terms like these used several times before), and I don't know what "making a path +, +, -, which multiplies to -" means. Is that last bit borrowing from category theory, or something?

[gnprice]

No, not category theory. I wouldn't so casually draw on that here, I hope 😉 That sentence is pretty shorthandy, though.

Here's the code for reference:

export type OwnProps<-C, -SP, -D> = $ReadOnly<
  BoundedDiff<BoundedDiff<$Exact<ElementConfig<C>>, SP>, {| +dispatch: D |}>,
>;

Then what that thought ending in "multiplies to -" expands to is:

• That place where the type parameter C gets used in the definition of OwnProps is:
  • as the first argument to ElementConfig
  • … the result of which is used as the argument to $Exact
  • … the result of which is used as the first argument to BoundedDiff
  • … the result of which is used as the first argument to (another) BoundedDiff
  • … the result of which is used as the argument to $ReadOnly
  • … the result of which is the definition.
• The variance of each of those type-operators in their respective type parameters is:
  • contravariant, aka input, written -: ElementConfig<-C> (this isn't obvious, more on it below)
  • (… gloss over $Exact)
  • covariant, aka output, written +: BoundedDiff<+U, -L>, and this is being passed as that first parameter U
  • covariant, aka output, written +: BoundedDiff<+U, -L>, and this is again being passed as that first parameter U
  • (… gloss over $ReadOnly)
• So the sequence of variances is -, +, +, reading outward, from where C is used out to the definition.
  • Or +, +, -, reading inward from the definition to that use of C, which is the direction I used in that comment.
• And the mathematical signs -, +, + multiply together to make -. (For example, the numbers -1 * +1 * +1 multiply to -1.)
  • Which is just shorthand for: if you flip something backward an odd number of times, it ends up backward. Whereas if you flip it an even number of times, it ends up facing forward again.
  • And contravariance means that a subtyping relationship between parameters produces a subtyping in the opposite direction between results: S <: T means F<T> <: F<S>. Whereas covariance means it produces a subtyping in the same direction: F<S> <: F<T>.

Perhaps that's helpful? Also happy to discuss in the office tomorrow, where we'll have a whiteboard too.

---

Now, why do I say ElementConfig is contravariant?

… Well, maybe I shouldn't, actually -- I just tried an isolated test case, without all the other stuff around it that I was figuring was getting things confused here, and Flow still says it's an input/output position (aka invariant):

type F<C>     = ElementConfig<C>; // ok
type Fout<+C> = ElementConfig<C>; // error
type Fin<-C>  = ElementConfig<C>; // error

Here's the same test case on flow.org/try.

I think of it as roughly contravariant because a React component taking props of type Props is basically like a function Props => React.Node. In fact, if it's a function component, it's exactly that. And for P1 => React.Node to be a subtype of P2 => React.Node, it's necessary and sufficient that P2 be a subtype of P1.

[gnprice]

We don't actually need the -C here, so I'll take it out given that Flow believes ElementConfig shouldn't be contravariant.

Also it turns out that while Flow does accept treating $Exact as covariant, that's actually unsound -- you can write coerce with it:

type E<+T> = $Exact<T>; // accepted; uh-oh

// We can use that to cast any type to any other, with Flow
// raising not a peep:
function coerce<A, B>(x: A): B {
  // Make an object that has a property `f` returning `x`…
  const yA: E<{ f: () => A, ... }> = { f: () => x };
  // … and pass it off as type {||}, using the supposed covariance of E.
  const y: E<{ ... }> = yA;

  // Now spread that object to sneak its `f` property into
  // an object that supposedly has `f` of a different type.
  const ythrow = { f: () => { throw null } };
  const yB: { f: () => B } = { ...ythrow, ...y };
  // Flow accepts this because it thinks `y` has no properties,
  // so that `yB` gets its `f` from `ythrow`.  But in fact it's from `y`.

  // Now call that `f`.  The value we actually get is `x`,
  // but Flow thinks it has whatever type we chose.
  return yB.f();
}

const a: number = coerce('a');
a.toFixed(); // crash!  but no Flow error anywhere

So that leaves me less eager to gloss over $Exact and treat it as covariant, either 😛

[chrisbobbe]

react-redux types: Propagate dispatch type properly, with BoundedDiff

Hmm, in this bit—

In the old version with `$Diff`, Flow wouldn't complain if a
component expected a `dispatch` prop with too specific a type.

—I think "too specific a type" could be made a little clearer. Is there a universal standard for how specific the type of a dispatch prop should be (I think not)? Or is it more context-dependent, like "…if a component expected a dispatch prop with too specific a type, i.e., a type that was more specific than what a connect call was trying to provide for that prop"?

[gnprice]

is it more context-dependent, like "…if a component expected a dispatch prop with too specific a type, i.e., a type that was more specific than what a connect call was trying to provide for that prop"?

Yes, exactly -- I mean "too specific" in that its expectations are too specific for what it's actually going to get.

When the type is the same as the one that's actually being provided, then the expectations are the same on both sides and that's great.

When the type that's being expected is a supertype of the one being provided, then that's still just fine: S subtype T means S encodes all the same expectations that T does, and possibly some more expectations too. So the code providing the value is promising more than the recipient happens to care about, which is fine.

But if the type that's expected isn't a supertype of the one provided, then that means it encodes some other expectations that aren't in the provided type. The code receiving the value will be believing it can rely on some assumptions that weren't actually promised by the code providing it, so the value provided might not actually satisfy those assumptions. For example if a React component expects { +a: number, +b: string, ... } for some prop, but the caller only thinks it's providing { +b: string, +c: boolean, ... } for that prop, then the actual value passed as the prop may not have a property a, and the child component could crash when it takes that property and tries to use it as a number.

I'll reword a bit to make this more explicit.

[gnprice]

Thanks for the review! Merging, after revisions discussed above. Take a look and see if it's clearer, and we can always revise the comments further.

Also happy to discuss covariance and contravariance when we're next in the office together.