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

chore(wallet): Upgrade RN to 0.72.3 #1167

Merged
merged 22 commits into from
Nov 8, 2023
Merged

chore(wallet): Upgrade RN to 0.72.3 #1167

merged 22 commits into from
Nov 8, 2023

Conversation

coreyphillips
Copy link
Collaborator

@coreyphillips coreyphillips commented Jul 19, 2023

Description

This PR ensures we're on top of RN upgrades and changes so it doesn't pile up too much down the road. This PR should also improve readability of error messages within the app when crashes occur and help with troubleshooting.

  • Upgrades react-native to version 0.72.3.
  • Upgrades other dependencies as needed.
  • Applies patch to react-native-camera-kit.

Type of change

  • Dependency Upgrade

Tests

  • No test

Upgrades react-native to version 0.72.3.
Upgrades other dependencies as needed.
Applies patch to react-native-camera-kit.
@coreyphillips coreyphillips requested a review from pwltr July 19, 2023 17:27
@pwltr
Copy link
Collaborator

pwltr commented Jul 20, 2023

Slashtags seems to be broken on this build:

Simulator Screenshot - iPhone 14 - 2023-07-20 at 21 13 26
Simulator Screenshot - iPhone 14 - 2023-07-20 at 21 11 26

@coreyphillips
Copy link
Collaborator Author

Thanks! Able to reproduce. Looking into it.

@pwltr
Copy link
Collaborator

pwltr commented Jul 20, 2023

The error looks like it has to do with async generator code which I believe hyperswarm doesn't support. Think we had a polyfill for that once maybe that got removed or is now needed again? Maybe @limpbrains knows more.

@coreyphillips
Copy link
Collaborator Author

coreyphillips commented Jul 21, 2023

@pwltr, I made the mistake of regenerating the yarn.lock file. As such, it bumped some of the slashtag-sdk dependency versions to something incompatible. Updated it here. Should be working as expected now.

@limpbrains
Copy link
Collaborator

@pwltr, I made the mistake of regenerating the yarn.lock file. As such, it bumped some of the slashtag-sdk dependency versions to something incompatible. Updated it here. Should be working as expected now.

this is why I'm for pinning all deps in package.json. Of course it doesn't solve all the issues

@pwltr
Copy link
Collaborator

pwltr commented Jul 22, 2023

Any idea why e2e tests are failing? It looks like some async tasks are taking way longer in this build in the detox environment. I can make the tests pass locally by increasing the timeouts but on CI it's still not enough it seems.

@coreyphillips
Copy link
Collaborator Author

Any idea why e2e tests are failing? It looks like some async tasks are taking way longer in this build in the detox environment. I can make the tests pass locally by increasing the timeouts but on CI it's still not enough it seems.

Same. I'm at a loss as to why this is failing.

@pwltr
Copy link
Collaborator

pwltr commented Jul 24, 2023

Any idea why e2e tests are failing? It looks like some async tasks are taking way longer in this build in the detox environment. I can make the tests pass locally by increasing the timeouts but on CI it's still not enough it seems.

Same. I'm at a loss as to why this is failing.

Detox currently doesn't support this version of react-native so probably we will have to wait for that. I don't think it's worth breaking e2e tests unless there is something major in this upgrade that we need?

@coreyphillips
Copy link
Collaborator Author

Sounds good. Lets wait. One of the added benefits of the upgrade is the potential for slightly improved error messages for troubleshooting.

@limpbrains limpbrains reopened this Jul 26, 2023
@limpbrains limpbrains marked this pull request as draft July 26, 2023 11:45
@coreyphillips coreyphillips removed the request for review from pwltr November 3, 2023 13:05
@limpbrains
Copy link
Collaborator

image

@limpbrains
Copy link
Collaborator

limpbrains commented Nov 7, 2023

Before merging I think we need to test it ourselfs, especially android
Also, please, sqash it

@coreyphillips
Copy link
Collaborator Author

Agreed. I'll run through some tests on Android in the morning.

@coreyphillips
Copy link
Collaborator Author

Android build is working as expected. No hiccups thus far. Seems we've lost the e2e-ios test again? Not sure what happened there.

@limpbrains
Copy link
Collaborator

I think we can ignore it for now, I will try to fix it in another PR

@coreyphillips coreyphillips marked this pull request as ready for review November 8, 2023 15:06
@limpbrains limpbrains merged commit e8f0491 into master Nov 8, 2023
3 of 4 checks passed
@coreyphillips coreyphillips deleted the rn-upgrade-72 branch November 8, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants