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.61 #3781

Closed
gnprice opened this issue Jan 9, 2020 · 9 comments · Fixed by #4151
Closed

Upgrade to RN v0.61 #3781

gnprice opened this issue Jan 9, 2020 · 9 comments · Fixed by #4151
Assignees
Labels
upstream: RN Issues related to an issue in React Native

Comments

@gnprice
Copy link
Member

gnprice commented Jan 9, 2020

We won't meaningfully start on this until we complete the upgrade to RN v0.60, #3548. Filing to have an identifier to refer to. (We're now on v0.60.)

Released 2019-09. Release notes here:
https://facebook.github.io/react-native/blog/2019/09/18/version-0.61

Exciting improvements include:

Possible incompatible changes, requiring work from us on upgrade, include:

  • "Remove React .xcodeproj". IIUC, this basically requires us to have moved fully to CocoaPods, from the old way of configuring the iOS build; I believe we'll do this already as part of the work to upgrade to RN v0.60.
  • That Flow upgrade, surely.
  • (Surely there will be more. But maybe a lot less than the last couple of upgrades! Discussion at Upgrade to RN v0.61 #3781 (comment) .)
@gnprice gnprice added the blocked on other work To come back to after another related PR, or some other task. label Jan 9, 2020
This was referenced Jan 9, 2020
@rk-for-zulip
Copy link
Contributor

rk-for-zulip commented Feb 4, 2020

(edit: mistaken, thankfully -- see #3781 (comment)) Debugger warnings in RN 0.60 indicate that the React componentWillReceiveProps lifecycle callback will be removed in RN 0.61. This callback is currently used in ComposeBox and AnimatedScaleComponent.

@gnprice
Copy link
Member Author

gnprice commented May 8, 2020

One exciting change is that this old RN bug is reportedly fixed in v0.61!
facebook/react-native#20119

TextInput becomes slow after lots of typing

That's the mitigated version of the RN bug that got much worse back in v0.55 (vs. v0.54 and earlier), which our users experienced as #2589. I fixed that one upstream so that in v0.56 it was back to the level of v0.55 and earlier; but it's encouraging that the underlying bug now seems to be fixed too.

(Though there's still some uncertainty because, as discussed on that issue thread, nobody knows how it got fixed. 🤞 )

@gnprice
Copy link
Member Author

gnprice commented Jun 4, 2020

I think it's likely that this upgrade will turn out to be a lot less work than the v0.60 upgrade (or the v0.59 upgrade before that.)

My information on this is circumstantial: it seems like for whatever reason a lot of people are (at least as of like a month ago) still on v0.59, and a lot are on v0.61 or v0.62, and very few are on v0.60. My data for that is from being subscribed to an RN issue thread where a few people a month report it's still broken for them, and I noticed sometime like a month ago that the reports in 2020-to-date or so followed that pattern. (Originally mentioned this the other day in chat.)

So, hopefully it turns out that way when we go to try it!

@gnprice gnprice removed the blocked on other work To come back to after another related PR, or some other task. label Jun 8, 2020
@gnprice
Copy link
Member Author

gnprice commented Jun 8, 2020

Debugger warnings in RN 0.60 indicate that the React componentWillReceiveProps lifecycle callback will be removed in RN 0.61.

This isn't correct. Here's the warning:

YellowBox.js:71 Warning: componentWillReceiveProps is deprecated and will be removed in the next major version. Use static getDerivedStateFromProps instead.

Please update the following components: Connect(HomeTab), Connect(IconUnreadConversations), Connect(IconUnreadMentions), Connect(LoadingBanner), Connect(OfflineNotice), Connect(OwnAvatar), Connect(UnreadCards), SafeView

Learn more about this warning here:
https://fb.me/react-async-component-lifecycle-hooks

As the linked article explains, this is a React warning, and the "next major version" in question is React 17. There is not yet a React 17, so RN v0.61 doesn't use it. Nor does RN v0.62 (which is out already), or v0.63 (which has a release candidate out.)

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Jun 9, 2020

EDIT: This has been resolved. Thanks, @gnprice!

I commented at #3635 (comment) about some new errors I'm seeing with Flow 105; it looks like that PR would make things more sensible in a way that probably eliminates those errors. 🙂

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Jun 10, 2020

Similarly to #3548 (comment), this comment lists the changes that appear in the upgrade helper but that we don't do in this series. Making this list is essential to make sure we don't miss anything, but putting it here instead of in commit messages will help avoid over-crowding the commit messages.

Platform-agnostic

  • We don't touch "scripts" in package.json; ours are working fine, and, in fact, the two newly added ones are already present.
  • We don't upgrade react-test-renderer because we removed that as unnecessary in 86768ac.
  • We don't upgrade ESLint from ^6.4.0 to ^6.5.1. Upgrading to ^6.8.0 is open as Upgrade to ESLint 6.8.0 #4120.
  • We don't take a cosmetic change to App.js (replacing <Fragment> with <>) that doesn't apply to our corresponding file, src/ZulipMobile.js.

Android

iOS

  • Lots of changes in the project.pbxproj file are observed in the upgrade helper, but none happen in the app template. Looking at the changes, it all just seems to be rearranging stuff. More discussion at RN v61 upgrade #4151 (comment).

@chrisbobbe chrisbobbe self-assigned this Jun 10, 2020
@gnprice
Copy link
Member Author

gnprice commented Jun 10, 2020

  • I'm not really sure what Buck is, but we don't use it

It's a build system: https://buck.build/. It's closely related to Bazel -- Buck is the result of ex-Googlers at Facebook reimplementing Google's internal build system called Blaze, and a few years later Google open-sourced Blaze and called it Bazel.

It's probably a fine system, but the open-source RN ecosystem uses Gradle and not Buck, so it wouldn't make sense for us to use it -- we'd have to manually maintain Buck config for each dependency that has any Android-native pieces. Really it doesn't make much sense to me why there's a Buck config in the template app, because the number of people who've benefited from that template file has got to be approximately zero.

(I assume that inside Facebook, they do use Buck for their RN. I think they use it for just about everything else.)

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Jun 11, 2020

"Fast Refresh", which replaces both "live reload" and "hot reload", and is supposed to be more complete and reliable. That'd be a pretty great improvement to the dev experience!

Goodness gracious, this is frighteningly fast, at least in my testing on iOS. Glad to see this. 🙂

@agrawal-d
Copy link
Member

Hot Reload almost never worked for me! I'm eagerly waiting to test Fast Refresh! 😀

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 7, 2020
The immediate need is to set up a way to mock the new `ZLPConstants`
in `NativeModules`, which we'll do in an upcoming commit.

Doing this, as recommended by React Native [1], also means we're
better prepared for the React Native v0.61 upgrade (zulip#3781), in which
Haste is removed [2]. A consequence of that removal, it seems, is
that mocks like this one, which we have now:

```
jest.mock('Linking', () => { ... }`
```

, won't work. Several people have handled this by changing 'Linking'
to something like 'react-native/Libraries/Linking/Linking', but this
is brittle because it couples our tests with the current directory
structure in 'react-native'. Better to do it this way.

We considered following the advice of others at that issue,
including a blog post [3] responding to the official suggestion with
an alternative. But we didn't reproduce the problems the post's
author mentioned, and we've so far been able to explain the hiccups
we've seen.

[1] facebook/react-native#26579 (comment)
[2] facebook/react-native#26579 (comment)
[3] facebook/react-native#26579 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 8, 2020
Doing this, as recommended by React Native [1], means we're better
prepared for the React Native v0.61 upgrade (zulip#3781), in which Haste
is removed [2]. A consequence of that removal, it seems, is that
mocks like this one, which we have now:

```
jest.mock('Linking', () => { ... }`
```

, won't work. Several people have handled this by changing 'Linking'
to something like 'react-native/Libraries/Linking/Linking', but this
is brittle because it couples our tests with the current directory
structure in 'react-native'. Better to do it this way.

We considered following the advice of others at that issue,
including a blog post [3] responding to the official suggestion with
an alternative. But we didn't reproduce the problems the post's
author mentioned, and we've so far been able to explain the hiccups
we've seen.

[1] facebook/react-native#26579 (comment)
[2] facebook/react-native#26579 (comment)
[3] facebook/react-native#26579 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 8, 2020
The immediate need is to set up a way to mock the new `ZLPConstants`
in `NativeModules`, which we'll do in an upcoming commit.

Doing this, as recommended by React Native [1], also means we're
better prepared for the React Native v0.61 upgrade (zulip#3781), in which
Haste is removed [2]. A consequence of that removal, it seems, is
that mocks like this one, which we have now:

```
jest.mock('Linking', () => { ... }`
```

, won't work. Several people have handled this by changing 'Linking'
to something like 'react-native/Libraries/Linking/Linking', but this
is brittle because it couples our tests with the current directory
structure in 'react-native'. Better to do it this way.

We considered following the advice of others at that issue,
including a blog post [3] responding to the official suggestion with
an alternative. But we didn't reproduce the problems the post's
author mentioned, and we've so far been able to explain the hiccups
we've seen.

[1] facebook/react-native#26579 (comment)
[2] facebook/react-native#26579 (comment)
[3] facebook/react-native#26579 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 9, 2020
Using the RN Upgrade Helper, a web app showing the diff from the
release/0.60.6 and the release/0.61.5 branches of the
`react-native-community/rn-diff-purge` repo, at
https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5.

In this commit:

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

- Change our Podfile to reflect new/changed iOS dependencies in RN,
  following the template

- Make changes to adapt to multiple upgrades of RN's Hermes
  dependency (details below)

- Run `yarn yarn-deduplicate && yarn` as prompted by
  `tools/test deps`

See (on the issue) a list of changes from the upgrade helper that we
don't do in this series [1].

Hermes details:

In facebook/react-native@c21e36db4, React Native started using
v0.1.1 of Hermes [2], which includes a rename of the NPM package
(facebook/hermes@c74842e) from 'hermesvm' to 'hermes-engine'. So,
use the new path in the one place where its path occurs in our code,
following the changes to the template in that commit.

In facebook/react-native@06c64f5f1, React Native started using
v0.2.1 of Hermes [3], which was came with the announcement, "The C++
runtime library is now packaged in a separate AAR to avoid
`pickFirst` nondeterminisms". Follow the changes to the template in
that commit, where several `pickFirst` lines are removed.

[1]: zulip#3781 (comment)
[2]: https://github.com/facebook/hermes/releases/tag/v0.1.1
[3]: https://github.com/facebook/hermes/releases/tag/v0.2.1

Fixes: zulip#3781
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 10, 2020
Doing this, as recommended by React Native [1], means we're better
prepared for the React Native v0.61 upgrade (zulip#3781), in which Haste
is removed [2]. A consequence of that removal, it seems, is that
mocks like this one, which we have now:

```
jest.mock('Linking', () => { ... }`
```

, won't work. Several people have handled this by changing 'Linking'
to something like 'react-native/Libraries/Linking/Linking', but this
is brittle because it couples our tests with the current directory
structure in 'react-native'. Better to do it this way.

We considered following the advice of others at that issue,
including a blog post [3] responding to the official suggestion with
an alternative. But we didn't reproduce the problems the post's
author mentioned, and we've so far been able to explain the hiccups
we've seen.

[1] facebook/react-native#26579 (comment)
[2] facebook/react-native#26579 (comment)
[3] facebook/react-native#26579 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Jul 10, 2020
Using the RN Upgrade Helper, a web app showing the diff from the
release/0.60.6 and the release/0.61.5 branches of the
`react-native-community/rn-diff-purge` repo, at
https://react-native-community.github.io/upgrade-helper/?from=0.60.6&to=0.61.5.

In this commit:

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

- Change our Podfile to reflect new/changed iOS dependencies in RN,
  following the template

- Make changes to adapt to multiple upgrades of RN's Hermes
  dependency (details below)

- Run `yarn yarn-deduplicate && yarn` as prompted by
  `tools/test deps`

See (on the issue) a list of changes from the upgrade helper that we
don't do in this series [1].

Hermes details:

In facebook/react-native@c21e36db4, React Native started using
v0.1.1 of Hermes [2], which includes a rename of the NPM package
(facebook/hermes@c74842e) from 'hermesvm' to 'hermes-engine'. So,
use the new path in the one place where its path occurs in our code,
following the changes to the template in that commit.

In facebook/react-native@06c64f5f1, React Native started using
v0.2.1 of Hermes [3], which was came with the announcement, "The C++
runtime library is now packaged in a separate AAR to avoid
`pickFirst` nondeterminisms". Follow the changes to the template in
that commit, where several `pickFirst` lines are removed.

[1]: zulip#3781 (comment)
[2]: https://github.com/facebook/hermes/releases/tag/v0.1.1
[3]: https://github.com/facebook/hermes/releases/tag/v0.2.1

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

In this commit:

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

- Change our Podfile to reflect new/changed iOS dependencies in RN,
  following the template

- Make changes to adapt to multiple upgrades of RN's Hermes
  dependency (details below)

- Run `yarn yarn-deduplicate && yarn` as prompted by
  `tools/test deps`

See (on the issue) a list of changes from the upgrade helper that we
don't do in this series [1].

Hermes details:

In facebook/react-native@c21e36db4, React Native started using
v0.1.1 of Hermes [2], which includes a rename of the NPM package
(facebook/hermes@c74842e) from 'hermesvm' to 'hermes-engine'. So,
use the new path in the one place where its path occurs in our code,
following the changes to the template in that commit.

In facebook/react-native@06c64f5f1, React Native started using
v0.2.1 of Hermes [3], which was came with the announcement, "The C++
runtime library is now packaged in a separate AAR to avoid
`pickFirst` nondeterminisms". Follow the changes to the template in
that commit, where several `pickFirst` lines are removed.

[1]: zulip#3781 (comment)
[2]: https://github.com/facebook/hermes/releases/tag/v0.1.1
[3]: https://github.com/facebook/hermes/releases/tag/v0.2.1

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

In this commit:

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

- Change our Podfile to reflect new/changed iOS dependencies in RN,
  following the template

- Make changes to adapt to multiple upgrades of RN's Hermes
  dependency (details below)

- Run `yarn yarn-deduplicate && yarn` as prompted by
  `tools/test deps`

See (on the issue) a list of changes from the upgrade helper that we
don't do in this series [1].

Hermes details:

In facebook/react-native@c21e36db4, React Native started using
v0.1.1 of Hermes [2], which includes a rename of the NPM package
(facebook/hermes@c74842e) from 'hermesvm' to 'hermes-engine'. So,
use the new path in the one place where its path occurs in our code,
following the changes to the template in that commit.

In facebook/react-native@06c64f5f1, React Native started using
v0.2.1 of Hermes [3], which was came with the announcement, "The C++
runtime library is now packaged in a separate AAR to avoid
`pickFirst` nondeterminisms". Follow the changes to the template in
that commit, where several `pickFirst` lines are removed.

[1]: zulip#3781 (comment)
[2]: https://github.com/facebook/hermes/releases/tag/v0.1.1
[3]: https://github.com/facebook/hermes/releases/tag/v0.2.1

Fixes: zulip#3781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream: RN Issues related to an issue in React Native
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants