Research into upgrading react-native

siddarthkay commented 1 year ago

Rn Upgrade helper is here : https://react-native-community.github.io/upgrade-helper/ Current version of React Native is 0.63.5 Attempt to upgrade to 0.68.5 Wish to ultimately reach 0.70.x (So far it's only possible to aim for 0.68.5)

Rough todos below :

[x] for android : Figure out the change here https://github.com/status-im/status-mobile/blob/develop/nix/pkgs.nix
[x] Some research on latest version that clojurescript officially supports ( I believe latest versions of React Native require us to switch to latest versions of Reactjs as well )

Attempt and log findings on upgrading IOS while ignoring Android

[x] update dependencies and dev dependencies in package.json
[x] update metro config
[x] update ios/~/Info.plist
[x] update ios/~/AppDelegate.m
[x] update ios/~xcworkspace/contents.xcworkspacedata
[x] update ios/~.xcodeproj/xcshareddata/xcschemes/RnDiffApp.xcscheme
[x] update ios/~.xcodeproj/project.pbxproj
[x] update ios/Podfile
[x] update .flowconfig

- Android Related Stuff

[x] update android/gradle.properties
[x] update android/build.gradle
[x] update android/app/src/main/res/values/styles.xml
[x] update android/app/src/main/AndroidManifest.xml
[x] update android/app/src/debug/AndroidManifest.xml
[x] update android/app/build.gradle

Take care of Libraries specially modified to work with Older Versions of React Native

[x] react-native-hole-view
[x] react-native-mail

siddarthkay commented 1 year ago

We use Reagent for Reactjs and Reagent does not support Reactjs version 18 yet https://github.com/reagent-project/reagent/pull/554 So the furthest we should aim at the moment would be to upgrade React Native which uses Reactjs 17 and that would be React Native 0.68.5

flexsurfer commented 1 year ago

RNN https://github.com/wix/react-native-navigation/issues/7466

siddarthkay commented 1 year ago

When attempting to upgrade react-native to 0.65 I currently run into this error while running make run-ios

Please file a bug at https://feedbackassistant.apple.com with this warning message and any useful information you can provide.
** BUILD FAILED **

The following build commands failed:
    PhaseScriptExecution [CP-User]\ Generate\ Specs /Users/siddarthkumar/Library/Developer/Xcode/DerivedData/StatusIm-fxiitjbbybrcsqcxqartjfyvqxdo/Build/Intermediates.noindex/Pods.build/Debug-iphonesimulator/FBReactNativeSpec.build/Script-C58A79D8656A804AAC57ACF6A48D5309.sh 
    (in target 'FBReactNativeSpec' from project 'Pods')
(1 failure)

info Run CLI with --verbose flag for more details.
make: *** [run-ios] Error 1

Investigating further.

siddarthkay commented 1 year ago

On opening XCode there is a more detailed version of the error and it is


Error: Could not determine react-native-codegen location. Try running 'yarn install' or 'npm install' in your project root.
Command PhaseScriptExecution failed with a nonzero exit code

trying out possible solutions outlined here : https://stackoverflow.com/questions/66805844/xcode-12-4-react-native-build-failed-in-ios-showing-all-messages-command-phasesc

flexsurfer commented 1 year ago

just wondering if it does make sense to upgrade gradually, i think in each steps there will be lots of issues, just related to the version, so it's better to start upgrading to the latest version imho

flexsurfer commented 1 year ago

or, i would probably try to create a new project, with our setup and then upgrade the entire project from scratch, because we're working on 2.0 and we'll have lots of testing, so it's a good opportunity to make structure and RN project from scratch, wdyt ?

siddarthkay commented 1 year ago

just wondering if it does make sense to upgrade gradually, i think in each steps there will be lots of issues, just related to the version, so it's better to start upgrading to the latest version imho

Hmm the latest we could go so far would be 0.67. i am currently researching to see what is the least version we can reach without issues. All upgrades will have issues eventually but I wanted to establish some baseline. Also the issues we face in minor upgrades will also exist in major ones so I feel it's better to take on smaller issues one by one..

siddarthkay commented 1 year ago

or, i would probably try to create a new project, with our setup and then upgrade the entire project from scratch, because we're working on 2.0 and we'll have lots of testing, so it's a good opportunity to make structure and RN project from scratch, wdyt ?

Well that was my initial thought and it would be a better way to upgrade if you could set up a brand new React native app with v0.67 I was under the impression that there were some challenges in the way the existing project is set up, but if you could get this up and running that would be awesome!

flexsurfer commented 1 year ago

https://www.metosin.fi/blog/reagent-towards-react-18/

siddarthkay commented 1 year ago

Got the IOS app to run successfully on major react-native version upgrade of v0.64.4

Screenshot 2022-12-09 at 12 31 46 AM

small wins 🥳

siddarthkay commented 1 year ago

One benefit I am seeing right away is that copy-pasting now works on IOS simulators, I don't know if its because the simulator is running IOS 16.1 or because we upgraded to react-native 0.64.4 Either way I am happy that this functionality works.

https://user-images.githubusercontent.com/64726664/206558215-ec80adf9-3fa4-44c1-8f7c-ddcc5fa853fb.mov

siddarthkay commented 1 year ago

successfully upgraded IOS App to v0.65.3 , I am on a roll today 😎

Screenshot 2022-12-09 at 2 22 44 AM

jakubgs commented 1 year ago

Nice!

flexsurfer commented 1 year ago

One benefit I am seeing right away is that copy-pasting now works on IOS simulators, I don't know if its because the simulator is running IOS 16.1 or because we upgraded to react-native 0.64.4 Either way I am happy that this functionality works.

Screen.Recording.2022-12-09.at.1.42.36.AM.mov

One benefit I am seeing right away is that copy-pasting now works on IOS simulators, I don't know if its because the simulator is running IOS 16.1 or because we upgraded to react-native 0.64.4 Either way I am happy that this functionality works.

Screen.Recording.2022-12-09.at.1.42.36.AM.mov

it's simulator bug, so it works already in develop with IOS 16.1

siddarthkay commented 1 year ago

It is now no surprise that the IOS app works with v0.66.5 as well, After this I will attempt to go to v0.67.5 and if things go well I will then focus on gradually upgrading Android side as well.

Reason for stopping at v0.67.5 is that the next version of react-native 0.68.x does not work well with our navigation library : react-native-navigation. We can not go beyond v0.68.5 because reagent does not support react 18 yet.

Will update this thread with more info.

siddarthkay commented 1 year ago

react-native upgraded to v0.67.5, will now focus on android side.

flexsurfer commented 1 year ago

It is now no surprise that the IOS app works with v0.66.5 as well, After this I will attempt to go to v0.67.5 and if things go well I will then focus on gradually upgrading Android side as well.

Reason for stopping at v0.67.5 is that the next version of react-native 0.68.x does not work well with our navigation library : react-native-navigation. We can not go beyond v0.68.5 because reagent does not support react 18 yet.

Will update this thread with more info.

both libraries work with latest RN and react versions

flexsurfer commented 1 year ago

so we should go to latest version

siddarthkay commented 1 year ago

both libraries work with latest RN and react versions

I'll upgrade further and find out if navigation works 👍🏻

siddarthkay commented 1 year ago

Moving to v0.68.x requires adding AppDelegate.mm and removing AppDelegate.m which is going to take a while. ref : https://react-native-community.github.io/upgrade-helper/?from=0.67.5&to=0.68.5

siddarthkay commented 1 year ago

AppDelegate.mm is similar to AppDelegate.m, but it includes additional code that allows the file to be compiled as a mixed-language file. This means that the file can contain both Objective-C and C++ code, which can be useful in certain situations where it is necessary to use C++ libraries or other C++ code in an Objective-C project. The extra "m" in the file extension simply indicates that the file contains mixed-language code.

flexsurfer commented 1 year ago

yes its needed for new Architecture and JSI

siddarthkay commented 1 year ago

Moving to v0.68.x requires adding AppDelegate.mm and removing AppDelegate.m which is going to take a while. ref : https://react-native-community.github.io/upgrade-helper/?from=0.67.5&to=0.68.5

I figured this out. ios App now works fine with react-native v0.68.5

siddarthkay commented 1 year ago

I will now focus on getting Android codebase gradually to v0.68.5

siddarthkay commented 1 year ago

Got the Android to react-native v0.64.4 without any changes to gradle or android folder which is very surprising, will continue to investigate further :)

https://react-native-community.github.io/upgrade-helper/?from=0.63.5&to=0.64.4

Screenshot 2023-01-05 at 8 06 48 AM

siddarthkay commented 1 year ago

We use Reagent for Reactjs and Reagent does not support Reactjs version 18 yet reagent-project/reagent#554 So the furthest we should aim at the moment would be to upgrade React Native which uses Reactjs 17 and that would be React Native 0.68.5

so a user here claims that Reactjs version 18 worked with Reagent but they had few issues with live-reloading and other third party libraries : https://github.com/reagent-project/reagent/issues/579 I will be looking into this next week and upgrading the ios build to another major version of react native

flexsurfer commented 1 year ago

https://github.com/facebook/react-native/issues/30034#issuecomment-1374355184

siddarthkay commented 1 year ago

Sometimes this weird error comes up with 0.68.5, It then goes away after refreshing metro server and the app works fine, logging it here for now

***
NAME: "Invariant Violation"
MESSAGE: "StatusIm" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called.

invariant@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=im.status.ethereum.debug:11076:26
runApplication@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=im.status.ethereum.debug:78507:50
__callFunction@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=im.status.ethereum.debug:11627:36
@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=im.status.ethereum.debug:11396:31
__guard@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=im.status.ethereum.debug:11586:15
callFunctionReturnFlushedQueue@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=im.status.ethereum.debug:11395:21
callFunctionReturnFlushedQueue@[native code]

***
 ERROR  Invariant Violation: "StatusIm" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called.
 LOG  Running "login" with {"rootTag":21,"initialProps":{"componentId":"login"}}

siddarthkay commented 1 year ago

More on this , seems like react-native version 0.68.5 is not stable.. probably culprit is react-native-navigation But I will investigate further.

More logs on this at the gist here: https://gist.github.com/siddarthkay/013fac0b76391e7c885c2ed6ac3755cb

siddarthkay commented 1 year ago

perhaps upgrading react-native-navigation to version 7.28.0 might fix this

siddarthkay commented 1 year ago

This is my most recent pain with getting IOS to 0.68.5

Undefined symbols for architecture x86_64:
  "StatusgoImageServerTLSCert()", referenced from:
      +[StatusDownloaderOperation URLSession:task:didReceiveChallenge:completionHandler:] in AppDelegate.o
     (found _StatusgoImageServerTLSCert in /Users/siddarthkumar/Library/Developer/Xcode/DerivedData/StatusIm-fxiitjbbybrcsqcxqartjfyvqxdo/Build/Products/Debug-iphonesimulator/Statusgo.framework/Statusgo(000005.o), declaration possibly missing extern "C")
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

link to AppDelegate.mm I'm using -> https://github.com/status-im/status-mobile/blob/d81a1a2980babc6fdfe5dedb8ea9cda1bdc08ffd/ios/StatusIm/AppDelegate.mm

siddarthkay commented 1 year ago

The symbol exists here : modules/react-native-status/ios/RCTStatus/Statusgo.xcframework/ios-arm64_x86_64-simulator/Statusgo.framework/Headers/Statusgo.objc.h

it looks like this

FOUNDATION_EXPORT NSString* _Nonnull StatusgoImageServerTLSCert(void);

All of this looks normal, don't know why if its present in the header file, its absent in the object file :(

One hypothesis is that : The issue maybe related to the change in the file extension from .m to .mm. The .mm extension stands for "Objective-C++" file, which means that the file will be compiled as a C++ file instead of a C file.

There could be differences in the way C++ and C handle function names, particularly with respect to name mangling.

"Name mangling" is used by C++ compilers to make function names unique across multiple translation units. The C++ compiler will add additional characters to the function name to make it unique, which can probably cause issues when linking against C-based libraries that don't use name mangling.

So this probably wasn't as straightforward as I thought, I will experiment more and find out

flexsurfer commented 1 year ago

cc @bitgamma

bitgamma commented 1 year ago

@siddarthkay try to change this https://github.com/status-im/status-mobile/blob/d81a1a2980babc6fdfe5dedb8ea9cda1bdc08ffd/ios/StatusIm/AppDelegate.mm#L52

with extern "C" NSString* StatusgoImageServerTLSCert();

siddarthkay commented 1 year ago

@bitgamma : I'll try this again today, but as far as I can remember this change produced the same error when I was trying to fix it weeks ago..

siddarthkay commented 1 year ago

I fixed the issue with 0.68.5 https://github.com/status-im/status-mobile/blob/d81a1a2980babc6fdfe5dedb8ea9cda1bdc08ffd/ios/StatusIm/AppDelegate.mm#L98 was the culprit.

I am doing some tests but 0.68.5 seems to be stable 🎉

flexsurfer commented 1 year ago

hey @siddarthkay any ETA when we could have 0.68.5 merged ?

siddarthkay commented 1 year ago

hey @siddarthkay any ETA when we could have 0.68.5 merged ?

Was focusing past few weeks on RC-1 and currently on 1 task that is in RC-2, I do plan to spend time today to upgrade Gradle version and move android to 0.68.5

I can estimate that there will be a PR ready for QA by end of march month this year 😁 to review. And we may have to spend sometime in April to fix potential issues that may arise.

siddarthkay commented 1 year ago

Got Gradle version up to v6.9 and resolved the issues that came with it..

siddarthkay commented 1 year ago

So this is the latest error while upgrading to react-native 0.65.3 on Android

To fix this we may have to update OkHttp Android Library, ref : https://github.com/square/retrofit/issues/3058

aaand the Yak Shaving continues ...

siddarthkay commented 1 year ago

hmmm, this makes sense : https://github.com/facebook/react-native/issues/31382 , it seems to be a known issue

siddarthkay commented 1 year ago

upgrading to okHttp 4.9.1 fixes this issue 🥳 So react-native on android is now working for version 0.65.3

Screenshot 2023-02-24 at 2 29 18 PM

siddarthkay commented 1 year ago

smooth sailing so far and was able to upgrade android to react-native version 0.66.5 Screenshot 2023-02-24 at 3 09 51 PM

siddarthkay commented 1 year ago

Go the android side upgraded to react-native version 0.67.5 without upgrading gradle version to 7.2 as suggested in the upgrade helper. I am surprised by how everything works, but at the same moment I am happy that it works 😅

Screenshot 2023-02-24 at 6 23 23 PM

siddarthkay commented 1 year ago

on my way to react-native 0.68.x and the build fails when compiling react-native-reanimated with the following error message :

> Configure project :react-native-reanimated

****************************************************************************************

FAILURE: Build failed with an exception.

* Where:
Build file '/Users/siddarthkumar/code/experiments/status-mobile/node_modules/react-native-reanimated/android/build.gradle' line: 99

* What went wrong:
A problem occurred evaluating project ':react-native-reanimated'.
> No such property: minor for class: java.lang.String

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

After looking into the change logs of react-native-reanimated library it seems like we have to update it to version 2.6.0 ref : https://github.com/software-mansion/react-native-reanimated/releases/tag/2.6.0 Trying that out 🫣

flexsurfer commented 1 year ago

actually we would want to upgrade all react native libs to latest

siddarthkay commented 1 year ago

bryan-cranston-replace-the-bulb

siddarthkay commented 1 year ago

After upgrading the following :

Gradle from 6.9 -> 7.5
Gradle Plugin from 4.1.0 -> 7.3.0
OpenJDK8 -> OpenJDK11
buildToolsVersion 30.0 -> buildToolsVersion 31.0
react-native-reanimated library from 2.3.3 -> 2.11.0

We get this issue while cleaning up android and building it :

> Configure project :react-native-mail
WARNING:Using flatDir should be avoided because it doesn't support any meta-data formats.

FAILURE: Build failed with an exception.

* Where:
Build file '/Users/siddarthkumar/code/experiments/status-mobile/node_modules/react-native-mail/android/build.gradle' line: 4

* What went wrong:
A problem occurred evaluating project ':react-native-mail'.
> Unsupported value: 31. Format must be one of:
  - android-31
  - android-31-ext2
  - android-T
  - vendorName:addonName:31

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
See https://docs.gradle.org/7.5/userguide/command_line_interface.html#sec:command_line_warnings

Seems like some compatibility issue with react-native-mail I see we have a fork of the package here : https://github.com/status-im/react-native-mail but in package.json we do not use the fork. We could either update this fork and fix the Gradle issue over there and then use this forked package OR We could remove this library and use some more up-to-date and maintained alternative.. This option will involve some refactor in places where we share logs with QA team.

For time being I will attempt to remove react-native-mail package and comment out its imports and usage in our codebase to verify if we get a successful build or not 🤞🏻

siddarthkay commented 1 year ago

Okay so I do get a successful build after doing the following :

updating kotlinPluginVersion from 1.4.3 -> 1.6.20
adding a const in build.gradle for react-native-gesture-handler library to use kotlinPluginVersion
some temp hacks to add https://mvnrepository.com/artifact/com.github.gundy/semver4j to the nix-store
updating deps.json, deps.list and deps.urls by following instructions at https://github.com/status-im/status-mobile/tree/develop/nix/deps/gradle#known-issues, Big thanks to @jakubgs
upgrading react-native-gesture-handler from 2.5.0 -> 2.9.0
removing react-native-mail from the project.
removing react-native-hole-view from the project.

next steps :

modify forks of react-native-hole-view and react-native-mail to make them compatible with the upgraded Gradle and kotlin versions :)

After these tasks I believe we will have the android side working with react-native 0.68.5 😁 🫣 🤞🏻

status-im / status-mobile

Research into upgrading react-native #14386