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 react-native to 0.75.3 #21268

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

siddarthkay
Copy link
Contributor

@siddarthkay siddarthkay commented Sep 15, 2024

Summary

This PR upgraded react-native to v0.75.3
Following changes were also made :

  • NDK upgraded to 26.1.10909125
  • @react-native/gradle-plugin is set up using includeBuild gradle mechanism which is a better way.
  • reactjs is upgraded to 18.3.1
  • react-native-config is upgraded to 1.5.3
  • react-native-navigation is upgraded to 7.40.1
  • react-native-reanimated is upgraded to 3.15.2
  • patches for react-native-navigation are updated. PR in RNN repo -> fix(Android): Kotlin build issues with RN 0.75.3 wix/react-native-navigation#7913
  • patched autolinking behaviour in react-native to prevent generating autolinking.json in a gradle step. We run gradle in a sandbox with --offline so this is not feasible for us. We do the same thing via make generate-autolink-android in CI and this is also part of scripts/build-android.sh when make run-android is called.
  • AGP is bumped to 8.5.0 and aapt2 is bumped to 8.5.0-11315950
  • we no longer need to patch out -X from script_phases-patched.sh
  • onboarding buttons now need rn/pressible because rn/touchable-without-feedback breaks the rules of react.
  • gradle deps were updated.
  • pods were updated.

Testing notes

Intense testing is required.

Platforms

  • Android
  • iOS

Side Effects

  • react-native-hole-view usage seemed broken in some areas, @flexsurfer suggested we can remove this lib.
  • styles of a few buttons seemed different
  • component tests are breaking due to some parts of UI being unresponsive

status: ready

@siddarthkay siddarthkay self-assigned this Sep 15, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Sep 15, 2024

Jenkins Builds

Click to see older builds (57)
Commit #️⃣ Finished (UTC) Duration Platform Result
42a6095 #1 2024-09-15 15:29:20 ~10 min tests 📄log
42a6095 #1 2024-09-15 15:29:24 ~10 min android-e2e 📄log
42a6095 #1 2024-09-15 15:29:33 ~10 min android 📄log
42a6095 #1 2024-09-15 15:33:30 ~14 min ios 📄log
24ef306 #2 2024-09-15 17:30:12 ~10 min android 📄log
24ef306 #2 2024-09-15 17:30:14 ~10 min android-e2e 📄log
24ef306 #2 2024-09-15 17:30:14 ~10 min tests 📄log
✔️ 24ef306 #2 2024-09-15 17:35:08 ~15 min ios 📱ipa 📲
✔️ 6537008 #3 2024-09-16 03:58:44 ~14 min ios 📱ipa 📲
6537008 #3 2024-09-16 04:07:09 ~22 min android 📄log
6537008 #3 2024-09-16 04:13:42 ~29 min android-e2e 📄log
91bef1c #4 2024-09-17 05:11:18 ~4 min tests 📄log
91bef1c #4 2024-09-17 05:11:42 ~4 min android-e2e 📄log
91bef1c #4 2024-09-17 05:11:44 ~4 min android 📄log
169d36e #5 2024-09-17 05:22:23 ~2 min tests 📄log
169d36e #5 2024-09-17 05:22:48 ~3 min android-e2e 📄log
169d36e #5 2024-09-17 05:22:54 ~3 min android 📄log
✔️ 169d36e #5 2024-09-17 05:32:10 ~12 min ios 📱ipa 📲
9539861 #6 2024-09-17 13:52:16 ~4 min android 📄log
9539861 #6 2024-09-17 13:53:45 ~6 min android-e2e 📄log
9539861 #6 2024-09-17 13:56:20 ~8 min tests 📄log
0f31894 #7 2024-09-17 14:06:51 ~2 min android-e2e 📄log
0f31894 #7 2024-09-17 14:08:00 ~3 min tests 📄log
0f31894 #7 2024-09-17 14:09:14 ~4 min android 📄log
✔️ 0f31894 #7 2024-09-17 14:19:32 ~14 min ios 📱ipa 📲
d81a2fd #8 2024-09-18 02:33:58 ~11 sec android-e2e 📄log
d81a2fd #8 2024-09-18 02:33:58 ~11 sec android 📄log
6a2ebae #9 2024-09-18 02:36:11 ~14 sec android-e2e 📄log
8abe656 #10 2024-09-18 02:41:35 ~3 min tests 📄log
✔️ 8abe656 #10 2024-09-18 02:46:40 ~8 min android-e2e 🤖apk 📲
✔️ 8abe656 #10 2024-09-18 02:46:50 ~8 min android 🤖apk 📲
✔️ 8abe656 #10 2024-09-18 02:48:44 ~10 min ios 📱ipa 📲
8d9c04d #12 2024-09-18 03:07:18 ~4 min tests 📄log
✔️ 8d9c04d #12 2024-09-18 03:12:13 ~9 min android 🤖apk 📲
✔️ 8d9c04d #12 2024-09-18 03:12:32 ~9 min android-e2e 🤖apk 📲
0dced72 #13 2024-09-18 03:17:56 ~4 min tests 📄log
✔️ 0dced72 #13 2024-09-18 03:20:28 ~6 min android 🤖apk 📲
✔️ 0dced72 #13 2024-09-18 03:22:56 ~9 min android-e2e 🤖apk 📲
96d667e #14 2024-09-18 03:33:33 ~2 min tests 📄log
✔️ 96d667e #14 2024-09-18 03:39:16 ~8 min android-e2e 🤖apk 📲
✔️ 96d667e #14 2024-09-18 03:39:42 ~8 min android 🤖apk 📲
✔️ 96d667e #14 2024-09-18 03:40:41 ~9 min ios 📱ipa 📲
e469409 #15 2024-09-18 05:13:30 ~3 min tests 📄log
✔️ e469409 #15 2024-09-18 05:19:32 ~9 min android-e2e 🤖apk 📲
✔️ e469409 #15 2024-09-18 05:19:53 ~9 min android 🤖apk 📲
✔️ e469409 #15 2024-09-18 05:27:54 ~17 min ios 📱ipa 📲
e469409 #16 2024-09-20 09:35:23 ~3 min tests 📄log
✔️ e469409 #16 2024-09-20 09:40:28 ~8 min android-e2e 🤖apk 📲
✔️ e469409 #16 2024-09-20 09:40:38 ~8 min android 🤖apk 📲
✔️ e469409 #16 2024-09-20 09:42:32 ~10 min ios 📱ipa 📲
307b76d #17 2024-09-22 13:32:18 ~3 min tests 📄log
307b76d #17 2024-09-22 13:32:31 ~3 min android 📄log
✔️ 307b76d #17 2024-09-22 13:35:40 ~6 min android-e2e 🤖apk 📲
953c927 #18 2024-09-22 13:41:19 ~4 min tests 📄log
✔️ 953c927 #18 2024-09-22 13:46:54 ~10 min android-e2e 🤖apk 📲
✔️ 953c927 #18 2024-09-22 13:48:19 ~11 min android 🤖apk 📲
✔️ 953c927 #18 2024-09-22 13:52:19 ~15 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 0070bfc #20 2024-10-01 07:51:03 ~6 min android-e2e 🤖apk 📲
✔️ 0070bfc #20 2024-10-01 07:52:58 ~8 min android 🤖apk 📲
✔️ 0070bfc #20 2024-10-01 07:53:51 ~9 min ios 📱ipa 📲
0070bfc #20 2024-10-01 07:55:01 ~10 min tests 📄log
898c6c4 #21 2024-10-03 12:16:52 ~3 min tests 📄log
✔️ 898c6c4 #21 2024-10-03 12:21:34 ~7 min android-e2e 🤖apk 📲
✔️ 898c6c4 #21 2024-10-03 12:23:10 ~9 min android 🤖apk 📲
✔️ 898c6c4 #21 2024-10-03 12:26:07 ~12 min ios 📱ipa 📲

@siddarthkay siddarthkay force-pushed the upgrade-react-native-to-0-75-x branch 2 times, most recently from 169d36e to 9539861 Compare September 17, 2024 13:47
@siddarthkay siddarthkay force-pushed the upgrade-react-native-to-0-75-x branch 2 times, most recently from 6a2ebae to 8abe656 Compare September 18, 2024 02:38
org.junit:junit-bom:5.9.3
org.junit:junit-bom:5.9.2
org.codehaus.mojo:animal-sniffer-annotations:1.23
com.android.tools.lint:lint-gradle:31.5.0' \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shit is tragic and unsustainable...

scripts/build-android.sh Outdated Show resolved Hide resolved
@@ -72,5 +100,7 @@ else
nixOpts+=("--option" "build-use-sandbox" "true")
fi

# needed since react-native v0.75
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a better comment

@siddarthkay siddarthkay requested review from a team September 18, 2024 05:00
@siddarthkay siddarthkay force-pushed the use-nixpkgs-unstable-gradle-8-8 branch 5 times, most recently from e64d894 to 3c5b671 Compare September 19, 2024 15:26
Base automatically changed from use-nixpkgs-unstable-gradle-8-8 to develop September 20, 2024 09:31
@@ -476,6 +478,10 @@ android-install: export BUILD_TYPE ?= release
android-install: ##@other Install APK on device using adb
adb install result/app-$(BUILD_TYPE).apk

generate-autolink-android: export TARGET := clojure
generate-autolink-android: ##@other Generate autolinking.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if it's required for Android why is it not a dependency in the Makefile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only need this in Jenkins file for Android build step, and this step runs one of the npm libraries react-native-config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually making this a dependency for android would be a good idea. I will try it out locally.

org.junit:junit-bom:5.9.3
org.junit:junit-bom:5.9.2
org.codehaus.mojo:animal-sniffer-annotations:1.23
com.android.tools.lint:lint-gradle:31.5.0' \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shit is tragic and unsustainable...

react-native config > "$AUTOLINKING_DIR/autolinking.json"
echo "Generated autolinking.json"

ROOT_VALUE=$(jq -r '.root' "$AUTOLINKING_DIR/autolinking.json")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a single comment explaining what this is and why we need to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that autolinking.json generated contains a full static path to the dependencies in node_modules and we face issues when android nix derivation tries to access those paths.

In this script i take the root key which is present in this json and which also contains the path to my project. similar to $GIT_ROOT and I replace that with ".." to make sure that the path becomes a relative path to node_modules directory.

I'll add a comment and explain why I had to do this.

@@ -72,5 +72,7 @@ else
nixOpts+=("--option" "build-use-sandbox" "true")
fi

# needed since react-native v0.75
source "${GIT_ROOT}/scripts/generate_autolink_android.sh"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you sourcing it if you don't use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad this maybe confusing, The thing is that generate_autolink_android.sh has a function named generate_android_autolink() and that function is called at the end of the script.
So sourcing it also ensures that the required function is executed.

@@ -49,6 +49,7 @@ pipeline {
script {
utils.symlinkEnv()
println("Build Number: ${utils.genBuildNumber()}")
sh 'make generate-autolink-android'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing this here? It seems kinda awkward do do this as part of Makefile if it's always necessary for Android.

Why can't this be part of the Gradle build process for example? Is there ever a case where it don't need to run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do this in 2 places, here for PR builds and inside scripts/build-android.sh for Debug builds which are run when we do make run-android.

The autolinking json file was introduced originally as a gradle build step by react-native team.
However the issue with that is this autolinking stuff runs npx @react-native-community/cli config at runtime and we can't run this inside our nix derivation since that runs in a sandbox environment.

Hence I have made these changes.
I have also patched out this behaviour from react-native to make sure it works well with our build process.

The second question about is there ever a case where it doesn't need to be run, well we only need the autolinking json file right before we have to do an android build. Otherwise we don't need it.

@siddarthkay siddarthkay force-pushed the upgrade-react-native-to-0-75-x branch 2 times, most recently from 4a62604 to 0070bfc Compare October 1, 2024 07:44
@siddarthkay
Copy link
Contributor Author

Converting this PR to draft mode till the following things happen :

  • react-native-navigation is swapped with react-navigation library.
  • react-native-hole-view usage is replaced with an appropriate fallback.

cc @flexsurfer @ilmotta

@siddarthkay siddarthkay marked this pull request as draft October 1, 2024 08:07
@flexsurfer
Copy link
Member

btw, i've looked to the code regarding react-native-navigation is swapped with react-navigation library, and seems like we're good to go, with no blockers. and should be straightforward to swap, but it might take some time though

@ilmotta
Copy link
Contributor

ilmotta commented Oct 1, 2024

btw, i've looked to the code regarding react-native-navigation is swapped with react-navigation library, and seems like we're good to go, with no blockers. and should be straightforward to swap, but it might take some time though

@flexsurfer @siddarthkay is it really a must that we migrate to react-navigation to unblock this PR? Or is it more like a really nice to have?

@siddarthkay
Copy link
Contributor Author

@flexsurfer : would you mind creating an issue for the migration? Thank you.
@ilmotta : I would say it makes more sense to merge this PR after the navigation migration is done, because without it, we have weird navigation related bugs already which might not even pass the QA stage :D

@siddarthkay siddarthkay force-pushed the upgrade-react-native-to-0-75-x branch from 0070bfc to 898c6c4 Compare October 3, 2024 12:13
@ilmotta
Copy link
Contributor

ilmotta commented Oct 3, 2024

I would say it makes more sense to merge this PR after the navigation migration is done, because without it, we have weird navigation related bugs already which might not even pass the QA stage :D

@siddarthkay, you know I'm all in for keeping software up-to-date, but there's one contention point. At the moment, we are committed to deliver a continuous stream of work for Keycard because we have a soft deadline coming up.

To give a bit more context, all other devs are allocated to important tasks and we anticipate some hiccups as usual in releases, therefore there's no other dev that can work in the Keycard stream while @flexsurfer works on the migration. Just a matter of capacity.

@flexsurfer it would be helpful to know a rough, but realistic estimate of the time it would take to migrate to react-navigation if you did the migration yourself so that we can plan accordingly.

What I think we should do is migrate and upgrade after 2.31 is out in a few weeks because that will give us plenty of time to test and fix possible regressions. 2.32 won't take too long to be published after 2.31, but we will have a nice & safe window of time. This is more or less how I envision all risky upgrades, never too close to releases, ideally soon after a release.

Does this plan make sense to everybody? cc @churik

@siddarthkay
Copy link
Contributor Author

yeah I don't mind waiting till after 2.31, this upgrade is not urgent.

@churik churik added the blocked label Oct 7, 2024
@churik
Copy link
Member

churik commented Oct 7, 2024

blocked until 2.31 is cut

@status-im-auto
Copy link
Member

14% of end-end tests have passed

Total executed tests: 7
Failed tests: 6
Expected to fail tests: 0
Passed tests: 1
IDs of failed tests: 727230,703133,727231,702745,727229,702843 

Failed tests (6)

Click to expand
  • Rerun failed tests

  • Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230
    Test setup failed: critical/test_wallet.py:23: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:330: in create_shared_drivers
        raise e
    base_test_case.py:320: in create_shared_drivers
        test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1
     '_asyncio.Future' object has no attribute 'session_id'
    



    2. test_wallet_send_eth, id: 727229

    Test setup failed: critical/test_wallet.py:23: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:330: in create_shared_drivers
        raise e
    base_test_case.py:320: in create_shared_drivers
        test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1
     '_asyncio.Future' object has no attribute 'session_id'
    



    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Test setup failed: critical/chats/test_public_chat_browsing.py:311: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:330: in create_shared_drivers
        raise e
    base_test_case.py:320: in create_shared_drivers
        test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1
     '_asyncio.Future' object has no attribute 'session_id'
    



    Device sessions

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231

    Test setup failed: critical/test_wallet.py:200: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(1)
    base_test_case.py:330: in create_shared_drivers
        raise e
    base_test_case.py:320: in create_shared_drivers
        test_suite_data.current_test.testruns[-1].jobs[drivers[i].session_id] = i + 1
     '_asyncio.Future' object has no attribute 'session_id'
    



    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    Device 1: Find Button by accessibility id: show-profiles
    Device 1: Tap on found: Button

    critical/chats/test_public_chat_browsing.py:244: in test_restore_multiaccount_with_waku_backup_remove_switch
        self.errors.verify_no_errors()
    base_test_case.py:192: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     zQ3...dWXh5 was not restored as a contact from waku backup!
    E    zQ3...Vacac was not restored as a contact from waku backup!
    



    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745

    Device 1: Find EditBox by accessibility id: enter-contact-code-input
    Device 1: Type eth:arb1:oeth:0xbe365708296f75716cd93de484873f5b1ca50b1a to EditBox

    Test setup failed: critical/chats/test_1_1_public_chats.py:37: in prepare_devices
        self.home_1.add_contact(self.public_key_2)
    ../views/home_view.py:437: in add_contact
        chat.element_by_translation_id("user-found").wait_for_visibility_of_element()
    ../views/base_element.py:147: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 1: Button by xpath:`//*[@text="User found"]` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Passed tests (1)

    Click to expand

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    @siddarthkay
    Copy link
    Contributor Author

    @status-im/mobile-qa : pls ignore this E2E I was testing the new automation on this PR

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    Status: E2E Tests
    Development

    Successfully merging this pull request may close these issues.

    6 participants