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

android build: Update to AndroidX. #3852

Merged
merged 1 commit into from
May 8, 2020
Merged

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jan 31, 2020

AndroidX, previously called the Android Support Library, is a collection of libraries developed by Android upstream which are bundled into an app at build time, rather than taken from the system on the device. Some are quite central to a typical Android app.

Upstream has renamed the whole thing to AndroidX, and this goes with a renaming of tons of classes. (Mostly to get them out of android.*, which was always very confusing because that's the same namespace used by the actual system packages.) Docs: https://developer.android.com/jetpack/androidx/migrate

This could have been sticky, because lots of apps and lots of third-party libraries refer to these core libraries, and in a distributed ecosystem it's impossible for everything to update at once. And because these are often so central, a given build of an app must use the same version of them throughout -- including from any third-party libraries it uses. So for example if some third-party library has updated to use AndroidX, then an app cannot switch to that version of that library until it too has updated to use AndroidX.

Fortunately, the planners of this migration recognized this could be a problem, and found a way to make the constraints run only in that one direction. When an app updates to use AndroidX, it can do so even while it's still using third-party libraries that are still built to use the Android Support Library. The key is a feature called "Jetifier": we set the flag android.enableJetifier=true, and then (quoting the doc linked above):

The Android plugin automatically migrates existing third-party libraries to use AndroidX by rewriting their binaries.

So the migration path is, apps go first; then libraries update at their leisure.


For us, the most important third-party library (third-party from an Android perspective) is React Native. RN took the AndroidX update in v0.60, so we have to do the same before we can complete that RN upgrade, i.e. #3548.

The Android upstream way to update a given project (as described by the doc linked above) has two prongs:

  • (a) The project's own source code can be updated automatically (barring fancy use of classloaders, etc.) by Android Studio, with the feature "Refactor > Migrate to AndroidX...".

  • (b) Third-party libraries get updated when they're linked in, by rewriting them as part of this project's build.

Both of those appear in this commit. But in an RN context, there's one other important set of code: third-party RN libraries get pulled in as part of our Gradle build, with build.gradle lines using project, like so:

implementation project(':react-native-vector-icons')

This means they're not covered by (b). This contrasts with RN itself, which is pulled in as a binary artifact:

implementation "com.facebook.react:react-native:+"

So for those third-party RN libraries, we need to edit the source code, like (a), before it gets built. Happily, someone has written a tool to do just that, and we use it:

  • (a') Third-party RN libraries get rewritten in place in node_modules/, after yarn installs them and before the Gradle build. This uses a different "jetifier" tool: https://github.com/mikehardy/jetifier

Specifically, the latter "jetifier" will crawl node_modules; find all .java, .kt, and .xml files; and apply a list of string substitutions (about 1900 of them) to rewrite old names to new ones. That sure is kind of heuristic... but it's good enough that it will in fact find any normal import of the affected classes, and on the other hand the patterns being substituted are all long boring fully-qualified Java class names, unlikely to be found in spurious places. For us, I also inspected (below) the exact edits the tool makes.


In this commit, we take the upgrade.

  • (a) I used "Refactor > Migrate to AndroidX..." in Android Studio. This updated a few spots in our own source code.

  • (b) The same auto-refactor added the gradle.properties settings that handle libraries we incorporate as binary artifacts, including RN itself.

    It also updated dependencies in our app/build.gradle. This made the supportLibVersion variable unused, and I deleted it.

  • (a') We add jetifier as a dev dependency, and invoke it as a postinstall hook. This means any run of yarn install (or bare yarn) will end by running it.

I inspected manually (by making copies of node_modules, and comparing with git diff --no-index) the effect of jetifier. All the edits look appropriate. Excluding a few that have no effect, the edits appear in the following packages:

  • react-native
  • react-native-image-picker
  • react-native-webview
  • react-native-photo-view
  • @react-native-community/netinfo

So we can remove jetifier once we've upgraded all of those to versions that use AndroidX in the first place.

I tested the app in both debug and release builds, and exercised each of the dependencies mentioned above: picked and uploaded an image; opened the lightbox; entered and left airplane mode to see the "No Internet connection" banner. Everything I tested works.

@rk-for-zulip
Copy link
Contributor

So we can remove jetifier once we've upgraded all of those to versions that use AndroidX in the first place.

Well... maybe not!

Per the Jetifier docs, Jetifier is automatically installed alongside and run by React Native 0.60, so we should get this for free as part of the React Native 0.60 upgrade. As such, I'm going to hold off on applying this commit until that's been confirmed.

Even if the diff turns out to be unnecessary, though, the identification and testing of the affected modules was definitely needed, and the analysis in the commit message is also helpful. Thanks!

@gnprice
Copy link
Member Author

gnprice commented Feb 3, 2020

Per the Jetifier docs, Jetifier is automatically installed alongside and run by React Native 0.60

True. I guess that means we can't easily completely remove jetifier even then. That's a little unfortunate, because it's a hack, but ah well. On the bright side, once we've made those upgrades, it should become the case that it's a no-op -- that it doesn't actually end up editing any files at all.

That description in the docs isn't very precise. What it turns out to mean, as I learned when studying this while preparing this change, is that react-native run-android, when running on RN v0.60, will run jetify first.

Even if the diff turns out to be unnecessary,

The changes in android/ are required in order to make the upgrade to AndroidX, and therefore required in order to upgrade to RN v0.60.

The bits that will get partly substituted for by the automatic call to jetify found in RN v0.60 are the two lines in package.json. That doesn't affect whether we should make this change -- we should in any case make this change, to unblock the RN upgrade, and then after the upgrade we can take those lines out if we like.

I think in fact we'll want to keep those lines there until we've upgraded all of the libraries mentioned above. That's because RN's automatic call to jetify doesn't happen unless you run react-native run-android -- so in particular it doesn't happen if you just run tests, or just make a build (e.g. a release build), without having used react-native run-android since the last time you ran yarn and it happened to update a relevant dependency. (A better place for this step might have been in the Gradle build scripts, but that's not the design they chose.)

@gnprice
Copy link
Member Author

gnprice commented Feb 4, 2020

Per the Jetifier docs, Jetifier is

BTW there's a name clash here which is pretty confusing:

  • "Jetifier" is the name of a tool from Android upstream. It does all the things you would hope -- it runs as part of the build pipeline, rather than munging source files in place, and it has a bunch of logic to parse the relevant kinds of files, find actual references to classes, and rewrite those. Correspondingly its source weighs in at about 5000 lines of Kotlin.

  • The author of the RN-world tool which rewrites source files in node_modules/ with string substitutions chose to use the exact same name for their thing. 😢 With the one difference that they don't seem to capitalize it -- it's "jetifier" throughout the README.

@gnprice
Copy link
Member Author

gnprice commented Feb 4, 2020

Pushed an update which emphasizes more strongly the distinction between the two "jetifier" tools, and which takes the point about how one of them won't quite go away even after it stops mattering.

@gnprice
Copy link
Member Author

gnprice commented Mar 20, 2020

(P1 because this is part of #3548.)

chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Mar 20, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX (open PR zulip#3852) and RN v0.60.0 (issue
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Mar 20, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX compatibility (open PR zulip#3852) and using
RN v0.60.0 (issue zulip#3548) may have to be done simultaneously.
@chrisbobbe chrisbobbe mentioned this pull request Mar 20, 2020
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Mar 20, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX compatibility (open PR zulip#3852) and using
RN v0.60.0 (issue zulip#3548) may have to be done simultaneously.

Note: The following failure occurred with `tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Mar 23, 2020
Following the parent, where we introduced Cocoapods, add
react-native-unimodules, using the example Podfile provided by that
package, at
https://gist.github.com/sjchmiela/6c079f2173938a9a61a7c6f053c45000.

react-native-unimodules lets us use individual packages that are
part of the Expo SDK. The one we want is expo-apple-authentication,
for zulip#3964.

Note that there is no caret (^) on 0.6.0! The maintainers have
assumed that if you're using the latest version of
react-native-unimodules, you're also using the latest version of
react-native. We encountered the error message posted at
https://github.com/unimodules/react-native-unimodules/issues/100,
and decided that staying with 0.6.0 is most feasible.

Support for react-native v0.59 dropped somewhere between 0.6.0 and
0.7.0-rc.4, so backwards compatibility (not to mention semantic
versioning) seems not to be a priority. 0.7.0-rc.4 will be important
for us, because that's when AndroidX compatibility is introduced,
according to
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.
This may mean that AndroidX compatibility (open PR zulip#3852) and using
RN v0.60.0 (issue zulip#3548) may have to be done simultaneously.

Note: The following failure occurred with `tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Mar 30, 2020
In particular, we plan to use expo-apple-authentication soon, for
in place of some existing dependencies, such as (non-exhaustively)
react-native-safe-area and react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Also, on Android, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

Note: The following failure occurred with `tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 1, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 1, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 1, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 1, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 1, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 2, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 3, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 3, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), the following failure occurred with
`tools/test deps`:

```
Package "ua-parser-js" wants ^0.7.18 and could get 0.7.21, but got 0.7.20

Found duplicate dependencies in yarn.lock which could be dedup'd.
Run:

  yarn yarn-deduplicate && yarn
```

The suggested command was run, and `tools/test deps` succeeded.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 7, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), ran `yarn yarn-deduplicate` to
deduplicate `ua-parser-js`, following a prompt from
`tools/test deps`.

Fixes: zulip#3987
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 7, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), ran `yarn yarn-deduplicate` to
deduplicate `ua-parser-js`, following a prompt from
`tools/test deps`.

Fixes: zulip#3987
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 8, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

It looks like Unimodules has also gone by the name Universal
Modules, but this may be an old name that has been superseded:
unimodules/unimodules.org@23c53dd...bdc80d9

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A later commit in this series describes another concern with Expo's
handling of version constraints, with expo-application (and others),
with a reference to expo/expo#7728, which
I just filed.

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), ran `yarn yarn-deduplicate` to
deduplicate `ua-parser-js`, following a prompt from
`tools/test deps`.

Fixes: zulip#3987
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Apr 8, 2020
This lets us use individual Expo packages. In particular, we plan to
use expo-apple-authentication soon, for in place of some existing
dependencies, such as (non-exhaustively) react-native-safe-area and
react-native-orientation.

So, add it, without yet using it, following instructions at
https://github.com/unimodules/react-native-unimodules/blob/30da302a8/README.md.

It looks like Unimodules has also gone by the name Universal
Modules, but this may be an old name that has been superseded:
unimodules/unimodules.org@23c53dd...bdc80d9

Note that we omit the caret in 0.6.0, to disallow silent minor
version upgrades. The maintainers are not adhering to semantic
versioning, and they expect that you're using the latest version of
React Native if you're using the latest version of unimodules,
including minor versions; see
https://github.com/unimodules/react-native-unimodules/issues/100#issuecomment-566436685.

In that same comment, a maintainer says that 0.7.0-rc.4 is the
minimum version with AndroidX compatibility. If this is true, then
we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
does not work with RN 0.59.10. That would mean we had to do the
AndroidX upgrade (PR zulip#3852) at the same time as the RN v0.60 upgrade
(issue zulip#3548). Greg thinks it's likely that the maintainer is
mistaken that there's a minimum version of unimodules required to
use AndroidX; the AndroidX engineers designed a smooth migration
path such that this shouldn't happen (see Greg's comment at
zulip#3987 (comment)).

A later commit in this series describes another concern with Expo's
handling of version constraints, with expo-application (and others),
with a reference to expo/expo#7728, which
I just filed.

A couple of fixes had to be made to get it to work on Android:

First, pass an additional argument to a constructor in
MainApplication.java, in a fix given by a maintainer at
https://github.com/unimodules/react-native-unimodules/issues/128#issuecomment-606027692.

Second, specify a new dependency for
`unimodules-react-native-adapter` in our own `android/build.gradle`,
in a fix that is necessary because we're locked on version 0.6.0 of
react-native-unimodules. Filed as
https://github.com/unimodules/react-native-unimodules/issues/130.

Also (not Android-specific), ran `yarn yarn-deduplicate` to
deduplicate `ua-parser-js`, following a prompt from
`tools/test deps`.

Fixes: zulip#3987
@gnprice
Copy link
Member Author

gnprice commented May 8, 2020

Coming back to this as we're now otherwise getting close to completing the RN v0.60 upgrade #3548, because we've successfully migrated to CocoaPods.

Copying from #3548 (comment) my notes on the state of this AndroidX upgrade, what's needed is to rebase, re-test, and merge it, with particular attention to this point from cb87f90:

    In that same comment, a maintainer says that 0.7.0-rc.4 is the
    minimum version with AndroidX compatibility. If this is true, then
    we may have a slight problem, because 0.7.0-rc.4, just like 0.6.0,
    does not work with RN 0.59.10. That would mean we had to do the
    AndroidX upgrade (PR #3852) at the same time as the RN v0.60 upgrade
    (issue #3548). Greg thinks it's likely that the maintainer is
    mistaken that there's a minimum version of unimodules required to
    use AndroidX; the AndroidX engineers designed a smooth migration
    path such that this shouldn't happen (see Greg's comment at
    https://github.com/zulip/zulip-mobile/pull/3987#issuecomment-601918696).

Chris adds:

(As discussed around here, I think that paragraph from my commit message for cb87f90 is correct if you remove "just like 0.6.0". I'm not sure how that made it in, but it's not true; we're happily using 0.6.0 with RN v0.59.10. My apologies.)

@gnprice
Copy link
Member Author

gnprice commented May 8, 2020

Rebased. The one nontrivial change I made to complete the rebase was that we now already have a postinstall script, so I added the jetify call there instead of as the postinstall script on its own.

In a quick smoketest it seems to work fine, but I haven't yet repeated the testing described at the end of the commit message. Next, I'll

  • check up on the unimodules story;
  • repeat that original testing;
  • then if all's well, merge.

@gnprice
Copy link
Member Author

gnprice commented May 8, 2020

I've exercised the unimodules machinery by going to the diagnostics screen and looking at the app version there, which we get via the expo-application / ExpoApplication unimodule. All seems well, so I think the unimodules maintainers were just confused about AndroidX in the way that I predicted earlier.

I've also redone the part where I inspected all the changes the jetify tool (the RN-world one as described above) makes in our node_modules/. There are a few more now, mostly in unimodules/Expo-land, but again they all look appropriate.

Next up is to repeat going through different flows in the app to exercise the affected dependencies, and in both debug and release builds. Then I think I'll be ready to merge this.

AndroidX, previously called the Android Support Library, is a
collection of libraries developed by Android upstream which are
bundled into an app at build time, rather than taken from the system
on the device.  Some are quite central to a typical Android app.

Upstream has renamed the whole thing to AndroidX, and this goes
with a renaming of tons of classes.  (Mostly to get them out of
`android.*`, which was always very confusing because that's the
same namespace used by the actual system packages.)  Docs:
  https://developer.android.com/jetpack/androidx/migrate

This could have been sticky, because lots of apps and lots of
third-party libraries refer to these core libraries, and in a
distributed ecosystem it's impossible for everything to update at
once.  And because these are often so central, a given build of an
app must use the same version of them throughout -- including from
any third-party libraries it uses.  So for example if some
third-party library has updated to use AndroidX, then an app cannot
switch to that version of that library until it too has updated to
use AndroidX.

Fortunately, the planners of this migration recognized this could
be a problem, and found a way to make the constraints run only in
that one direction.  When an app updates to use AndroidX, it can do
so even while it's still using third-party libraries that are still
built to use the Android Support Library.  The key is a tool called
"Jetifier", described in detail here:
  https://developer.android.com/studio/command-line/jetifier
To use it, we set the flag `android.enableJetifier=true`, and then
(quoting the AndroidX migration doc linked above):

  The Android plugin automatically migrates existing third-party
  libraries to use AndroidX by rewriting their binaries.

So the migration path is, apps go first; then libraries update at
their leisure.

---

For us, the most important third-party library (third-party from an
Android perspective) is React Native.  RN took the AndroidX update
in v0.60, so we have to do the same before we can complete that RN
upgrade, i.e. zulip#3548.

The Android upstream way to update a given project (as described by
the doc linked above) has two prongs:

 (a) The project's own source code can be updated automatically
     (barring fancy use of classloaders, etc.) by Android Studio,
     with the feature "Refactor > Migrate to AndroidX...".

 (b) Third-party libraries get updated when they're linked in, by
     rewriting them as part of this project's build.

Both of those appear in this commit.  But in an RN context, there's
one other important set of code: third-party RN libraries get
pulled in as part of *our* Gradle build, with build.gradle lines
using `project`, like so:
    implementation project(':react-native-vector-icons')
This means they're not covered by (b).  This contrasts with RN
itself, which is pulled in as a binary artifact:
    implementation "com.facebook.react:react-native:+"

So for those third-party RN libraries, we need to edit the *source*
code, like (a), before it gets built.  Happily, someone has written
a tool to do just that, and we use it:

 (a') Third-party RN libraries get rewritten in place in
      node_modules/, after `yarn` installs them and before the
      Gradle build.  This uses a different "jetifier" tool:
        https://github.com/mikehardy/jetifier

Specifically, the latter "jetifier" will crawl node_modules; find all
.java, .kt, and .xml files; and apply a list of string substitutions
(about 1900 of them) to rewrite old names to new ones.  That sure is
kind of heuristic... but it's good enough that it will in fact find
any normal import of the affected classes, and on the other hand the
patterns being substituted are all long boring fully-qualified Java
class names, unlikely to be found in spurious places.  For us, I
also inspected (below) the exact edits the tool makes.

---

In this commit, we take the upgrade.

 (a) I used "Refactor > Migrate to AndroidX..." in Android Studio.
     This updated a few spots in our own source code.

 (b) The same auto-refactor added the `gradle.properties` settings
     (in particular enabling Jetifier, the Android-upstream tool)
     that handle libraries we incorporate as binary artifacts,
     including RN itself.

     It also updated dependencies in our `app/build.gradle`.  This
     made the `supportLibVersion` variable unused, and I deleted it.

 (a') We add `jetifier`, the NPM-world tool, as a dev dependency,
      and invoke it in our `postinstall` hook.  This means any run
      of `yarn install` (or bare `yarn`) will end by running it.

I inspected manually (by making copies of `node_modules`, and
comparing with `git diff --no-index`) the effect of `jetifier`.
All the edits look appropriate.  Excluding a few that have no
effect, the edits appear in the following packages:
  react-native
  react-native-image-picker
  react-native-webview
  react-native-photo-view
  rn-fetch-blob
  @react-native-community/netinfo
  @unimodules/react-native-adapter
  unimodules-image-loader-interface
  expo-constants
  expo-file-system
  expo-permissions

(FTR the other edits were in react-native-notifications, which on
Android we don't use or build, and in react-native-device-info and
react-native-sound, limited to IDE metadata plus copies of parts of
the support library itself in build-intermediate directories.)

So once we've upgraded all of those to versions that use AndroidX
in the first place, the `jetifier` tool should become a no-op and
we can delete our references to it.  (It won't go away entirely,
because RN v0.60 has `react-native run-android` run the tool; but
it'll stop mattering.)

I tested the app in both debug and release builds, and exercised
relevant dependencies: picked and uploaded an image; opened the
lightbox; downloaded an image there; entered and left airplane mode
to see the "No Internet connection" banner.  Together those cover all
but the unimodules and expo dependencies; I'm not sure we actually
use each of those anywhere at all, but I exercised the core unimodules
machinery by going to the diagnostics screen to view the app version,
which uses an Expo unimodule.  Everything I tested works.
@gnprice gnprice merged commit e433197 into zulip:master May 8, 2020
@gnprice
Copy link
Member Author

gnprice commented May 8, 2020

Done and merged!

Now 🤞 on what's left of #3548.

@gnprice gnprice deleted the pr-androidx branch May 8, 2020 22:50
@chrisbobbe
Copy link
Contributor

I've exercised the unimodules machinery by going to the diagnostics screen and looking at the app version there, which we get via the expo-application / ExpoApplication unimodule. All seems well, so I think the unimodules maintainers were just confused about AndroidX in the way that I predicted earlier.

I've also redone the part where I inspected all the changes the jetify tool (the RN-world one as described above) makes in our node_modules/. There are a few more now, mostly in unimodules/Expo-land, but again they all look appropriate.

Next up is to repeat going through different flows in the app to exercise the affected dependencies, and in both debug and release builds. Then I think I'll be ready to merge this.

Thanks so much for verifying all this!! The thought hadn't fully formed in my mind that you'd have to do this Unimodules-related investigation yourself, when I said I'd work on #3548 following your merge of #3852. But it sure makes sense in retrospect, and I appreciate you doing it.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request May 18, 2020
A lot of work has already been done toward this:

- adjusting to the newer Flow version (zulip#3827)
- updating to AndroidX (zulip#3852)
- migrating to using CocoaPods at all (zulip#3987)

Also, do what needs to be done atomically with the upgrade.

----- Platform-agnostic --------------------------------------------

After the upgrade, we get this peer dep warning:

warning " > react-native-webview@5.12.1" has incorrect peer dependency "react-native@>=0.57 <0.60".

`react-native-webview@7.0.0` is the minimum supported with RN
v0.60.0. But if we try to get `react-native-webview@7.0.0 before the
upgrade, we get this warning:

warning " > react-native-webview@7.0.0" has incorrect peer dependency "react-native@>=0.60 <0.62".

So, handle `react-native-webview` in the same commit as `react-native`.

----- iOS ----------------------------------------------------------

As far as I've seen, facebook/react-native@2321b3fd7 is the only
cause of iOS changes that must happen atomically with the RN v0.60.0
upgrade.

In that commit, all the "subspecs" that were nested under the
"React" pod are un-nested so they each become their own pod, with
many of these living in their own directory under
node_modules/react-native/Libraries [0].

In our own code, this means changing our Podfile to refer to all
these new pods, instead of the "React" pod's subspecs. So, do.

Our Podfile has a list of "Pods we need that depend on React
Native". We must also be sure all the dependencies in this list
adapt to facebook/react-native@2321b3fd7.

The syntax for pointing explicitly to a subspec is a slash, as in
"React/Core" [1]. Knowing this, we check that list for pods that
explicitly depended on those subspecs, with "React/[...]":

grep -Rl dependency.*React/ --include=\*.podspec --exclude="node_modules/react-native/*" node_modules

There are two, and here's how they handled the change in new
versions that we take in this commit:

 - `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React"
 - `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core"

So, those respond to *explicit* dependencies on a subspec. There are
quite a few that depended on the "React" pod before, and they still
depend on the "React" pod now. So, let's find out if any of these
might have been *implicitly* depending on a subspec.

For that, first note that React Native was a special case before the
upgrade: the React podspec declared a `default_subspec` [1] [2] of
"Core". From reading the docs, that seems to mean that a dependency
on the "Core" subspec, specifically, could have been spelled in one
of two ways: "React/Core", or just "React" [3]. From reading the doc
on `default_subspec`, it seems that this flexibility did not apply
to *all* of the subspecs, though it would have if `default_subspec`
were absent.

So, the only remaining worry is that a "React/Core" / "React-Core"
dependency can no longer be spelled as "React" after the upgrade. If
so, it's possible that some of these "React"-dependent pods were
secretly depending on "React/Core" before, and will need to get an
`s.dependency "React-Core"` line added to them now.

I don't think we have to worry about this. Here's one theory:
"React.podspec", after `facebook/react-native@2321b3fd7`, newly
declares `s.dependency "React-Core"` [4]. I suspect this
accomplishes much the same thing as having "Core" as a
`default_subspec`, as far as we're concerned here: namely, that if
you want "React-Core", you can still just say "React". I wish it
were clear in the CocoaPods docs, or that React Native had cleared
this up in the RN v0.60 release notes and said it's fine to just
depend on "React" when you want "React-Core".

The `zmxv/react-native-sound@2f2c25a69` story (going from
"React/Core" to just "React") may support the conclusion even more
strongly, in that the "React/Core" specificity may have been
intentional (there's very little to say either way, though), and
there was demonstrably no harm in losing it.

`rn-fetch-blob` showed less certainty about handling the upgrade. In
`01f10cb10`, they chose "React". Then, in the same PR [5], they
moved to "React-Core" in 0f6c3e3cc. That PR is a little ambiguous to
read, but to me, it looks like things were working fine with
"React", and they moved to "React-Core" more or less at random.

To be sure, test for ourselves. Before and after making this change
(and clearing `ios/Pods` and running `pod install`):

node_modules/rn-fetch-blob/rn-fetch-blob.podspec
```diff
- s.dependency 'React-Core'
+ s.dependency 'React'
```

there are no build failures, and I can still log `NativeModules`
from `react-native` and see `RNFetchBlob` there. I also see
`RNSound`.

So, it looks like we've done what we need to for
facebook/react-native@2321b3fd7.

[0]: They do still live in node_modules. It's possible for pods to
be hosted online and downloaded on `pod install`, npm-style. For an
example, see the 'Toast' dependency in
node_modules/react-native-simple-toast/react-native-simple-toast.podspec.
But that's not what's happening here, yet.
facebook/react-native@2321b3fd7 hints that this will be the way of
the future for React Native, "to make the experience nicer for
library consumers".

[1]: https://guides.cocoapods.org/syntax/podspec.html#subspec

[2]: https://guides.cocoapods.org/syntax/podspec.html#default_subspecs

[3]: One blog post has an example of this; see
http://www.dbotha.com/2014/12/04/optional-cocoapod-dependencies/#default-subspec.

[4]: It also newly declares a lot of those formerly-subspec
dependencies, like "React-RCTImage", etc. I don't see why this would
have been strictly necessary, and it means we newly can't avoid
linking these things (which we could before, since only "Core" was
listed as a `default_subspec`) and potentially enlarging the iOS
app. Ah well, not a big problem.

[5]: joltup/rn-fetch-blob#397
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request May 23, 2020
Using the RN Upgrade Helper at
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.60.0.

A lot of work has already been done toward this:

- adjusting to the newer Flow version (zulip#3827)
- updating to AndroidX (zulip#3852)
- migrating to using CocoaPods at all (zulip#3987)

Also, do what needs to be done atomically with the upgrade.

Run `yarn yarn-deduplicate && yarn`, following a prompt to do so
from `tools/test deps` that also said this:

"""
Package "babel-preset-fbjs" wants ^3.2.0 and could get 3.3.0, but
got 3.2.0
"""

----- Platform-agnostic --------------------------------------------

We decided never to take a few small changes in this series:

- Edits to `"scripts": { ... }` in our package.json; these are
  configured as we like them to be.

After the upgrade, we get this peer dep warning:

warning " > react-native-webview@5.12.1" has incorrect peer dependency "react-native@>=0.57 <0.60".

`react-native-webview@7.0.0` is the minimum supported with RN
v0.60.0. But if we try to get `react-native-webview@7.0.0 before the
upgrade, we get this warning:

warning " > react-native-webview@7.0.0" has incorrect peer dependency "react-native@>=0.60 <0.62".

So, handle `react-native-webview` in this commit.

----- Android ------------------------------------------------------

Some Android changes never appear in this series:

- Adding the `debug.keystore` file and its config in
  `android/app/build.gradle`. See more explanation and a code
  comment there in another commit in this series.

- Adding `android.useAndroidX=true` and
  `android.enableJetifier=true` in `android/gradle.properties`.
  These were done in e433197.

- Deleting `android/keystores/BUCK` and
  `android/keystores/debug.keystore.properties`: We don't use Buck
  (removal of some of its boilerplate was 1c86488), and we don't
  have an `android/keystores` directory. As mentioned in another
  commit in this series, we're happy with the Android
  Studio-provided debug keystore, and our own release keystore
  described in our release guide.

There are no updates on the Android side that must happen atomically
with the RN upgrade.

----- iOS ----------------------------------------------------------

Some iOS changes don't appear in this series. They fall neatly into
a few touched files:

- ios/ZulipMobile-tvOS/Info.plist: We're not (yet) in the TV
  business. ;)

- ios/ZulipMobile.xcodeproj/project.pbxproj: The purge of many large
  chunks of this file, often known as "the Xcode config file", was
  already done, in 33f4b41.

- ios/ZulipMobile.xcworkspace/contents.xcworkspacedata: After
  33f4b41, this file already looks the way it should.

- ios/ZulipMobile/Info.plist: These changes are done upstream in
  facebook/react-native@7b44fe56f. They fix a duplication issue and
  a code-comment issue, which were apparently causing build
  failures. Our code doesn't have these issues to begin with.

As far as I've seen, facebook/react-native@2321b3fd7 is the only
cause of iOS changes that must happen atomically with the RN v0.60.0
upgrade.

In that commit, all the "subspecs" that were nested under the
"React" pod are un-nested so they each become their own pod, with
many of these living in their own directory under
node_modules/react-native/Libraries [1].

In our own code, this means changing our Podfile to refer to all
these new pods, instead of the "React" pod's subspecs. So, do.

Our Podfile has a list of "Pods we need that depend on React
Native". We must also be sure all the dependencies in this list
adapt to facebook/react-native@2321b3fd7.

The syntax for pointing explicitly to a subspec is a slash, as in
"React/Core" [2]. Knowing this, we check that list for pods that
explicitly depended on those subspecs, with "React/[...]":

```
grep -Rl dependency.*React/ --include=\*.podspec \
  --exclude="node_modules/react-native/*" node_modules
```

There are two, and they both have new versions that adapt to the new accepted shape:

- `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React"
- `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core"

So, take these new versions.

[1]: They do still live in node_modules. It's possible for pods to
     be hosted online and downloaded on `pod install`, npm-style.
     For an example, see the 'Toast' dependency in
     node_modules/react-native-simple-toast/react-native-simple-toast.podspec.
     But that's not what's happening here, yet.
     facebook/react-native@2321b3fd7 hints that this will be the way
     of the future for React Native, "to make the experience nicer
     for library consumers".

[2]: https://guides.cocoapods.org/syntax/podspec.html#subspec
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jun 1, 2020
Closes zulip#3548!

Using the RN Upgrade Helper, a web app showing the diff from the
release/0.59.10 and the release/0.60.6 branches of the
`react-native-community/rn-diff-purge` repo, at
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.60.6.

A lot of work has already been done toward this:

- most of the fixes necessary to adapt to Flow 0.98, without
  upgrading Flow yet (zulip#3827). We do the upgrade in this commit, with
  warnings temporarily suppressed, for smaller commits.

- updating to AndroidX (zulip#3852)

- migrating to using CocoaPods at all (zulip#3987)

Also, do what needs to be done atomically with the upgrade, as
follows.

----- Platform-agnostic --------------------------------------------

After the upgrade, we get this peer dep warning:

warning " > react-native-webview@5.12.1" has incorrect peer dependency "react-native@>=0.57 <0.60".

`react-native-webview@7.0.0` is the minimum supported with RN
v0.60.0. But if we try to get `react-native-webview@7.0.0 before the
upgrade, we get this warning:

warning " > react-native-webview@7.0.0" has incorrect peer dependency "react-native@>=0.60 <0.62".

So, handle `react-native-webview` in this commit.

Also run `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

We've left "scripts" in `package.json` alone; ours work fine as-is.

----- Android ------------------------------------------------------

There are no updates on the Android side that must happen atomically
with the RN upgrade.

Some Android changes never appear in this series:

- Adding the `debug.keystore` file and its config in
  `android/app/build.gradle`. See more explanation and a code
  comment there in another commit in this series.

- Adding `android.useAndroidX=true` and
  `android.enableJetifier=true` in `android/gradle.properties`.
  These were done in e433197.

- Deleting `android/keystores/BUCK` and
  `android/keystores/debug.keystore.properties`: We don't use Buck
  (removal of some of its boilerplate was 1c86488), and we don't
  have an `android/keystores` directory. As mentioned in another
  commit in this series, we're happy with the Android
  Studio-provided debug keystore, and our own release keystore
  described in our release guide.

----- iOS ----------------------------------------------------------

facebook/react-native@2321b3fd7 appears to be the only cause of iOS
changes that must happen atomically with the RN v0.60 upgrade.

In that commit, all the "subspecs" that were nested under the
"React" pod are un-nested so they each become their own pod, with
many of these living in their own directory under
node_modules/react-native/Libraries [1].

In our own code, this means changing our Podfile to refer to all
these new pods, instead of the "React" pod's subspecs. So, do.

Our Podfile has a list of "Pods we need that depend on React
Native". We must also be sure all the dependencies in this list
adapt to facebook/react-native@2321b3fd7.

The syntax for pointing explicitly to a subspec is a slash, as in
"React/Core" [2]. Knowing this, we check that list for pods that
explicitly depended on those subspecs, with "React/[...]":

```
grep -Rl dependency.*React/ --include=\*.podspec \
  --exclude="node_modules/react-native/*" node_modules
```

There are two, and they both have new versions that adapt to the new accepted shape:

- `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React"
- `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core"

So, take these new versions, and job done.

Some iOS changes from the upgrade guide don't appear in this series.
They fall neatly into a few touched files:

- ios/ZulipMobile-tvOS/Info.plist: We're not (yet) in the TV
  business. ;)

- ios/ZulipMobile.xcodeproj/project.pbxproj: The purge of many large
  chunks of this file, often known as "the Xcode config file", was
  already done, in 33f4b41.

- ios/ZulipMobile.xcworkspace/contents.xcworkspacedata: After
  33f4b41, this file already looks the way it should.

- ios/ZulipMobile/Info.plist: These changes are done upstream in
  facebook/react-native@7b44fe56f. They fix a duplication issue and
  a code-comment issue, which were apparently causing build
  failures. Our code doesn't have these issues to begin with.

[1]: They do still live in node_modules. It's possible for pods to
     be hosted online and downloaded on `pod install`, npm-style.
     For an example, see the 'Toast' dependency in
     node_modules/react-native-simple-toast/react-native-simple-toast.podspec.
     But that's not what's happening here, yet.
     facebook/react-native@2321b3fd7 hints that this will be the way
     of the future for React Native, "to make the experience nicer
     for library consumers".

[2]: https://guides.cocoapods.org/syntax/podspec.html#subspec
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jun 1, 2020
Closes zulip#3548!

Using the RN Upgrade Helper, a web app showing the diff from the
release/0.59.10 and the release/0.60.6 branches of the
`react-native-community/rn-diff-purge` repo, at
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.60.6.

A lot of work has already been done toward this:

- most of the fixes necessary to adapt to Flow 0.98, without
  upgrading Flow yet (zulip#3827). We do the upgrade in this commit, with
  warnings temporarily suppressed, for smaller commits.

- updating to AndroidX (zulip#3852)

- migrating to using CocoaPods at all (zulip#3987)

Also, do what needs to be done atomically with the upgrade, as
follows.

Note: The following warning appears and will be fixed with follow-up
work:

```
warn The following packages use deprecated "rnpm" config that will
stop working from next release:
  - react-native-orientation:
    https://github.com/yamill/react-native-orientation#readme
  - rn-fetch-blob: https://npmjs.com/package/rn-fetch-blob
```

----- Platform-agnostic --------------------------------------------

After the upgrade, we get this peer dep warning:

warning " > react-native-webview@5.12.1" has incorrect peer dependency "react-native@>=0.57 <0.60".

`react-native-webview@7.0.0` is the minimum supported with RN
v0.60.0. But if we try to get `react-native-webview@7.0.0 before the
upgrade, we get this warning:

warning " > react-native-webview@7.0.0" has incorrect peer dependency "react-native@>=0.60 <0.62".

So, handle `react-native-webview` in this commit.

Also run `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

We've left "scripts" in `package.json` alone; ours work fine as-is.

----- Android ------------------------------------------------------

There are no updates on the Android side that must happen atomically
with the RN upgrade.

Some Android changes never appear in this series:

- Adding the `debug.keystore` file and its config in
  `android/app/build.gradle`. See more explanation and a code
  comment there in another commit in this series.

- Adding `android.useAndroidX=true` and
  `android.enableJetifier=true` in `android/gradle.properties`.
  These were done in e433197.

- Deleting `android/keystores/BUCK` and
  `android/keystores/debug.keystore.properties`: We don't use Buck
  (removal of some of its boilerplate was 1c86488), and we don't
  have an `android/keystores` directory. As mentioned in another
  commit in this series, we're happy with the Android
  Studio-provided debug keystore, and our own release keystore
  described in our release guide.

----- iOS ----------------------------------------------------------

facebook/react-native@2321b3fd7 appears to be the only cause of iOS
changes that must happen atomically with the RN v0.60 upgrade.

In that commit, all the "subspecs" that were nested under the
"React" pod are un-nested so they each become their own pod, with
many of these living in their own directory under
node_modules/react-native/Libraries [1].

In our own code, this means changing our Podfile to refer to all
these new pods, instead of the "React" pod's subspecs. So, do.

Our Podfile has a list of "Pods we need that depend on React
Native". We must also be sure all the dependencies in this list
adapt to facebook/react-native@2321b3fd7.

The syntax for pointing explicitly to a subspec is a slash, as in
"React/Core" [2]. Knowing this, we check that list for pods that
explicitly depended on those subspecs, with "React/[...]":

```
grep -Rl dependency.*React/ --include=\*.podspec \
  --exclude="node_modules/react-native/*" node_modules
```

There are two, and they both have new versions that adapt to the new accepted shape:

- `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React"
- `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core"

So, take these new versions, and job done.

Some iOS changes from the upgrade guide don't appear in this series.
They fall neatly into a few touched files:

- ios/ZulipMobile-tvOS/Info.plist: We're not (yet) in the TV
  business. ;)

- ios/ZulipMobile.xcodeproj/project.pbxproj: The purge of many large
  chunks of this file, often known as "the Xcode config file", was
  already done, in 33f4b41.

- ios/ZulipMobile.xcworkspace/contents.xcworkspacedata: After
  33f4b41, this file already looks the way it should.

- ios/ZulipMobile/Info.plist: These changes are done upstream in
  facebook/react-native@7b44fe56f. They fix a duplication issue and
  a code-comment issue, which were apparently causing build
  failures. Our code doesn't have these issues to begin with.

[1]: They do still live in node_modules. It's possible for pods to
     be hosted online and downloaded on `pod install`, npm-style.
     For an example, see the 'Toast' dependency in
     node_modules/react-native-simple-toast/react-native-simple-toast.podspec.
     But that's not what's happening here, yet.
     facebook/react-native@2321b3fd7 hints that this will be the way
     of the future for React Native, "to make the experience nicer
     for library consumers".

[2]: https://guides.cocoapods.org/syntax/podspec.html#subspec
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jun 3, 2020
Using the RN Upgrade Helper, a web app showing the diff from the
release/0.59.10 and the release/0.60.6 branches of the
`react-native-community/rn-diff-purge` repo, at
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.60.6.

- Upgrade `react-native`, `react`, and `flow-bin` following the
  templates.

- Upgrade `react-native-webview` to satisfy peer dependencies and
  remove the outdated libdef (to be replaced in an upcoming commit).

- Adapt our Podfile to RN v0.60's new layout of pods, following the templates.

- Upgrade `react-native-sound` and `rn-fetch-blob` to versions with
  podspecs compatible with the new pod layout in RN v0.60. A lot of
  work has already been done toward this:

- We ignore or have already handled several changes to the Xcode and
  Gradle configs.

A lot of work has already been done toward the upgrade:

- most of the fixes necessary to adapt to Flow 0.98, without
  upgrading Flow yet (zulip#3827). We do the upgrade in this commit, with
  warnings temporarily suppressed, for smaller commits.

- updating to AndroidX (zulip#3852)

- migrating to using CocoaPods at all (zulip#3987)

Note: The following warning appears when running Metro and will be
fixed with follow-up work:

```
warn The following packages use deprecated "rnpm" config that will
stop working from next release:
  - react-native-orientation:
    https://github.com/yamill/react-native-orientation#readme
  - rn-fetch-blob: https://npmjs.com/package/rn-fetch-blob
```

----- Platform-agnostic --------------------------------------------

After the upgrade, we get this peer dep warning:

"""
warning " > react-native-webview@5.12.1" has incorrect peer
dependency "react-native@>=0.57 <0.60".
"""

`react-native-webview@7.0.0` is the minimum supported with RN
v0.60.0. But if we try to get `react-native-webview@7.0.0 before the
upgrade, we get this warning:

"""
warning " > react-native-webview@7.0.0" has incorrect peer
dependency "react-native@>=0.60 <0.62".
"""

In a previous commit, we removed the outdated libdef, with a
suppression. We upgrade `react-native-webview` to at least 7.0.0 in
this commit, then add the updated libdef in an upcoming commit.

`react-native-webview` does declare that it follows semantic
versioning [1], so we can feel comfortable taking a 7.x version
later than 7.0.0. We take 7.6.0, the latest. Nicely, this gets us
the changes from one of our PRs, released in 7.0.3; see 1982f3f
and its reversion in the commit that followed, bbfac73.

Handling the declared breaking changes [1] is straightforward:

- Update to AndroidX (v6.0.2). If this is a real breaking change, we
  handled it in e433197.

- UIWebView removed (7.0.1). This prompted the `scalesPageToFit`
  prop to be removed, but we don't use it. The `useWebKit` prop was
  also removed because it doesn't make sense for it to be anything
  but `true` now. So, remove our use of it.

Also run `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

We've left "scripts" in `package.json` alone; ours work fine as-is.

[1]: https://github.com/react-native-community/react-native-webview#versioning

----- Android ------------------------------------------------------

There are no updates on the Android side that must happen atomically
with the RN upgrade.

Some Android changes never appear in this series:

- Adding the `debug.keystore` file and its config in
  `android/app/build.gradle`; we don't take these changes at all.
  See 9ccaa21.

- Adding `android.useAndroidX=true` and
  `android.enableJetifier=true` in `android/gradle.properties`.
  These were done in e433197.

- Deleting `android/keystores/BUCK` and
  `android/keystores/debug.keystore.properties`: We don't use Buck
  (removal of some of its boilerplate was 1c86488), and we don't
  have an `android/keystores` directory. As mentioned in 9ccaa21,
  we're happy with the Android Studio-provided debug keystore, and
  our own release keystore described in our release guide.

----- iOS ----------------------------------------------------------

facebook/react-native@2321b3fd7 appears to be the only cause of iOS
changes that must happen atomically with the RN v0.60 upgrade.

In that commit, all the "subspecs" that were nested under the
"React" pod are un-nested so they each become their own pod, with
many of these living in their own directory under
node_modules/react-native/Libraries [1].

In our own code, this means changing our Podfile to refer to all
these new pods, instead of the "React" pod's subspecs. So, do.

Our Podfile has a list of "Pods we need that depend on React
Native". We must also be sure all the dependencies in this list
adapt to facebook/react-native@2321b3fd7.

The syntax for pointing explicitly to a subspec is a slash, as in
"React/Core" [2]. Knowing this, we check that list for pods that
explicitly depended on those subspecs, with "React/[...]":

```
grep -Rl dependency.*React/ --include=\*.podspec \
  --exclude="node_modules/react-native/*" node_modules
```

There are two, and they both have new versions that adapt to the new
accepted shape:

- `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React"
- `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core"

So, take these new versions, and job done.

Some iOS changes from the upgrade guide don't appear in this series.
They fall neatly into a few touched files:

- ios/ZulipMobile-tvOS/Info.plist: We're not (yet) in the TV
  business. ;)

- ios/ZulipMobile.xcodeproj/project.pbxproj: The purge of many large
  chunks of this file, often known as "the Xcode config file", was
  already done, in 33f4b41.

- ios/ZulipMobile.xcworkspace/contents.xcworkspacedata: After
  33f4b41, this file already looks the way it should.

- ios/ZulipMobile/Info.plist: These changes are done upstream in
  facebook/react-native@7b44fe56f. They fix a duplication issue and
  a code-comment issue, which were apparently causing build
  failures. Our code doesn't have these issues to begin with.

[1]: They do still live in node_modules. It's possible for pods to
     be hosted online and downloaded on `pod install`, npm-style.
     For an example, see the 'Toast' dependency in
     node_modules/react-native-simple-toast/react-native-simple-toast.podspec.
     But that's not what's happening here, yet.
     facebook/react-native@2321b3fd7 hints that this will be the way
     of the future for React Native, "to make the experience nicer
     for library consumers".

[2]: https://guides.cocoapods.org/syntax/podspec.html#subspec

Fixes: zulip#3548
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Jun 3, 2020
Using the RN Upgrade Helper, a web app showing the diff from the
release/0.59.10 and the release/0.60.6 branches of the
`react-native-community/rn-diff-purge` repo, at
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.60.6.

In this commit:

- Upgrade `react-native`, `react`, and `flow-bin` following the
  templates

- Upgrade `react-native-webview` to satisfy peer dependencies

- Adapt our Podfile to RN v0.60's new layout of pods, following the
  templates

- Upgrade `react-native-sound` and `rn-fetch-blob` to versions with
  podspecs compatible with the new pod layout in RN v0.60

A lot of work has already been done toward the upgrade:

- most of the fixes necessary to adapt to Flow 0.98, without
  upgrading Flow yet (zulip#3827). We do the upgrade in this commit, with
  warnings temporarily suppressed, for smaller commits.

- updating to AndroidX (zulip#3852)

- migrating to using CocoaPods at all (zulip#3987)

- We ignore or have already handled several changes, e.g., to the
  Xcode and Gradle configs (see comment at zulip#3548).

Note: The following warning appears when running Metro and will be
fixed with follow-up work (e.g., zulip#4118):

```
warn The following packages use deprecated "rnpm" config that will
stop working from next release:
  - react-native-orientation:
    https://github.com/yamill/react-native-orientation#readme
  - rn-fetch-blob: https://npmjs.com/package/rn-fetch-blob
```

----- Platform-agnostic --------------------------------------------

We upgrade `react-native-webview` in this commit because versions
before 7.x aren't compatible with RN v0.60, and versions 7.x aren't
compatible with RN v0.59 (judging by peer dependency warnings). We
take the latest 7.x version, 7.6.0.

Nicely, this gets us the changes from one of our PRs, released in
7.0.3; see 1982f3f and its reversion in the commit that followed,
bbfac73.

There's just one declared breaking change [1] in 7.x.x:

- UIWebView removed (7.0.1).

This prompted the `scalesPageToFit` prop to be removed, but we don't
use it. The `useWebKit` prop was also removed because it doesn't
make sense for it to be anything but `true` now. So, remove our use
of it.

Also run `yarn yarn-deduplicate && yarn`, as prompted by
`tools/test deps`.

[1]: https://github.com/react-native-community/react-native-webview#versioning

----- Android ------------------------------------------------------

There are no updates on the Android side that must happen atomically
with the RN upgrade.

----- iOS ----------------------------------------------------------

facebook/react-native@2321b3fd7 appears to be the only cause of iOS
changes that must happen atomically with the RN v0.60 upgrade.

In that commit, all the "subspecs" that were nested under the
"React" pod are un-nested so they each become their own pod, with
many of these living in their own directory under
node_modules/react-native/Libraries [1].

In our own code, this means changing our Podfile to refer to all
these new pods, instead of the "React" pod's subspecs. So, do.

Our Podfile has a list of "Pods we need that depend on React
Native". We must also be sure all the dependencies in this list
adapt to facebook/react-native@2321b3fd7.

The syntax for pointing explicitly to a subspec is a slash, as in
"React/Core" [2]. Knowing this, we check that list for pods that
explicitly depended on those subspecs, with "React/[...]":

```
grep -Rl dependency.*React/ --include=\*.podspec \
  --exclude="node_modules/react-native/*" node_modules
```

There are two, and they both have new versions that adapt to the new
accepted shape:

- `zmxv/react-native-sound@2f2c25a69`: "React/Core" -> "React"
- `joltup/rn-fetch-blob`, `01f10cb10^..0f6c3e3cc`: "React/Core" -> "React-Core"

So, take these new versions, and job done.

[1]: They do still live in node_modules. It's possible for pods to
     be hosted online and downloaded on `pod install`, npm-style.
     For an example, see the 'Toast' dependency in
     node_modules/react-native-simple-toast/react-native-simple-toast.podspec.
     But that's not what's happening here, yet.
     facebook/react-native@2321b3fd7 hints that this will be the way
     of the future for React Native, "to make the experience nicer
     for library consumers".

[2]: https://guides.cocoapods.org/syntax/podspec.html#subspec

Fixes: zulip#3548
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.

None yet

3 participants