WIP: Upgrade to RN 0.59

[borisyankov]

Addresses #3399

Fixes #3381

[gnprice]

Thanks @borisyankov !

Just reviewed #3296 and made some comments there; looks quite close.

This branch is marked WIP, so I won't review in detail. High-level feedback:

• Would you add a few words about how you determined each of these changes? E.g., why this particular set of version upgrades, and why these particular changes to the Babel config files?

• Ideally this would meaning explaining the motivation for these changes, and what they mean.

  If you're not sure of those and it's hard to figure them out: how did you determine the changes? Did you use a tool -- if so, what commands did you use?

• Do things actually work after the version-upgrade commit? Or if not: what's broken?

  As you know, my strong preference is to keep things not-broken at every commit. Sometimes an upgrade like this can make that hard... but if we do make exceptions, it's important that they be super explicit. That way at least the effect on trying to read and understand the changes is mitigated.

[borisyankov]

The JavaScriptCore.framework commit is definitely needed for iOS to work.
There are few changes needed for Android that are more complicated as our config does not match the default RN one anymore.

[gnprice]

The JavaScriptCore.framework commit is definitely needed for iOS to work.

Thanks, good to know!

Can that commit come before the upgrade?

(There isn't much of a commit message in this WIP version -- and I don't really understand what the change is doing.)

If so, that'd be much preferable. (But as above, at a bare minimum if we can't do any better then this fact should be super explicit in the commit message where we do the upgrade.)

[borisyankov]

It might be. I will explore that.
The WebView upgrade become a bit more complicated so focusing on that first.

[gnprice]

Just merged #3296, which was blocking this.

[gnprice]

As I mentioned on #3428, I just spent a bit of time looking at the Flow aspect of this. Folding in here the relevant points:

• At the tip of the branch, there are 188 errors, and 130 of them are related to react-redux's connect.
• I've previously spent some time digging into that rather gnarly type -- which, it turns out, isn't even right. Let's have me pick that part of the problem up, to make use of my previous investigations. Looks like it's triggered just by upgrading Flow, even with no other changes.
• Handy command for summarizing Flow errors: node_modules/.bin/flow --json | jq '.errors|map(.message[0].descr)|.[]' -r | sort | uniq -c

Another thought:

• In general the bulk of the Flow errors seem independent of the upgrade of anything but Flow itself. And I expect nearly all of them will be compatible with the old Flow version. So the fixes for those are all excellent candidates for merging before the upgrade.
  • Though actually upgrading Flow may have to be in tandem with upgrading RN -- because it finds errors in the old version of RN itself!

[gnprice]

I just pushed one of these commits that had a Flow fix (4851f34) as well as a few other Flow 0.92 fixes that I spotted when poking at this. Thanks!

And I'll bump the connect issue up to my near-term todo list.

[gnprice]

I have a branch at gnprice/zulip-mobile@connect that upgrades to the new libdef for connect; and I've pushed a couple series of commits now to fix type errors that that exposes. ✨ (2da96e7^..f89f317 today, and more in the previous few days.)

At this point I think it's reduced to just one issue, which is that the new libdef seems not to take into account the funky semantics of defaultProps -- I get a bunch of errors like

Cannot create Screen element because property autoFocus is missing in
MergeOPSP [1] but exists in object type [2] in type argument Props [3].

where, say, Screen is invoked without mentioning the (optional! has a default!) prop autoFocus.

So, next up is I'll dig into that. Then connect should be a go! We might even be able to upgrade the libdef before the big upgrades (of Flow version, and of RN itself) -- the reason we don't already get it on yarn update-libdefs is that it's marked in the flow-typed repo as only being for Flow versions v0.89+, but seems to WFM just fine on our v0.78.

[gnprice]

#3399 is now complete! Closing this version.