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

Upgrade to RN v0.60 #3548

Closed
gnprice opened this issue Jul 9, 2019 · 12 comments · Fixed by #4133
Closed

Upgrade to RN v0.60 #3548

gnprice opened this issue Jul 9, 2019 · 12 comments · Fixed by #4133
Assignees

Comments

@gnprice
Copy link
Member

gnprice commented Jul 9, 2019

Released last week:
https://facebook.github.io/react-native/blog/2019/07/03/version-60

First we'll want to complete the upgrade to v0.59 <_< >_>, aka #3399. (done!)

From the release notes, notable changes include:

  • Migrated to AndroidX -- this will take some work on upgrading.
  • [edit:] CocoaPods. See Set up CocoaPods #3983.
  • There's a new recommended upgrade process.
  • (I should read the rest of the release notes more completely -- getting this down quickly for now.)

Also:

  • Flow v0.98.

The bulk of this work is now done. Details in #3548 (comment) and in #4133 .

Some known changes that we already take advantage of in #4133:

Other known changes that this will open up the option to do -- in particular I went through this thread and followed all the backlinks to related issues:

Known cleanups we should probably follow this with:

Oh and 🙂 :

@gnprice gnprice added blocked on other work To come back to after another related PR, or some other task. P1 high-priority labels Jul 9, 2019
@gnprice gnprice self-assigned this Jul 9, 2019
@gnprice gnprice removed the blocked on other work To come back to after another related PR, or some other task. label Jul 25, 2019
@gnprice
Copy link
Member Author

gnprice commented Jul 25, 2019

#3399 is now done! We're on RN v0.59, and so this upgrade to v0.60 is no longer blocked.

jainkuniya added a commit to jainkuniya/zulip-mobile that referenced this issue Aug 18, 2019
Not upgrading to latest 4.x.x because it comes with AndroidX,
which will not work for us until we upgrade to RN v0.60 (zulip#3548).

Tested on both Android & iOS, it is working fine.

Fixes: zulip#3568
gnprice pushed a commit to jainkuniya/zulip-mobile that referenced this issue Aug 20, 2019
Not upgrading to latest 4.x.x because it comes with AndroidX,
which will not work for us until we upgrade to RN v0.60 (zulip#3548).

Tested on both Android & iOS, it is working fine.

Fixes: zulip#3568
gnprice added a commit that referenced this issue Sep 5, 2019
I've just made this "fork" repo, and a `zulip` branch in it, in order
to make a fix for a noisy bit of logging that was showing up even in
`tools/test android`, where we suppress non-error logs.

This commit zulip/react-native-webview#49aca8206 consists of a single
small patch atop v5.12.1, which we had been using.

I sent the fix as a PR upstream:
  react-native-webview/react-native-webview#844
which was promptly merged (thanks, upstream maintainers!), and
even promptly released, as 7.0.3.  We're still on 5.x until we
deal with the RN upgrade to v0.60 (#3548), so for now we stick
the patch in this small fork-branch of our own.

(This is one of those times I really wish we had comments in
package.json. :-/  The key information in this commit message
really ought to be in a 1- or 2-line comment in the file.)
@rk-for-zulip
Copy link
Contributor

rk-for-zulip commented Jan 16, 2020

A list of known dependencies whose update is blocked by the transition to RN v0.60:

Additionally, react-native-photo-view is unmaintained, and must be brought up to date via Jetifier if still used. (Possible replacements include image-viewing and image-zoom-viewer, both of which are JS-only.)

@gnprice
Copy link
Member Author

gnprice commented Jan 29, 2020

(Belatedly updating the assignment metadata.)

gnprice added a commit to gnprice/zulip-mobile that referenced this issue Jan 30, 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. 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
     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.

Tested and confirmed working:
 * `tools/test android`
 * `react-native run-android` -> app starts up,
   I can navigate around and read messages, etc.
 * Lightbox works (which uses react-native-photo-view).

To do:
 * Test a release build.
 * Test functionality of react-native-image-picker
   and @react-native-community/netinfo .
@gnprice
Copy link
Member Author

gnprice commented Jan 30, 2020

Good news on the AndroidX upgrade! I sat down yesterday afternoon to try it, and it looks like it will be (a) a small change that (b) does not depend on the RN upgrade, and that in fact we could have done at any time in the last year or so.

I have a draft commit to do this; I think it's most likely complete as is, but there are a few more things I want to test.

Detailed discussion in this (long!) commit message: 5cec78f

gnprice added a commit to gnprice/zulip-mobile that referenced this issue 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. 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
     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.
gnprice added a commit to gnprice/zulip-mobile that referenced this issue Feb 4, 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 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 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 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 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.
chrisbobbe pushed a commit to chrisbobbe/zulip-mobile that referenced this issue 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.
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this issue 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 7, 2020

Returning to this issue because #4091 is a reminder that it'd be great to be on a more recent version.

I believe at this point we've largely completed the hard work required for this:

The remaining tasks are

    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).
  • Try the upgrade, test for any remaining breakage, and triage that

  • Fix that remaining breakage 🙂 which hopefully is all small

@chrisbobbe
Copy link
Contributor

(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.)

I also think most of the work of the RN v0.60 upgrade has been done. @gnprice, I'm happy to drive that, if you like (following your merge of #3852), but I want to finish Sign in with Apple first, if that's OK. I think it's wise to suppose a RN upgrade will be full of nice surprises, some of which could lurk undetected in our dependencies for days or weeks after a main PR is merged, and will have to be dealt with, possibly by the person who did the actual upgrade. (Obviously we'll try our best to avoid this happening.)

@gnprice gnprice assigned chrisbobbe and unassigned rk-for-zulip May 8, 2020
@gnprice
Copy link
Member Author

gnprice commented May 8, 2020

That plan sgtm!

gnprice added a commit to gnprice/zulip-mobile that referenced this issue May 8, 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 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
  @react-native-community/netinfo

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 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.
gnprice added a commit to gnprice/zulip-mobile that referenced this issue May 8, 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 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.
@chrisbobbe
Copy link
Contributor

chrisbobbe commented May 18, 2020

Following facebook/react-native@2321b3fd7, we're pretty sure it's fine for any dependency's .podspec file to have a line like s.dependency "React" when it depends on code specific to one of the modules that was previously a "subspec" but is now a separate pod, e.g., "React/Core" (the "Core" subspec) n.k.a. "React-Core" (a pod thus named).

"React/Core", n.k.a. "React-Core", is AFAIK the only one that might ever need to be examined, because a default_subspec was declared to be "Core" in "React.podspec", previously, so "React/Core" could have been spelled "React", while, e.g., "React/RCTImage" could not have been spelled "React". More speculation, experiments, and links here.

For the ones besides "Core", they would have had to be spelled, e.g., "React/RCTImage", which would have immediately thrown errors with RN 60 and prompted the switch to something correct, likely "React-RCTImage".

This was referenced May 29, 2020
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue 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 issue 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 issue 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
Copy link
Contributor

As Greg suggests at #4133 (comment), I'm cutting out the parts from the commit message that talk about work not done in the upgrade series, and putting it here:

Platform-agnostic

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

Android

  • 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

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.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue 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
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 4, 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

We've already done a lot of the work toward the upgrade, and we've
ignored some changes suggested by the upgrade helper. See comments
on 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
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 5, 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

We've already done a lot of the work toward the upgrade, and we've
ignored some changes suggested by the upgrade helper. See comments
on 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

Fixes: zulip#4093
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 5, 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

We've already done a lot of the work toward the upgrade, and we've
ignored some changes suggested by the upgrade helper. See comments
on 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

Fixes: zulip#4093
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jun 8, 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

We've already done a lot of the work toward the upgrade, and we've
ignored some changes suggested by the upgrade helper. See comments
on 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
Fixes: zulip#4093
@gnprice gnprice closed this as completed in 56f44d3 Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants