flow: Annotate some more exports.

[chrisbobbe]

Toward #4907.

These are the four items I'd flagged as "tricky." Here, I follow Greg's suggestions for handling them at #4907 (comment). As it turns out, some of them are not all that tricky! 🙂

[gnprice]

Thanks! Comments below.

[gnprice]

nit: can simplify a bit by dropping the inner Promise<Message[]> annotation, which is now redundant

(There are one or two others like this, basically where the return type was something more interesting than void or Promise<void>.)

Also could use the [] shorthand here, as we did in that inner annotation.

[gnprice]

I don't like these new names -- I'd much rather avoid proliferating different names like this. The good thing about the name eg.action.realm_init is that REALM_INIT is already a name that exists in the code, in lines like case REALM_INIT: in reducers. (Maybe it'd even be better if we called it eg.action.REALM_INIT, but that'd be a change orthogonal to this typing change.)

The name RealmInitAction does exist in actionTypes.js, but that didn't matter so much as long as it was an unexported local name that was used just once 🙂

Is the motivation for these names to put the definitions at the top level of the module, in order to add annotations? I'd hope we can annotate just by casting:

export const action = deepFreeze({
  account_switch: ({
    type: ACCOUNT_SWITCH,
    index: 0,
  }: AccountSwitchAction),

(Oh and I guess either ditching the deepFreeze, or moving it inward:

export const action = Object.freeze({
  account_switch: deepFreeze({
    type: ACCOUNT_SWITCH,
    index: 0,
  }: AccountSwitchAction),

because it isn't transparent to the types-first quick pass.)

If that's too much for types-first to see through, we can always just put an object-type annotation at the top level:

export const action: {|
  account_switch: AccountSwitchAction,
  // …
|} = deepFreeze({
  account_switch: {
    type: ACCOUNT_SWITCH,
    index: 0,
  },

[chrisbobbe]

Is the motivation for these names to put the definitions at the top level of the module, in order to add annotations?

It was. But looking back at your message on CZO, I see I misread what you wanted me to try. 😛 I'd thought you were asking for the members of eg.action to be exported individually, IOW, to pull them out into their own exported variables. Now I see you were talking about exporting the types themselves, in a different file, like RealmInitAction.

[gnprice]

On the one hand, this is just making explicit what was already the case, so there's no regression here and I don't want to block it on any more significant cleanup that would be more work.

But as we're adding these, I would like to avoid having it look like we're saying these are just honestly the types. There is something to fix here, and when looking at this function's interface the reader should see that. One quick solution is to propagate the "beware" jsdoc comment that I put on apiCall in that previous commit.

Ooh, or better: instead of empty we can write $FlowFixMe. … Hmm, no, that's better in one way, but it has the major disadvantage that it won't automatically get flagged when we take that RN upgrade that lets fetch have a real type.

Anyway, propagating the comment would be good enough. Another alternative would be to add a line like type FixmeUntypedFetchResult = empty, and say Promise<FixmeUntypedFetchResult>.

[gnprice]

And for this one and savePushToken, it looks like we don't actually inspect the returned result at all -- so this could just be Promise<mixed>.

[gnprice]

This could be Promise<{ status: number, ... }>.

[gnprice]

This one actually doesn't need to be exported at all! So that's the simplest fix for this one.

[chrisbobbe]

Thanks for the review! Revision pushed.

[gnprice]

Thanks! Looks good -- merging.