Flow-related work on the path to RN v0.64 upgrade, part 3. #4836
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
gnprice
reviewed
Jul 2, 2021
There was a problem hiding this comment.
Thanks @chrisbobbe ! This all looks good -- glad to have this upgrade to our Flow feature-set completed.
I'm about to merge; I'll just edit a commit message to add the answer to the question I had below.
Comment on lines
-1
to
-2
| // flow-typed signature: 01a139bd4c71685f16dbe4315660c576 | ||
| // flow-typed version: <<STUB>>/react-native-document-picker_v^3.2.4/flow_v0.92.0 |
There was a problem hiding this comment.
libdefs: Remove two stub libdefs that duplicate non-stub libdefs.
Yeah. So the reason this duplicate is here (and I haven't checked the other one but I'd guess it's the same story) is:
commit 6219e5a
Author: Vishwesh Jainkuniya jainkuniya@gmail.com
Date: Sun Aug 11 14:38:51 2019 +0530
flow: Add dummy libdef for react-native-document-picker.
In e17b1a1ee we replaced the stub libdef for this library with
our own real one. But `yarn update-libdefs` (and the underlying
`flow-typed update`) doesn't notice our real one, and keeps wanting
to generate a new stub.
It's annoying to have to keep deleting the stub whenever we use
`yarn update-libdefs`, so just let it have its way. Fortunately
the real libdef takes precedence, empirically: if we artificially
introduce a type error in ComposeMenu.js where this library is
used, Flow indeed catches it.
What was causing the duplicate to get in your way?
… Ah I see, this later commit answers that question:
libdefs: Remove stub libdefs.
And use `$FlowFixMe[untyped-import]` where these stub libdefs were
suppressing complaints from Flow in a way that was harder to notice.
Sounds like a good plan to me!
We're about to remove all the stub libdefs.
From experimentation, I don't see any complaints from Flow when I remove this. Remove it, just for that reason.
And use `$FlowFixMe[untyped-import]` where these stub libdefs were suppressing complaints from Flow in a way that was harder to notice.
Like we do for eslint, in our .eslintignore; see discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/RN.20v0.2E64.20upgrade.3A.20.23M3452/near/1222822.
…iends. In particular, more like `WebViewError, `WebViewMessage`, etc., just above it. An instance of zulip#3452.
…act. An instance of zulip#3452.
…xact. An instance of zulip#3452.
An instance of zulip#3452.
I'm not so sure we need a libdef for `prettier`, actually. Anyway, an instance of zulip#3452.
An instance of zulip#3452.
…exact. An instance of zulip#3452.
|
Thanks for the review! |
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 and 2 were #4518 and #4520.
As I explained at #3452 (comment),
The first few commits fix a few small things that have crept into our own code since #4520, and then the rest deal with the libdefs.
Fixes: #3452