deps: Upgrade to Expo SDK 45 #5507
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
gnprice
reviewed
Oct 12, 2022
Comment on lines
-61
to
+68
| // Allow jcenter for only me.relex:photodraweeview:1.0.0: | ||
| // Allow jcenter for only me.relex:photodraweeview:1.1.3: |
There was a problem hiding this comment.
Regarding the `jcenter` exception for `photodraweeview` (see
android/build.gradle in this commit), I tried removing the exception
entirely, hoping that 1.1.3 might be available in the non-jcenter
repos consulted by the build, even though 1.0.0 was not. But no, I
again got an error that `photodraweeview` could not be found, so it
looks like we still need to make an exception.
Huh interesting. That seems at variance with the latest commit in react-native-photo-view: alwx/react-native-photo-view#208
Perhaps the build worked for that person because he actually still had the jcenter dependency in the app's own build.gradle.
There was a problem hiding this comment.
Yeah, odd.
Perhaps the build worked for that person because he actually still had the
jcenterdependency in the app's own build.gradle.
That sounds plausible. I read https://mvnrepository.com/artifact/me.relex/photodraweeview as saying that photodraweeview is published to the "Central" repository (as in mavenCentral) only at version 2.1.0:
And version 1.1.3 (the version apparently used by react-native-photo-view) is published in a "Spring Plugins" repo:
And (strangely) the only version published in JCenter is 2.0.0:
There was a problem hiding this comment.
And that (strangely) the only version published in JCenter is 2.0.0
Strange because my investigation that you quoted suggested we were getting 1.1.3 from JCenter…
And I think maybe that's still true in some way? https://jcenter.bintray.com/me/relex/photodraweeview/1.1.3/photodraweeview-1.1.3.aar serves a file. 🤷 (I tried that URL because I followed the "2.0.0" link in that last screenshot, then the "aar (21 KB)" link from there, and tweaked the version number in the URL.)
There was a problem hiding this comment.
I read https://mvnrepository.com/artifact/me.relex/photodraweeview as saying that
photodraweeviewis published to the "Central" repository (as inmavenCentral) only at version 2.1.0:
Neat, seems like a useful resource!
I guess one has to hope that its incompleteness as to what's in JCenter is related to limitations in the latter, perhaps to its partial shutdown, and doesn't affect other repos we'll want to use.
| "expo-screen-orientation": "~4.1.1", | ||
| "expo-sqlite": "^10.0.0", | ||
| "expo-web-browser": "10.1.1", | ||
| "expo": "^45.0.0", |
There was a problem hiding this comment.
When we do the RN v0.68, we should study some special code that Expo
wants us to add as part of that upgrade, beyond what the RN template
app says. But that's a job for later, and I didn't see anything to
do right now in this item.
What's the special code that Expo wants us to add as part of that upgrade? I don't think I totally follow.
If it is something we'll want to do as part of the RN upgrade, then we should mention it on the upgrade issue in our tracker.
(also nit in commit message: AppDelegate.mm, not App.Delegate.mm)
There was a problem hiding this comment.
Huh, this is strange. The quote in my commit message—
> Update your `App.Delegate.mm` (you should have moved from .m to
> .mm in the previous step) according to this diff
—doesn't actually match what's in the Expo blog post as accessed today. From there:
Update your
AppDelegate.mm(you should have moved from.mto.mmin the previous step) update your Expo module configuration as done in this diff.
I haven't yet managed to track down the thing I copy-pasted, and in fact the blog's current text matches what it said in June. 🤷
Since my quote failed to include the link on "this diff", I can't confirm by commit ID that I was looking at the same diff as the one linked in the actual current blog post, so I'd better take another look at that.
There was a problem hiding this comment.
Probably the right thing to do is go through these:
$ git log --oneline --reverse upstream/sdk-44..upstream/sdk-45 -- templates/expo-template-bare-minimum
709dc40c7 Added android main file resolution (#14964)
38181698b [templates] Update for latest SDK 44 and remove reanimated from bare template
b94d8a00e [templates] Remove reanimated babel plugin from bare template
b955ef624 [template] Remove RNGH import from index.js
69b878efb [autolinking] Introduce patcher for double-quoted import issue (#15655)
e842fb6c1 [autolinking] Fix errors from `expo_patch_react_imports!` (#15699)
8619d89df [fix] Invoke super methods in the AppDelegate.m to support ExpoAppDelegateSubscriber (#15684)
6126967d6 Reorder AppDelegate methods to prevent breaking behavior. (#15881)
6357d7dae Added platform gitignores (#15954)
60a411ee5 [template] Automatically apply android.packagingOptions from gradle.properties (#15863)
402679cac Upgrade to react-native 0.66 (#15914)
f5787adbc Upgrade to react-native 0.67 (#16038)
bae4b0eb1 [templates] update locks after bumping to RN 67 (#16397)
607fef4ab [android] remove jcenter (#16846)
abd66f4e8 Bump minimist in /templates/expo-template-bare-minimum (#16870)
b41c14cc1 [android][ios] Upgrade to react-native 0.68 (#16532)
7f2f41705 [all packages][Android] Migrate to `Android SDK 31` and `Java 11` (#16941)
ad83b875c [templates] remove lockfiles (#16975)
a3527a58b [android][ios] Upgrade react-native to 0.68.1 (#17063)
109ab9fcd Publish project templates
e95a20416 [templates] workaround expo-notifications broken when there's firebase sdk (#17201)
6b2c02fca [templates] add missing __apply_Xcode_12_5_M1_post_install_workaround (#17266)
fdddce11a [templates] Update bare SDK 45 template
6a62fea19 [build-properties] Introduce build-properties config-plugin (#17106)
4cb982247 [build-properties] Fix android build failure (#17321)
e0877701b [templates] Bump versions [skip ci]
e32e0d34e [templates] Update for release
adf446abf [android][ios] upgrade to react-native 0.68.2 (#17438)
cc0e84eb2 [templates] bump versions
6b95bbf60 [templates] Fix android.kotlinVersion support for expo-build-properties (#17625)
75e2cf7e3 [templates] Update outdated fresco versions on android (#17636)
79350c77f [templates] Support `pod install --project-directory=ios` (#17637)
875976b53 [build-properties][templates] add minSdkVersion support (#17647)
7f3099565 [templates] bump versions
There was a problem hiding this comment.
Since my quote failed to include the link on "this diff", I can't confirm by commit ID that I was looking at the same diff as the one linked in the actual current blog post, so I'd better take another look at that.
It looks like it's the same link in the blog post today as it is in that quote from it from June. So I think we can pretty safely assume that when you read it in September, it was the same link then too.
It is conceivable that they changed it, then changed it back. But that seems awfully low probability compared to many other ways it's possible for us to get things wrong.
There was a problem hiding this comment.
What's the special code that Expo wants us to add as part of that upgrade? I don't think I totally follow.
If it is something we'll want to do as part of the RN upgrade, then we should mention it on the upgrade issue in our tracker.
Done; see the two adjacent comments #5344 (comment) and #5344 (comment)
|
OK; here's my audit of the template-app commits. The list of commits is current as of yesterday, 2022-11-21. In fact, from this audit, I didn't discover any changes to make for now! Details below. When we do the RN 68 upgrade, we'll want to revisit the commits related to Expo's take of that upgrade, so I've listed those commits in our RN 68 upgrade issue, in the adjacent comments #5344 (comment) and #5344 (comment).
|
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Nov 22, 2022
…AGP 7 Before this change, we were pointing Yarn to a commit from 2018 on this repo's `master` branch, since a new release with the commit hadn't been made to NPM in a reasonable time. (It still hasn't.) Now, just bump to this project's current `master`, which is three commits ahead of that commit: https://github.com/alwx/react-native-photo-view/commits/master In particular, we want alwx/react-native-photo-view@ef532a348, which has the project's build.gradle stop using the `compile` dependency configuration in favor of `implementation`. The former has long been deprecated, and is an error with AGP 7, which we'd like to use soon: https://developer.android.com/studio/releases/gradle-plugin#dependency-configurations-removed We'd still like to stop using this library since it's almost totally unmaintained; that's zulip#4217. Anyway, I tested on my iPhone 13 Pro running iOS 15.6, and on the office Android device (Samsung Galaxy S9 running Android 9), and I didn't notice any change in behavior of the lightbox. Regarding the `jcenter` exception for `photodraweeview` (see android/build.gradle in this commit), I tried removing the exception entirely, hoping that 1.1.3 might be available in the non-jcenter repos consulted by the build, even though 1.0.0 was not. But no, I again got an error that `photodraweeview` could not be found, so it looks like we still need to make an exception. See discussion of some further findings: zulip#5507 (comment)
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Nov 22, 2022
Following the "Bare workflow" instructions for upgrading: https://blog.expo.dev/expo-sdk-45-f4e332954a68#5954 As with the last one, 44 in 3edc37a, the `expo upgrade` command went through a phase it described as "Updating packages to compatible versions (where known)," and that changed several of our dependency ranges, even of non-Expo things. (For speculation on what that phase is about, see 3edc37a.) We rejected version-range changes to these libraries that would have resulted in downgrades to the libraries' resolved versions: - react-native-safe-area-context from 4.3.1 to 4.2.4 - react-native-screens from 3.13.1 to 3.11.1 - react-native-webview from 11.22.2 to 11.18.1 We rejected upgrades to @react-native-community/netinfo and react-native-reanimated for the reasons we gave in the Expo 44 upgrade; see those at zulip#5441 (comment) . And we rejected bumping react-native from 0.67.4 to 0.68.2, with optimism that we can get this Expo upgrade done before that RN upgrade, which is zulip#5344. We took the rest. For the step > Update your `App.Delegate.mm` (you should have moved from .m to > .mm in the previous step) according to this diff , we found that we'd already taken some of those changes when Expo backported them to the Expo 44 template app. Others, like the .m to .mm rename, we'd like to postpone until we do the RN 68 upgrade. A full audit of Expo's template-app commits from 44 to 45 found no changes we'd be interested in applying, except some related to RN 68 that we'd like to do with that upgrade; I've mentioned those on our RN 68 upgrade issue. See: zulip#5507 (comment) I tested basic functionality on my iPhone 13 Pro running iOS 15.6, and on the office Android device, a Samsung Galaxy S9 running Android 9. I didn't see any problems with building or running.
chrisbobbe
force-pushed
the
pr-expo-45
branch
from
0e4f1e4
to
2a26b83
Compare
|
Thanks for the review! Revision pushed. This commit— —no longer appears in this revision, because that upgrade was taken in 36a03ec. |
|
Thanks! I read through your handy table of upstream template commits, and everything looks good. Merging. |
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) .
Done to follow the template-app change in facebook/react-native@9ae336743. That commit was released with RN v0.67, but we didn't take it on the path to that upgrade; see previous commit for details. Done by following the "How to upgrade Gradle" instructions at the top of gradle-wrapper.properties. This made a bunch of changes that didn't make it into the RN template-app commit. It looks like the template app caught up with those when it bumped to 7.3, in facebook/react-native@c180627ac.
Done to follow the template-app change in facebook/react-native@c180627ac, on the path to the RN v0.68 upgrade. Done by following the "How to upgrade Gradle" instructions at the top of gradle-wrapper.properties. The template-app commit makes a bunch of changes that look like the ones Gradle wanted to make in the 7.2 bump, so it's probably just belatedly catching up with those. See previous commit.
…AGP 7 Before this change, we were pointing Yarn to a commit from 2018 on this repo's `master` branch, since a new release with the commit hadn't been made to NPM in a reasonable time. (It still hasn't.) Now, just bump to this project's current `master`, which is three commits ahead of that commit: https://github.com/alwx/react-native-photo-view/commits/master In particular, we want alwx/react-native-photo-view@ef532a348, which has the project's build.gradle stop using the `compile` dependency configuration in favor of `implementation`. The former has long been deprecated, and is an error with AGP 7, which we'd like to use soon: https://developer.android.com/studio/releases/gradle-plugin#dependency-configurations-removed We'd still like to stop using this library since it's almost totally unmaintained; that's zulip#4217. Anyway, I tested on my iPhone 13 Pro running iOS 15.6, and on the office Android device (Samsung Galaxy S9 running Android 9), and I didn't notice any change in behavior of the lightbox. Regarding the `jcenter` exception for `photodraweeview` (see android/build.gradle in this commit), I tried removing the exception entirely, hoping that 1.1.3 might be available in the non-jcenter repos consulted by the build, even though 1.0.0 was not. But no, I again got an error that `photodraweeview` could not be found, so it looks like we still need to make an exception. See discussion of some further findings: zulip#5507 (comment)
Done to follow the template-app change in facebook/react-native@272cfe5d1, on the path to the RN v0.68 upgrade. Here's the release-notes page for 7.0.0, with a note about 7.0.1: https://developer.android.com/studio/releases/gradle-plugin#7-0-0 Fixes: zulip#5377
We take this major version bump just before trying the Expo 45 upgrade, because the upgrade guide says "Update to the latest version of Expo CLI": https://blog.expo.dev/expo-sdk-45-f4e332954a68
Following the "Bare workflow" instructions for upgrading: https://blog.expo.dev/expo-sdk-45-f4e332954a68#5954 As with the last one, 44 in 3edc37a, the `expo upgrade` command went through a phase it described as "Updating packages to compatible versions (where known)," and that changed several of our dependency ranges, even of non-Expo things. (For speculation on what that phase is about, see 3edc37a.) We rejected version-range changes to these libraries that would have resulted in downgrades to the libraries' resolved versions: - react-native-safe-area-context from 4.3.1 to 4.2.4 - react-native-screens from 3.13.1 to 3.11.1 - react-native-webview from 11.22.2 to 11.18.1 We rejected upgrades to @react-native-community/netinfo and react-native-reanimated for the reasons we gave in the Expo 44 upgrade; see those at zulip#5441 (comment) . And we rejected bumping react-native from 0.67.4 to 0.68.2, with optimism that we can get this Expo upgrade done before that RN upgrade, which is zulip#5344. We took the rest. For the step > Update your `App.Delegate.mm` (you should have moved from .m to > .mm in the previous step) according to this diff , we found that we'd already taken some of those changes when Expo backported them to the Expo 44 template app. Others, like the .m to .mm rename, we'd like to postpone until we do the RN 68 upgrade. A full audit of Expo's template-app commits from 44 to 45 found no changes we'd be interested in applying, except some related to RN 68 that we'd like to do with that upgrade; I've mentioned those on our RN 68 upgrade issue. See: zulip#5507 (comment) I tested basic functionality on my iPhone 13 Pro running iOS 15.6, and on the office Android device, a Samsung Galaxy S9 running Android 9. I didn't see any problems with building or running.
chrisbobbe
added a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Nov 23, 2022
This is the Flow version used by React Native 68. It's a surprisingly easy upgrade this time, following the recent Expo 45 upgrade (zulip#5507), which caused `fbemitter` to be bumped such that it no longer used Flow's legacy "weak" mode, removed in Flow v0.166.
gnprice
pushed a commit
to chrisbobbe/zulip-mobile
that referenced
this pull request
Dec 1, 2022
This is the Flow version used by React Native 68. It's a surprisingly easy upgrade this time, following the recent Expo 45 upgrade (zulip#5507), which caused `fbemitter` to be bumped such that it no longer used Flow's legacy "weak" mode, removed in Flow v0.166.
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.
Following the "bare workflow" instructions for upgrading: https://blog.expo.dev/expo-sdk-44-4c4b8306584a#8ff0
I've kicked the tires with this revision on iOS and Android, with and without "Debug JS Remotely" activated, and things seem fine.
Along the way, we upgrade to Gradle 7 and AGP 7, fixing #5377.
Fixes: #5377