Add React lifecycles getDerivedStateFromProps, getSnapshotBeforeUpdate

[motiz88]

Closes #6101, closes #6104.

Broadly speaking, there are three possible approaches for handling getSnapshotBeforeUpdate and componentDidUpdate:

1. Type the snapshot as any and assume the user knows what they're doing (this is what I did here).
2. Type the snapshot as mixed and force the user to cast / "promise" they know what they're doing.
3. Add another type parameter Snapshot to React.Component, see if Flow can infer it from usage, etc.

I thought (3) was overkill for a rarely-used React feature and (2) would mostly add unhelpful noise, hence I stuck with (1). Happy to take guidance from a reviewer if you think another option is preferable though.

[motiz88]

This will have merge conflicts with #6510 which I'd be happy to resolve once either PR is merged.

[mrkev]

Can you link to documentation or code backing this change, to ensure this is the correct definition?

[motiz88]

getDerivedStateFromProps: docs, call site

getSnapshotBeforeUpdate: docs, call site

1. I've just noticed that React emits a warning on gDSFP() === undefined, so we should probably narrow the return type to $Shape<State> | null.
2. gSBU() === undefined also gives a warning but our return type is any per the reasoning above - do we have an idiomatic & working way of expressing "not undefined, but otherwise any"? If not, this might be a justification for typing it as some derivative of mixed.

[mrkev]

1. I agree
2. Hmmm unfortunately $NonMaybeType<mixed> doesn't seem to work (I've opened an issue here $NonMaybeType<mixed> still takes null and undefined #6606). If it worked, we could use $NonMaybeType<mixed> | null, but for the time being I think mixed should do.

any kinda just short-circuits the typesystem-- mixed is generally preferred 👍

[motiz88]

1. So apparently $Shape<T> accepts null and undefined, and $NonMaybeType<$Shape<T>> doesn't help. Happy to still make the change to getDerivedStateFromProps but this will not actually typecheck differently, nor show up in the tests. 😭
2. @mrkev just to confirm that this is what we want, because subjectively I have my concerns: Typing the snapshot as mixed will force users to type it (not very "gradual typing" IMHO), and won't let them do it in what I think is an intuitive way:

   componentDidUpdate(prevProps, prevState, snapshot: number) { // Error: number is incompatible with mixed
     let x: number = snapshot;
   }

   I guess my point is that the snapshot behaves almost like a local variable shared between the two methods - what you put in there is what you get back (except undefined which is disallowed), even though the indirection erases the type information. Which is why, to me, typing it as mixed feels like putting up a barrier for little added value. (I would personally love it if Flow could just infer the snapshot type and enforce consistency between getSnapshotBeforeUpdate and componentDidUpdate somehow... I'm allowed to dream, right?)

[motiz88]

I can get getDerivedStateFromProps to behave if I use ($Shape<State> & Object) | null as the return type though. Would want to include a TODO comment to change it to $NonMaybeType<$Shape<State>> | null as soon as the issue's fixed.

[mrkev]

@motiz88 I think $Shape<State> & Object works because Object works, no? Object is a super type of all objects, which should include anything returned by $Shape<T>. Or does using $Shape<State> & Object only allow the keys on State?

[motiz88]

@mrkev

$Shape<State> & Object represents the intersection of the following sets of values:

1. All non-null objects
2. null | void | /* all objects with the shape of State */

= objects with the shape of State.

Your comment would have been correct if I was referring to $Shape<State> | Object, which reduces to ?Object - not what we want here (we want a non-maybe $Shape<State>)

[motiz88]

Back working on this after a busy few weeks (also helped by the fact that CI builds are working again).

8a7bb7d brings us to what I think is the best of all possible worlds right now:

1. componentDidUpdate still takes snapshot: any, which I subjectively prefer and nobody has definitively rejected following Add React lifecycles getDerivedStateFromProps, getSnapshotBeforeUpdate #6511 (comment).
2. getDerivedStateFromProps returns either null or $Shape<State> excluding undefined. This is the correct behaviour; It's just typed as the temporary $FIXME$NonMaybeObject<$Shape<State>> | null since $NonMaybeType<$Shape<State>> | null doesn't work ($NonMaybeType<mixed> still takes null and undefined #6606).
3. getSnapshotBeforeUpdate returns mixed, which is not ideal because undefined should be disallowed. It's typed as the temporary $FIXME$NonMaybeMixed to draw attention to this fact (and also to "magically" fix things once $NonMaybeType<mixed> still takes null and undefined #6606 is fixed).

[gkz]

We are not updating class component typing at this point.