React native upgrade 0.62.2 rebased #1586

andrepimenta · 2020-05-18T15:42:28Z

Description

This PR addresses the upgrade from React Native 0.59.10 to 0.62.2. Although there were lots of changes necessary, an effort was made in order to keep as much as possible the version of the libraries that were already being used (no unnecessary library upgrades were made), and also to keep as much of the code as intact as possible (no unnecessary code changes were made). This makes it easier to track any possible problems to the React Native upgrade itself. The upgrading of libraries to a newer current version is a step 2 outside of this PR context.

Note: To run this PR, don't forget to check the changes on the README. More specifically installing the pods:
brew install cocoapods
And on the root of the project
cd ios && pod install && cd ..

To open the project on Xcode, the file MetaMask.xcworkspace should be used instead of MetaMask.xcodeproj.

yarn clean also installs the pods now.

General file changes

The first part of the upgrade was making all the file changes needed in order to integrate the new React Native version. Those changes can be followed on this upgrade-helper.

Library changes

One of the biggest changes in React 0.60+ is the integration by default of the pods system. This means every library needed to be unlinked. This is because with the new system, every library is automatically linked. More specifically, the libraries that were unlinked were:

react-native-sensors
react-native-reanimated
react-native-webview
@react-native-community/netinfo
react-native-view-shot
lottie-react-native
@react-native-community/async-storage
react-native-push-notification
react-native-background-timer
react-native-device-info
react-native-svg
react-native-gesture-handler
react-native-screens
react-native-branch
react-native-camera
react-native-fs
react-native-i18n
react-native-keychain
react-native-share
react-native-randombytes
react-native-vector-icons

Libraries that did not support auto-linking

Some libraries couldn't be unlinked because they did not support auto-linking. Furthermore, in order for some of them to work, changes needed to be made on each library.

More specifically on iOS:

react-native-aes-crypto. See changes here.
react-native-os. See changes here.
react-native-search-api. See changes here.
react-native-tcp See changes here.
react-native-scrollable-tab-view. The fix for this library was actually bumping the version, since the new version supported auto-linking and it seemed to already integrate our custom changes. Version bumped from our custom version to original v1.0.0

and on Android:

lottie-react-native
react-native-gesture-handler

Patches removed

react-native+0.59.10.patch. Since a new version of React Native was installed, this patch did not make sense anymore. No patch for the new version was necessary, at least for successfully compiling and running the app.

Removed assets

Removed fonts: Because of the auto-linking features, all the fonts that were manually added to Xcode from react-native-vector-icons were removed. The auto-linking should add those fonts automatically now.

Errors fixed

After the app compiled successfully, there were some errors encountered when running the app. More specifically:

App crashed on Android on any screen with our WebView. After some research, the fix was modifying our custom WebView.
App crashed when closing the side nav drawer on iOS release. The fix was adding import 'react-native-gesture-handler'; to the index.js file. See here.
Metro bundler did not work with the new React Native version. The metro version needed to be upgraded from 0.55.0 to 0.59.0.

Warnings fixed

Animated: 'useNativeDriver' was not specified. This is a required 'option and must be explicitly set to true or false. This warning was fixed by upgrading react-native-modal from 9.0.0 to 11.5.6.
Animated.event now requires a second argument for options. This warning was fixed by upgrading react-native-modal from 9.0.0 to 11.5.6.
Calling getNode() on the ref of an Animated component is no longer necessary. Since a lot of libraries were causing this warning like react-native-scrollable-tab-view, react-native-keyboard-aware-scroll-view and react-native-tab-view, this warning was added to the ignore list. We should address this by upgrading the libraries mentioned.
componentWillUpdate deprecated. Since this appears on a lot of our components, this warning was added to the ignore list. Check this for possible fix.
componentWillReceiveProps deprecated. Since this appears on a lot of our components, this warning was added to the ignore list. Check this for possible fix.

Lint

The lint plugin we used called eslint-config-react-native no longer worked correctly with the new React Native version. The reccommended plugin now is @react-native-community/eslint-config. An effort was made so that our current rules remained the same. Because of this, there were lots of rules added so a new file called .eslintrc.js was created to accommodate all the rules. It's possible that this new config is more strict, so we may still change the config file to our liking.

Issue

Resolves #1201

estebanmino · 2020-05-18T18:36:55Z

.gitignore

@@ -69,3 +70,6 @@ coverage

 # editor
 .vscode
+
+# CocoaPods
+/ios/Pods/


I see a lot of No new line ... warnings

Thanks I will fix that!

estebanmino · 2020-05-18T18:37:46Z

android/app/build.gradle

+            storeFile file('debug.keystore')
+            storePassword 'android'
+            keyAlias 'androiddebugkey'
+            keyPassword 'android'


are we adding new keys?

do we need keys for debug?

Yes if you start a new RN project those are there now. Take a look at this:
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.62.1

estebanmino · 2020-05-18T18:41:03Z

android/app/src/main/AndroidManifest.xml

@@ -36,7 +36,7 @@
 			android:launchMode="singleTask"
 			android:name=".MainActivity"
 			android:label="@string/app_name"
-			android:configChanges="keyboard|keyboardHidden|orientation|screenSize"
+			android:configChanges="keyboard|keyboardHidden|orientation|screenSize|uiMode"


estebanmino · 2020-05-18T18:44:10Z

android/keystores/debug.keystore.properties

-key.store=debug.keystore
-key.alias=androiddebugkey
-key.store.password=android
-key.alias.password=android


I made a comment before about this, so you passed these vars to that file?

I followed this:
https://react-native-community.github.io/upgrade-helper/?from=0.59.10&to=0.62.1

So, new RN projects now are like that

estebanmino · 2020-05-18T18:46:07Z

index.js

+	"Cannot read property 'hash' of null",
+	'componentWillUpdate',
+	'componentWillReceiveProps',
+	'getNode()'


I saw your comment about this on the PR description. I tried to find where we were using these on our side as you said and I couldn't... this seems to come from our deps right?

The componentWillUpdate and componentWillReceiveProps are from our code (and probably from libs). We need to use hooks. The other ones are from libs yes.

hm strange I'm searching on vscode on the project and I can't find these. But anyways I agree about hooks

Oh you are right! I thought we were using those, because the project started before they were deprecated. Nevermind then, they are from libs!

ibrahimtaveras00

Following issues are happening only on Android

Issue 1:

Detox e2e isn't able to run on debug mode, nor release mode

Issue 2:

Assets are not showing up on wallet view; nor the option to add tokens and/or collectibles

Issue 3:

Similar to issue 2, when you are on a dapp and initiate a txn, the confirm view is missing both details and data

You can reproduce by going to https://metamask.github.io/test-dap and tapping on deploy contract after connecting

Issue 4:

Upon going to drawer > settings > Security & Privacy and then scrolling down to both SHOW PRIVATE KEY and REVEAL SEED PHRASE; once tapping those buttons and entering password, you're unable to see anything be displayed.

andrepimenta · 2020-05-21T16:14:38Z

@ibrahimtaveras00 Those issues should be fixed now!

ibrahimtaveras00

Fixes look good, QA Passed 👍

estebanmino

approving now, waiting for your decision on d111f27

* all files changed * init Pods * pods working * app running * fixed native and metro problems * fix pods deployment target * android running but with RN errors * viewpager working * fixed our custom webview * Fix RCTAnalytics * lint [IMPORTANT] * fix warnings & gesture handler * update scritps and readme * fix most tests * tests working * fix TouchableHighlight tests * rebased * return to correct pod platform version * Upgrade to RN 0.62.2 * Fix detox * fix format * fix metro server start * fix bitcode error * fix e2e release ios * returning to previous deployment target * detox bump * fix yarn clean * Create react-native-scrollable-tab-view+1.0.0.patch * Back to 11.0 deployment target and fix sentry properties * Allow more memory for daemon * Fixed end of file warnings * remove extra space * Fix for e2e tests * Ignore VirtualizedLists warning

Since this is still calling scripts/cocoapods/flipper.rb: Setting React-RCTAppDelegate > config.build_settings > FB_SONARKIT_ENABLED=1 Causing React/AppSetup/RCTAppSetupUtils.mm to wrongly try importing FlipperKit Introduced in RN v0.62 - https://github.com/facebook/react-native/blob/v0.62.0/template/ios/Podfile - #1586 We should have removed in RN v0.64 - https://github.com/facebook/react-native/blob/v0.64.0/template/ios/Podfile - https://github.com/facebook/react-native/blob/v0.63.5/template/ios/Podfile

Should have been added only as debug (not release) config in our RN 0.62.2 upg: #1586 Ref: https://github.com/facebook/react-native/blob/v0.62.0/template/ios/HelloWorld.xcodeproj/project.pbxproj#L500

Should have been added as an Apple Clang preprocessor macro (not compiler flag) in our RN 0.62.2 upg: #1586 Refactor in xcodeproj target - from: Apple Clang - Custom Compiler Flags > Other C Flags - to: Apple Clang - Preprocessing > Preprocessor Macros Ref: https://github.com/facebook/react-native/blob/v0.62.0/template/ios/HelloWorld.xcodeproj/project.pbxproj#L500 Nb: rn-tester sticking to "-DFB_SONARKIT_ENABLED=1" for OTHER_CFLAGS and OTHER_SWIFT_FLAGS

Originally added in our RN 0.62.2 upg: #1586 Should have been removed since RN 0.63.0: https://github.com/facebook/react-native/blob/v0.63.0/template/ios/HelloWorld.xcodeproj/project.pbxproj Ref: facebook/react-native#28796

## **Description** There is a bug with the RPC url not being displayed correctly when trying to add a network via a dapp. This raises a security concern because the user can potentially add a malicious network if a network RPC URL is not shown. Furthermore, the height of the network added sheet extends further than that of prod. ## **Related issues** Fixes: [#1586 ](MetaMask/mobile-planning#1586) ## **Manual testing steps** 1. Given I am on the browser view 2. And I connect my wallet to chainlist.wtf 3. When I add "Avalanche" 4. Then the add network sheet is displayed 5. But the RPC URL is not displayed correctly ## **Screenshots/Recordings**  ### **Before**  <img width="370" alt="before-bug" src="https://github.com/MetaMask/metamask-mobile/assets/26223211/32c69155-458e-4d88-bc27-d4a175827b59"> ### **After**  <img width="382" alt="fix-bug" src="https://github.com/MetaMask/metamask-mobile/assets/26223211/04b9b02d-d5de-4dad-8bd5-8e937045e74f"> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

andrepimenta added 17 commits May 16, 2020 17:06

all files changed

483c9dc

init Pods

2cf16e9

pods working

ce7791d

app running

9893fbc

fixed native and metro problems

61ee331

fix pods deployment target

adbd785

android running but with RN errors

a52d8f3

viewpager working

872d0a7

fixed our custom webview

c64e59a

Fix RCTAnalytics

b4609c3

lint [IMPORTANT]

be47c7a

fix warnings & gesture handler

3e1b7ff

update scritps and readme

c8b47f9

fix most tests

a4a3920

tests working

e4820e5

fix TouchableHighlight tests

a7f9d54

rebased

0d36222

andrepimenta added needs-qa Any New Features that needs a full manual QA prior to being added to a release. next up prioritized to be picked up next labels May 18, 2020

andrepimenta mentioned this pull request May 18, 2020

React native upgrade 0.62.2 hermes rebased #1587

Closed

return to correct pod platform version

d80e5b3

estebanmino reviewed May 18, 2020

View reviewed changes

Upgrade to RN 0.62.2

88efcb4

andrepimenta changed the title ~~React native upgrade 0.62.1 rebased~~ React native upgrade 0.62.2 rebased May 18, 2020

andrepimenta added 2 commits May 18, 2020 23:19

Fix detox

9576ca2

fix format

dc08b85

andrepimenta added 2 commits May 20, 2020 20:06

returning to previous deployment target

d111f27

detox bump

0bb849d

ibrahimtaveras00 suggested changes May 21, 2020

View reviewed changes

andrepimenta added 2 commits May 21, 2020 13:41

fix yarn clean

46cfc3a

Create react-native-scrollable-tab-view+1.0.0.patch

dcbe6e7

ibrahimtaveras00 approved these changes May 21, 2020

View reviewed changes

ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels May 21, 2020

estebanmino approved these changes May 21, 2020

View reviewed changes

Back to 11.0 deployment target and fix sentry properties

f5e18d3

andrepimenta requested a review from a team as a code owner May 22, 2020 15:34

andrepimenta added 6 commits May 22, 2020 17:23

Allow more memory for daemon

a518263

Fixed end of file warnings

89dcf6d

remove extra space

0e9d979

Fix for e2e tests

35bda39

Ignore VirtualizedLists warning

7f8916c

Merge branch 'develop' into react-native-upgrade-0.62.1-rebased

51c265f

andrepimenta merged commit e0df5aa into develop May 23, 2020

andrepimenta deleted the react-native-upgrade-0.62.1-rebased branch May 23, 2020 15:19

whymarrh mentioned this pull request Aug 10, 2020

Remove unused metamask-mobile-provider dependency #1763

Merged

leotm mentioned this pull request Oct 16, 2023

feat: React Native 0.71.14 with Xcode 15 stable #7114

Merged

21 tasks

salimtb mentioned this pull request Mar 7, 2024

fix: fix url bug display #8877

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React native upgrade 0.62.2 rebased #1586

React native upgrade 0.62.2 rebased #1586

andrepimenta commented May 18, 2020 •

edited

Loading

estebanmino May 18, 2020

andrepimenta May 18, 2020

estebanmino May 18, 2020

estebanmino May 18, 2020

andrepimenta May 18, 2020

estebanmino May 18, 2020

estebanmino May 18, 2020

andrepimenta May 18, 2020

estebanmino May 18, 2020

andrepimenta May 18, 2020

estebanmino May 18, 2020

andrepimenta May 18, 2020

ibrahimtaveras00 left a comment •

edited

Loading

andrepimenta commented May 21, 2020

ibrahimtaveras00 left a comment

estebanmino left a comment

React native upgrade 0.62.2 rebased #1586

React native upgrade 0.62.2 rebased #1586

Conversation

andrepimenta commented May 18, 2020 • edited Loading

General file changes

Library changes

Libraries that did not support auto-linking

Patches removed

Removed assets

Errors fixed

Warnings fixed

Lint

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibrahimtaveras00 left a comment • edited Loading

Choose a reason for hiding this comment

andrepimenta commented May 21, 2020

ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

estebanmino left a comment

Choose a reason for hiding this comment

andrepimenta commented May 18, 2020 •

edited

Loading

ibrahimtaveras00 left a comment •

edited

Loading