Upgrade to RN v0.64! #4991
Merged
Upgrade to RN v0.64! #4991
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
chrisbobbe
added
upstream: RN
|
These are the RN commits that touch the RN template app that are in v0.64.2 but are not in v0.63.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
Sep 8, 2021
There was a problem hiding this comment.
Thanks @chrisbobbe! There was a lot in this upgrade project; excited to see it near completion.
I've just read up through the main upgrade commit, and have only the comment below. I'll plan to look at the rest tomorrow when we're both in the office.
| @@ -7,6 +7,7 @@ const transformModulesWhitelist = [ | |||
| 'expo-application', | |||
| 'expo-web-browser', | |||
| 'react-native', | |||
| '@react-native', | |||
There was a problem hiding this comment.
This would be handled already if our transformIgnorePatterns
extended the transformIgnorePatterns in React Native's Jest preset,
because that preset was adapted in facebook/react-native@a77f2c40d,
released in RN v0.64.
It would also be handled already if our transformIgnorePatterns
extended the transformIgnorePatterns in `jest-expo`'s Jest preset
after *that* preset was adapted, in the (as-yet-unreleased) change
in expo/expo@24bce422c.
But we ignore and clobber transformIgnorePatterns in those presets,
so we add this line ourselves.
Can we easily borrow one (or both) of those? That'd be handy if so. Though it's not worth a lot of work -- it stops being worth it when it becomes more work than continuing to update it like this, which isn't a lot.
There was a problem hiding this comment.
Yeah, I'm not totally sure, but I think it's likely that we can't easily extend one or both. transformIgnorePatterns is an array, so we could potentially just concat our own custom thing onto one or both of the others. But I think once an item in that array says to ignore a path, you probably can't have a different item cause it to be un-ignored. And each of these items expresses what paths it wants compiled by saying "ignore everything except for this narrow range" and those ranges don't overlap.
There was a problem hiding this comment.
Cool, yeah, that makes sense.
In that case I'd just tweak this message so it's not pointing toward "alas, we don't extend those and instead have chosen to ignore and clobber them". Mentioning that this item appears in those is fine context to have, but this API just isn't suitable to taking a list from elsewhere and extending it.
|
OK, finished reading the rest of the branch, and all looks good! One commit-message quibble in the thread above; otherwise, please merge at will 🚀 |
Closed
…v0.64. This would be handled already if our transformIgnorePatterns extended the transformIgnorePatterns in React Native's Jest preset, because that preset was adapted in facebook/react-native@a77f2c40d, released in RN v0.64. It would also be handled already if our transformIgnorePatterns extended the transformIgnorePatterns in `jest-expo`'s Jest preset after *that* preset was adapted, in the (as-yet-unreleased) change in expo/expo@24bce422c. But we ignore and clobber transformIgnorePatterns in those presets, because the API just isn't suitable to taking a list from elsewhere and extending it [1]. So we add this line ourselves. Since we're upgrading RN before taking that `jest-expo` change, we'll see the following harmless warning for a while when we run Jest: react-native/jest-preset contained different transformIgnorePatterns than expected. It's harmless because we don't use `jest-expo`'s transformIgnorePatterns anyway; we fully specify our own instead. [1] zulip#4991 (comment) Related: zulip#4426
`jest-expo` has a bug where it imports `fbjs-scripts` library but doesn't declare it as a dependency. This hasn't caused any issues so far because React Native has been pulling in `fbjs-scripts`, and `jest-expo` uses it from there. But RN v0.64 stops pulling in `fbjs-scripts`, so things fail [1]. So, until we get a version of `jest-expo` that declares `fbjs-scripts` as a dependency, or (preferably) consumes the code from its new home at @jest/create-cache-key-function [2] (and hopefully declares *that* as a dependency), we should keep `fbjs-scripts` as our own direct dependency. Since this is meant to be a temporary state, and to keep down complexity, just pin to the exact version that RN pulls in in RN v0.63, which is 1.2.0. [1] expo/expo#13886 (comment) [2] See facebook/react-native@fd9787ecc.
In this commit: - Update `react-native`, `react`, and `flow-bin` in package.json, and bump the Flow version in the Flow config. All of this is normal. - Add a `resolutions` line for `react-test-renderer`, while we wait for `jest-expo` to officially target RN v0.64. I'd opened the relevant change in expo/expo#13549, but it landed first in expo/expo@d49e7070a. As of 2021-09-03, that change hasn't been released, but it might be soon. This line raises the following warning from Yarn: warning Resolution field "react-test-renderer@17.0.1" is incompatible with requested version "react-test-renderer@~16.11.0" This is expected. The doc says [1], "You will receive a warning if your resolution version or range is not compatible with the original version range." We've managed to get away with incompatible `resolutions` lines in the past; I guess that's a Yarn bug that we're not running into this time. - Enable `exact_by_default`, to follow facebook/react-native@050a7dd01. (We'd done a lot of work to prepare for this.) We couldn't enable it earlier because RN v0.63 didn't pass type-checking with it on. - Do two find-and-replace-all tasks, for some places where we're importing from React Native's internals and their paths have changed: 'react-native/Libraries/Animated/src/nodes/AnimatedValue' -> 'react-native/Libraries/Animated/nodes/AnimatedValue', from facebook/react-native@9c9e67791. 'react-native/Libraries/StyleSheet/StyleSheetTypes' -> 'react-native/Libraries/StyleSheet/StyleSheet' from facebook/react-native@0a6713312. And, for this one, they give the reason as "to encourage its use in product code"! 🎉 - And that's it! [1] https://classic.yarnpkg.com/en/docs/selective-version-resolutions/#toc-tips-tricks Fixes: zulip#4426
As planned; see the code comment.
We decided to postpone taking 4.0.1 or later until after the RN v0.64 upgrade. That's because someone reported an issue with 4.0.1 [1], and a supposed fix landed in facebook/react-native@53f55001a, which was a change to RN's internal code released in v0.64. Release notes here: https://developer.android.com/studio/releases/gradle-plugin#4-1-0 Quite a few major changes announced, hopefully they're all good ones. Our tests pass, and a debug build on Android built and ran fine for me. Part of the RN v0.63 -> v0.64 changes to the template app, corresponding to: facebook/react-native@cf8368f20 facebook/react-native@553fb8b28 facebook/react-native@dfa9db49e See discussion at https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Pin.20to.20explicit.20NDK.20version.3F/near/1210089. [1] facebook/react-native#29013 (comment)
Done to follow the template-app change in facebook/react-native@eb85d1dbd. Done after the RN v0.64 upgrade because Hermes on iOS is advertised as new in v0.64. We don't actually intend to use Hermes at all until RN v0.65, when it gains support for Intl when used on Android. Intl support on iOS is still under discussion, and they don't have a timeline yet: facebook/hermes#23 (comment)
These defaults are set in https://github.com/facebook/react-native/blob/v0.64.2/scripts/react_native_pods.rb: versions['Flipper'] ||= '~> 0.75.1' versions['Flipper-Folly'] ||= '~> 2.5.3' versions['Flipper-RSocket'] ||= '~> 1.3' We started pinning to some specific versions in 912cb9e, following published advice from RN in facebook/react-native#31480 for fixing a build issue with Xcode 12.5. Checking that advice again, its entry suggests that we don't have to do anything as long as we're at RN v0.64.1 or higher. Which makes sense, seeing those defaults.
Done to follow the template-app changes in facebook/react-native@5c09b3fb5 and facebook/react-native@ed237b479. Done after the RN v0.64 upgrade because the new code depends on changes to internal RN code (scripts/react_native_pods.rb). The two names refer to two different functions, and react_native_post_install is new in RN v0.64. That function calls flipper_post_install (if we've enabled Flipper by calling `use_flipper!`, that is), and it does some other stuff. That other stuff includes some changes to our project.pbxproj, apparently.
Thanks for the review! Merged. |
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Dec 13, 2021
This is just a minor version that came out a month ago, which was after our big v0.64 upgrade, zulip#4991. The changelog entry [1] has just one item: ### Fixed - For Android, general fixes to Appearance API and also fixes AppCompatDelegate.setDefaultNightMode(). For iOS, now works correctly when setting window.overrideUserInterfaceStyle ([25a2c608f7](facebook/react-native@25a2c60) by [@mrbrentkelly](https://github.com/mrbrentkelly)) We don't use those mentioned APIs, so this should be fine and not disruptive. There are no changes to the template app. Tested on my iPhone 13 Pro running iOS 15, and on the office Android device (Samsung Galaxy S9 running Android 9), and I didn't encounter any building or running failures, and nothing was obviously wrong in the app. [1] https://github.com/facebook/react-native/blob/main/CHANGELOG.md#v0643
gnprice
pushed a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Dec 16, 2021
This is just a minor version that came out a month ago, which was after our big v0.64 upgrade, zulip#4991. The changelog entry [1] has just one item: ### Fixed - For Android, general fixes to Appearance API and also fixes AppCompatDelegate.setDefaultNightMode(). For iOS, now works correctly when setting window.overrideUserInterfaceStyle ([25a2c608f7](facebook/react-native@25a2c60) by [@mrbrentkelly](https://github.com/mrbrentkelly)) We don't use those mentioned APIs, so this should be fine and not disruptive. There are no changes to the template app. Tested on my iPhone 13 Pro running iOS 15, and on the office Android device (Samsung Galaxy S9 running Android 9), and I didn't encounter any building or running failures, and nothing was obviously wrong in the app. [1] https://github.com/facebook/react-native/blob/main/CHANGELOG.md#v0643
sumj25
pushed a commit
to sumj25/zulip-mobile
that referenced
this pull request
Jan 12, 2022
This is just a minor version that came out a month ago, which was after our big v0.64 upgrade, zulip#4991. The changelog entry [1] has just one item: ### Fixed - For Android, general fixes to Appearance API and also fixes AppCompatDelegate.setDefaultNightMode(). For iOS, now works correctly when setting window.overrideUserInterfaceStyle ([25a2c608f7](facebook/react-native@25a2c60) by [@mrbrentkelly](https://github.com/mrbrentkelly)) We don't use those mentioned APIs, so this should be fine and not disruptive. There are no changes to the template app. Tested on my iPhone 13 Pro running iOS 15, and on the office Android device (Samsung Galaxy S9 running Android 9), and I didn't encounter any building or running failures, and nothing was obviously wrong in the app. [1] https://github.com/facebook/react-native/blob/main/CHANGELOG.md#v0643
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.
Following the now-merged PRs #4433, #4518, #4520, #4836, #4843, #4846, #4887, #4894, #4891, #4907, #4928, #4982, #4983, #4985, #4987, and #4989, complete the RN v0.64 upgrade! Some of those were small, some very large. And there were probably other ones, possibly including some we didn't realize were on the path to this upgrade. 🙂
Fixes: #4426