deps: Upgrade to RN v0.66! #5372
Merged
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
|
Here are the RN commits that touch the RN template app that are in v0.65.2 but are not in v0.66.4. They're ordered from first to last (oldest to newest). I.e., they're the commits in the output of Along with whether, how, and when we propagate the changes to our app. We shouldn't forget to update this, if necessary, to follow any changes made during code review and merge.
|
gnprice
reviewed
May 10, 2022
There was a problem hiding this comment.
Thanks @chrisbobbe for taking care of this upgrade! This all looks good; just small commit-message comments below.
| @@ -27,7 +27,7 @@ buildscript { | |||
| } | |||
| dependencies { | |||
| classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" | |||
| classpath 'com.android.tools.build:gradle:4.2.1' | |||
| classpath 'com.android.tools.build:gradle:4.2.2' | |||
There was a problem hiding this comment.
Here's the release-notes page for 4.2.0; I don't see a note about
any of the patch releases there though.
It looks like you meant to include a URL here.
Comment on lines
+5
to
+6
| export type Storage = interface { | ||
| multiSet(keyValuePairs: Array<Array<string>>): Promise<mixed>, |
There was a problem hiding this comment.
nit: In the commit message:
(a) It looks like the error message it comes from the spiffy terminal version of Flow's output which relies on terminal color codes, but naturally it doesn't have the color. Can you include the version that doesn't rely on color?
The universal way (that works on basically all CLI programs) is by saying flow | cat instead of just flow, so that it sees its output isn't a terminal. The explicit way is flow --color=never.
(b) The quoted error is long because it has (most of) the whole text of getItem, with all its comments. Can you trim it down for the commit message?
|
Thanks for the review! Revision pushed. |
|
Before merging, let's let a release with RN v0.65 go out to all users. |
(That way if there are issues that seem related to the upgrade, we have a head start on narrowing them down and don't have to wonder whether they were introduced by RN v0.65 or v0.66.) |
|
Meanwhile, this branch all looks good! So once we have a release out to prod with RN v0.65, this will be ready for merge. |
Otherwise, when we start using Flow v0.158, we'd see errors like
```
Error ------------------------------------ src/boot/store.js:168:12
Cannot assign object literal to `reduxPersistConfig` because
function type [1] cannot be unbound from the context [1] where it
was defined in the `this` parameter of property `storage.getItem`.
[method-unbinding]
src/boot/store.js:168:12
168| storage: CompressedAsyncStorage,
^^^^^^^^^^^^^^^^^^^^^^
References:
src/storage/CompressedAsyncStorage.js:21:3
v----------------------------------------------------------
21| static async getItem(key: string): Promise<string | null> {
:
64| }
^ [1]
```
Greg explains, at
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flow.20158.20errors/near/1375554
, comparing the definition before and after this change:
> Those two types are very similar. But the difference is about
> `this`.
>
> The interface just says, you can call this method, and you should
> pass these types and you'll get this type.
>
> The object type says, you can get this *property*, and then go on
> to call that property as a function, passing these types and
> getting this type back.
>
> And the point that error seems to be making is that the latter
> thing isn't actually true here, while the first thing might be.
With Flow v0.158, we'd get errors like: Cannot get AsyncStorage.clear because typeof-annotation AsyncStorage.clear [1] cannot be unbound from the context [2] where it was defined. [method-unbinding] Greg points out, at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Flow.20158.20errors/near/1375541 : > Those can be fixed by just replacing typeof `AsyncStorage.clear` > with what that type actually is. There aren't very many of those > and their types aren't actually complicated, so that duplication > is fine; I just wrote it that way because it seemed neater to > avoid the duplication if I could. So, do that.
….66.2. Done to follow the template-app changes in facebook/react-native@ca440b910 (for ^0.66.1) facebook/react-native@cfdc4fed0 (for ^0.66.2) on the path to the RN v0.66 upgrade. (Note, from the yarn.lock, that the actual selected version doesn't change. It was 0.66.2 before, and it still is now.)
Done to follow the template-app change in facebook/react-native@41f45a77a, on the path to the RN v0.66 upgrade.
Done to follow the template-app change in facebook/react-native@ae494e7ce, on the path to the RN v0.66 upgrade. Here's the release-notes page for 4.2.0. I don't see a note about any of the patch releases there though: https://developer.android.com/studio/releases/gradle-plugin#4-2-0
Note: If you get an error like CocoaPods could not find compatible versions for pod "RCT-Folly" on `yarn` or `pod install` when crossing this commit (checking out a branch that has it from one that doesn't, or vice versa), run `pod update RCT-Folly` from `ios/` and try the command again. Background: facebook/react-native#32659 (comment) In this commit: - Bump `react-native` and `flow-bin` versions - Suppress a few errors from the new Flow version (see comments) - Remove react-native-codegen, following the template-app change in facebook/react-native@ab829005b - Add a hack line to the Podfile, following the template-app change in facebook/react-native@ea5109fc0
|
v27.186 is now out to all users, and it has RN v0.65. I've rebased this PR, and on a quick manual smoketest everything still seems good. Merging! |
|
Great! Thanks for the review! |
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.
Fixes: #5231