Flow-related work on the path to RN v0.64 upgrade, part 4. #4891
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flow v0.137, which we'll start using with the upcoming RN v0.64 upgrade (zulip#4426), seems to prefer it this way. I don't see it as worse, for readability, so might as well.
Not totally sure why it wants this now; maybe because I changed the text-wrap settings in VSCode a few weeks ago? Anyway, seems quite harmless and fine.
This is another round in the spirit of zulip#4433, to prepare for Flow v0.132 and later; there's this in the changelog entry [1]: > Added warnings against suppressions without error codes As we noted there, we still haven't enforced that suppressions have error codes, but at least we can clean up these ones that have crept in. [1] https://github.com/facebook/flow/blob/master/Changelog.md#01320
Without this, when I try upgrading to Flow v0.137, for the upcoming
RN v0.64 upgrade, I get this error:
```
Error --------------------------------------------------------------------------------------- src/boot/reducers.js:73:33
Cannot call `maybeLogSlowReducer` with `key` bound to `key` because type variable `Key` [1] cannot escape from the scope
in which it was defined [2] (try adding a type annotation to `key` [3]). [escaped-generic]
src/boot/reducers.js:73:33
73| maybeLogSlowReducer(action, key, startMs, endMs);
^^^
References:
src/boot/reducers.js:43:8
43| key: Key,
^^^ [1]
src/boot/reducers.js:42:23
42| function applyReducer<Key: $Keys<GlobalState>, State>(
^^^ [2]
src/boot/reducers.js:36:38
36| function maybeLogSlowReducer(action, key, startMs, endMs) {
^^^ [3]
```
I haven't reasoned all of the way through it yet, but the annotation
seems harmless and correct.
We already have a type wrapper for this function, from 8f56964, so we could just as well have suppressed type-checking for this whole function here. But this is easy. The Flow error that caused me to notice this appeared when I tried to upgrade to Flow v0.137, for the RN v0.64 upgrade: ``` Error -------------------------------------------------------- flow-typed/react-native-safe-area-context_vx.x.x.js:60:23 Cannot use `React.ComponentType` as a type because it is an `any`-typed value. Type `React.ComponentType` properly, so it is no longer `any`-typed, to use it as an annotation. [value-as-type] 60| WrappedComponent: React.ComponentType<T>, ^^^^^^^^^^^^^^^^^^^ ```
|
Looks good, thanks! Merging. |
gnprice
force-pushed
the
pr-more-flow-rn-64
branch
from
9f91add
to
a55b0e9
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Parts 1, 2, and 3 were #4518, #4520, and #4836.
This doesn't upgrade Flow at all or add any new config; it's just cleaning up some stuff that I noticed (one commit is another round like #4433, for example). These kinds of small messes could easily creep back in before we add config to prevent them for good, but might as well sweep things up as we see them.
After this, the work I see before the RN v0.64 upgrade (#4426) is:
exact_by_defaultFor
exact_by_default, I think I have a handle on all the little issues in our libraries' built-in Flow definitions, so that won't be hard unless new problems come up (e.g., with libraries or library versions we aren't using right now). One of those issues might best be solved with a small one-line PR to @react-native-community/cameraroll; hopefully they'd take it.