Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: Upgrade to RN v0.67! #5426

Merged
merged 4 commits into from
Jun 25, 2022
Merged

deps: Upgrade to RN v0.67! #5426

merged 4 commits into from
Jun 25, 2022

Conversation

chrisbobbe
Copy link
Contributor

@chrisbobbe chrisbobbe commented Jun 23, 2022

Fixes: #5232

@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented Jun 23, 2022

Here are the RN commits that touch the RN template app that are in v0.66.4 but are not in v0.67.4. They're ordered from first to last (oldest to newest).

I.e., they're the commits in the output of git log --oneline --reverse v0.66.4..v0.67.4 -- template, when I run it in a clone of the facebook/react-native repo.

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.

NOTE: This time, we'd like to postpone the Gradle upgrade that appears in the template-app changes. Before taking that upgrade, we need to take expo/expo@97ff91217 in our expo-* dependencies, and we haven't done that yet. We haven't done it yet because we're waiting to see if this RN upgrade (or any RN upgrade) will unblock taking expo-modules-core@0.6.0; see #5203 (comment) (expand expo-screen-orientation and see the comment on the Installing ExpoModulesCore 0.6.4 output from pod install).

RN commit When resolved Comment
facebook/react-native@13107fa3d Import template/android/ as part of the top level build (#32124) Not in this upgrade The only interesting change here is bumping Gradle to 7.0.2 (which we don't want to do in this upgrade; see note at top); looks like RN meant to do this in facebook/react-native@85249cafe. The line endings of template/android/gradlew.bat get messed with, but this is reverted later, in facebook/react-native@ab2bdee73.
facebook/react-native@ac4ddec54 OSS: add Xcode 12.5 + M1 machines CocoaPods post_install workaround fe300d5 This was backported to RN v0.66 in facebook/react-native@ea5109fc0 and we processed it for that upgrade.
facebook/react-native@329b026f3 Switch order of search libraries to fix M1 build error N/A This was backported to RN v0.66 in facebook/react-native@038cdda23 and we processed it for that upgrade.
facebook/react-native@b1120c6a6 Bump Gradle to 7.1.1 (#32138) Not in this upgrade Skipping this for this RN upgrade; see note at top.
facebook/react-native@99f3a6a56 Deploy 0.159.0 to xplat N/A
facebook/react-native@ab2bdee73 Fix encoding for gradlew.bat files (#32178) N/A This cleanly reverts a change from facebook/react-native@13107fa3d that we didn't take; ignore.
facebook/react-native@fc553ed8e Revert removal of CRLF from Windows bat files (#31398) N/A I think we're happy with how we manage our line endings.
facebook/react-native@671068f28 Re-sync with internal repository (#32212) N/A I still think we're happy with how we manage our line endings…(why do they keep changing them??)
facebook/react-native@b4ac21152 Remove mavenLocal() PR
facebook/react-native@b494ae070 Move react-native-codegen dependency to react-native root package.json fe300d5 This was backported to RN v0.66 in facebook/react-native@ab829005b and we processed it for that upgrade.
facebook/react-native@c6deb903d Deploy 0.160.2 to xplat N/A
facebook/react-native@a1c445a39 Fix: Xcode 12.5+ build of iPhone Simulator on Apple M1 (#32284) 80a8621 We handled this in 80a8621; see our mention of this RN commit there.
facebook/react-native@51bf55794 Feat/ios m1 improvements (#32296) N/A This gets reverted in facebook/react-native@eede05c91; ignore
facebook/react-native@80e24834b Make react-native depend on react-native-gradle-plugin N/A This gets reverted in facebook/react-native@1ee16fc2f; ignore.
facebook/react-native@1ee16fc2f Back out "Make react-native depend on react-native-gradle-plugin" N/A This reverts facebook/react-native@80e24834b, which we didn't take; ignore.
facebook/react-native@30c64a592 Deploy 0.161.0 to xplat N/A
facebook/react-native@254493e1f Fix - TextInput Drawable to avoid Null Pointer Exception RuntimeError #17530 (#29452) N/A We decided not to take this: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/RN.2067.3A.20add.20to.20styles.2Exml.3F/near/1395349
facebook/react-native@57aa70c06 Introduce Gemfile, ruby-version (#32303) N/A We decided not to take this: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/RN.2067.3A.20ruby.20boilerplate.20to.20pin.20cocoapods.20version.3F/near/1395360
facebook/react-native@046b02628 Move mavenCentral repo below local paths (#32326) PR
facebook/react-native@72ef5e280 Update gradle.properties example (#32314) N/A This typo was cleared away in our 1e2390d
facebook/react-native@1ad45f516 Update build.gradle (#32382) PR
facebook/react-native@2f8e52b52 Restrict mavenCentral to exclude react-native older packages PR This looks like a fixup to facebook/react-native@046b02628
facebook/react-native@9ae336743 Bump Kotlin and Gradle versions (#32319) Not in this upgrade The template app is just for the Gradle version bump, which we're skipping for this RN upgrade; see note at top.
facebook/react-native@b2e648387 deploy v0.162.0 to xplat main commit
facebook/react-native@e18cf90d7 (tag: v0.67.0-rc.0) [0.67.0-rc.0] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@8c2a667e2 iOS Ruby Updates (#32456) N/A This bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@f6895e9b5 (tag: v0.67.0-rc.1) [0.67.0-rc.1] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@eede05c91 Add back Xcode_12_5_M1_post_install_workaround N/A This reverts facebook/react-native@51bf55794, which we didn't take; ignore.
facebook/react-native@a7d3ffee5 (tag: v0.67.0-rc.2) [0.67.0-rc.2] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@209bb94b9 (tag: v0.67.0-rc.3) [0.67.0-rc.3] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@cd8fa9d3e (tag: v0.67.0-rc.4) [0.67.0-rc.4] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@73afb97e8 (tag: v0.67.0-rc.5) [0.67.0-rc.5] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@c3aa86bea (tag: v0.67.0-rc.6) [0.67.0-rc.6] Bump version numbers N/A This bumps react-native in package.json
facebook/react-native@857dbd9b2 (tag: v0.67.0) [0.67.0] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@25f3d310d (tag: v0.67.1) [0.67.1] Bump version numbers N/A This bumps react-native in package.json
facebook/react-native@da420c99a (tag: v0.67.2) [0.67.2] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@b94a36ad8 (tag: v0.67.3) [0.67.3] Bump version numbers N/A This bumps react-native in package.json, and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)
facebook/react-native@0e06185f4 (HEAD, tag: v0.67.4, origin/0.67-stable) [0.67.4] Bump version numbers main commit This bumps react-native to 0.67.4 (which we take), and bumps some versions in Ruby files that we don't want and didn't add (see note on facebook/react-native@57aa70c06)

@gnprice
Copy link
Member

gnprice commented Jun 24, 2022

I think we're happy with how we manage our line endings.

😄

…(why do they keep changing them??)

Because they don't have Git set up properly to keep them consistent.

Specifically, they don't appear to have a .gitattributes file, and in particular any eol lines. Like these in our .gitattributes:

# Maintain LF (Unix-style) newlines in text files.
* text=auto eol=lf
*.bat eol=crlf

(Hmm, that comment isn't quite right -- it's describing the line right after it, but then the next line says that *.bat files should instead maintain CRLF, aka Windows-style, newlines. I guess I should fix that.)

Looks like here's where I added those two lines, back in 2018: 58c70ec

gitattributes: Auto-convert line endings in text files.

This simplifies things for Windows users, preventing PRs that are
full of ^M (aka 0x0d, aka CR) noise.

This is the same configuration as in zulip.git, with one extra
wrinkle: the Android build (following standard templates) contains a
Windows batch file, giving us one file that's actually supposed to end
lines with CR+LF.

Maybe I should attempt to get that fixed in react-native upstream.

… Oh, I see. They actually had that, and ripped it out in facebook/react-native@fc553ed8e . ¯_(ツ)_/¯ Not worth trying to push on, then -- there are bigger things to fix.

It seems like at least part of the churn came from enabling that configuration, and then disabling it again.

@gnprice
Copy link
Member

gnprice commented Jun 24, 2022

and bumps some versions in Ruby files that we don't want and didn't add

Sure are a lot of these! That is strengthening my feeling that we don't want those files.

Sometimes the numbers even go backward, as in facebook/react-native@73afb97e8 / v0.67.0-rc.5 . Which is a nice illustration of how this setup doesn't entirely succeed at its goal of getting all the versions under control.

Done to follow the template-app change in
facebook/react-native@b4ac21152, on the path to the RN v0.67
upgrade.

The app built and ran fine with `tools/run-android` after this, in
my testing just now.
Done to follow the template-app change in
facebook/react-native@046b02628, on the path to the RN v0.67
upgrade, with the fixup in facebook/react-native@2f8e52b52.

The fixup is aimed at projects that use RN's nightly builds. We
don't, but we might as well follow the template app. And using
nightly releases is mentioned in a (possibly early/outdated?) draft
for opting into RN's "new architecture":
  https://reactnative.dev/docs/next/new-architecture-app-intro#use-a-react-native-nightly-release
and I suppose one day we'll want to be using that new architecture.
Done to follow the template-app change in
facebook/react-native@1ad45f516, on the path to the RN v0.67
upgrade.
In this commit:

- Bump `react-native` and `flow-bin` versions; point Flow config to
  new Flow version

- Suppress an error from the new Flow version

- rm -rf node_modules && rm -rf ios/Pods && yarn

The project.pbxproj changes seem to have happened with the
`pod install` that got run as part of our Yarn postinstall script.
@gnprice
Copy link
Member

gnprice commented Jun 25, 2022

Thanks @chrisbobbe for taking care of this upgrade!

This all looks good to me. Merging.

@gnprice gnprice merged commit 0d2cc3a into zulip:main Jun 25, 2022
gnprice added a commit that referenced this pull request Jun 25, 2022
The comment was accurate about the first line but not the second.

For some history in RN upstream for comparison, see:
  #5426 (comment)
@gnprice
Copy link
Member

gnprice commented Jun 25, 2022

(Hmm, that comment isn't quite right -- it's describing the line right after it, but then the next line says that *.bat files should instead maintain CRLF, aka Windows-style, newlines. I guess I should fix that.)

0a0655b29

@chrisbobbe chrisbobbe deleted the pr-rn-67 branch July 5, 2022 22:14
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 26, 2022
Done to follow the template-app change in
facebook/react-native@13107fa3d. (Which it looks like RN actually
intended to make in facebook/react-native@85249cafe but forgot;
that's not important, I think.)

We didn't take facebook/react-native@13107fa3d on the path to the RN
v0.67 upgrade because of a blocker that was resolved with the Expo
44 upgrade in expo/expo@3edc37ae4. For details, expand the
`expo-screen-orientation` block in
  zulip#5203 (comment)
and see the comment on the `Installing ExpoModulesCore 0.6.4` output
from `pod install`.

Done by following the "How to upgrade Gradle" instructions at the
top of gradle-wrapper.properties. The result matched the
template-app commit except for upstream's line endings got messed
with because they didn't have Git set up properly to keep them
consistent:
  zulip#5426 (comment) .
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Nov 22, 2022
Done to follow the template-app change in
facebook/react-native@13107fa3d. (Which it looks like RN actually
intended to make in facebook/react-native@85249cafe but forgot;
that's not important, I think.)

We didn't take facebook/react-native@13107fa3d on the path to the RN
v0.67 upgrade because of a blocker that was resolved with the Expo
44 upgrade in expo/expo@3edc37ae4. For details, expand the
`expo-screen-orientation` block in
  zulip#5203 (comment)
and see the comment on the `Installing ExpoModulesCore 0.6.4` output
from `pod install`.

Done by following the "How to upgrade Gradle" instructions at the
top of gradle-wrapper.properties. The result matched the
template-app commit except for upstream's line endings got messed
with because they didn't have Git set up properly to keep them
consistent:
  zulip#5426 (comment) .
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Nov 23, 2022
Done to follow the template-app change in
facebook/react-native@13107fa3d. (Which it looks like RN actually
intended to make in facebook/react-native@85249cafe but forgot;
that's not important, I think.)

We didn't take facebook/react-native@13107fa3d on the path to the RN
v0.67 upgrade because of a blocker that was resolved with the Expo
44 upgrade in expo/expo@3edc37ae4. For details, expand the
`expo-screen-orientation` block in
  zulip#5203 (comment)
and see the comment on the `Installing ExpoModulesCore 0.6.4` output
from `pod install`.

Done by following the "How to upgrade Gradle" instructions at the
top of gradle-wrapper.properties. The result matched the
template-app commit except for upstream's line endings got messed
with because they didn't have Git set up properly to keep them
consistent:
  zulip#5426 (comment) .
@chrisbobbe chrisbobbe mentioned this pull request Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to RN v0.67
2 participants