More type fixes for RN v0.59 / Flow v0.92 #3453
Conversation
| @@ -24,8 +24,8 @@ export type Dimensions = {| | |||
| |}; | |||
|
|
|||
| export type InputSelectionType = {| | |||
| start: number, | |||
| end: number, | |||
| +start: number, | |||
There was a problem hiding this comment.
Duh, this is the most subtle thing a diff can show - start vs + +start.
There was a problem hiding this comment.
I'm not sure if that's a request to do something different, or just a lament about Flow's syntax. 🙂
I do think this is a good reason to keep this change separate from large other changes, or other changes to this chunk of code! As is, though, as the reader you know something changed in this block because there's a diff shown here... so it should get your attention, and then hopefully it isn't too hard to spot what.
One other tool that helps: diff-highlight. See my .gitconfig for how to enable it:
https://gist.github.com/gnprice/5f71b63464a76bd7e0480f207b1bbff3
The tool itself is distributed with Git; the Debian package puts it in /usr/share/doc/git/contrib/diff-highlight, and in general locate diff-highlight should find it on your system. In cases like this, it causes the one small piece of each line that actually changed to get highlighted in git diff, git log -p, etc.
The next commit is actually a yet stronger example of where diff-highlight is super helpful -- every one of the diff hunks has the word Type in red, which helps me see at a glance that the exact same boring thing is happening in each one of them.
Some change in the upgrade to RN v0.59 (or perhaps to Flow v0.92 which accompanies it) causes Flow to start giving errors whenever we pass a value of the type `DangerouslyImpreciseStyleProp` -- which is what our `Style` is actually an alias for -- to a component like `View` or `TextInput`. So, just fix most of them to the specific style type they ought to be. Turns out this isn't hard; mostly just follow the error messages where they lead.
With Flow v0.92 this causes a kind of complicated-looking error message... but when unpacked it turns out to just mean the caller of this callback wants assurances we'll treat these properties as read-only. So, give those assurances.
It's a type, yes -- but we don't signal that by adding the word "type". That's reserved for when the *values* themselves are "types" in some sense, like with React's `ComponentType`.
Caught by Flow v0.92. Thanks, Flow!
Flow v0.78 actually quite happily infers the correct answer here. But Flow v0.92 (presumably since v0.85) insists on an explicit annotation... so fine, sure, it's easy to add.
|
Merged. Thanks @borisyankov for taking a look! |
Fixes zulip#3399! The bulk of the work of this upgrade went into a large number of commits previous to this one. The majority of the last few dozen commits, after 1c86488^, were part of the upgrade; some discussion in zulip#3561. An earlier wave of effort focused on getting things working with the upgrade of Flow to v0.92; see zulip#3450 and especially f8c9810, and then zulip#3453 and zulip#3442. Some other early discussion is at zulip#3422. This commit itself was done mainly by running: $ tools/upgrade rn v0.59.10 Then, that tool needs a bit of an update for changes upstream. Because I'm feeling pressed to get this upgrade out the door (and to deal with the various separate trouble on iOS), I just took care of the other relevant packages by hand: updated `@babel/core` and `metro-react-native-babel-preset` in package.json according to the diff seen in `rn-diff-purge`, then reran `yarn`.
This is another piece of #3399; sort of a continuation of #3450.
Basically I took a branch based on #3422, plus my changes in #3450, and looked at the type errors that remained. The errors that jumped out at me as either (a) potentially involving something subtle in the type system, or (b) being about code which has some subtlety and I remember writing, I investigated and fixed.