-
Notifications
You must be signed in to change notification settings - Fork 594
chore(deps): bump react-native 0.70.13 #9189
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Niiiiiiice 🔥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woohoo, excited to test it out
Android release builds are broken in Android because of the Sentry: getsentry/sentry-java#2601 |
Created a ticket for upgrading sentry here -> https://artsyproduct.atlassian.net/browse/PHIRE-207 |
efb0dc3
to
b6af64d
Compare
state: billingAddress?.state, | ||
postalCode: billingAddress?.postalCode, | ||
country: billingAddress?.country.shortName, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So a major react native upgrade, a major sentry upgrade and you got us off a forked dependency? Legend 💪
Awesome work.
I am wondering if it might make sense to separate out the stripe change into a separate pr similar to the sentry change just to keep the separation clean in case something goes wrong. Otherwise this looks great.
Thanks for doing this!
I wondered that about Stripe too, but tipisi-stripe was breaking Android(during build time) with some weird Java stuff, then the upgrade seemed like a quick win given no breaking changes. |
yeah I hear that not saying not to do the stripe change, just if possible put it into a separate PR and merge first, that way if there is breakage with credit cards or something down the line it is easy to isolate |
Actually, I can do it that way, not sure if kotlin bump would work with the old React native version but I can try separating it for a revert in case of issues. |
Amazing @araujobarret!! |
This reverts commit 9152b92.
b6af64d
to
516b6ca
Compare
@brainbicycle I was working on retiring tipsi in another branch but found a blocker when replacing it with official stripe library(stripe/stripe-react-native#1044). The solution was to rever tipisi commit here and add jetifier to the install script to fix android -> androidx packaging translation from tipsi, with that the builds are 100% fine. Going to generate the betas to test one more time and if everything is fine merge this PR. |
Description
This PR bumps react-native to 0.70.13. One blocker I've found for Android was the outdated tipsi-stripe library,
I replaced it with the official library making the adjustments and adjusting the teststhe solution was to addjetifier
to the install script as we do in the android build script.This PR is divided into 3 commits:
reverted on 50fba80tipsi-stripe
replacement with@stripe/react-native
1.8.0
.Dimensions.removeEventListener
with the new format, this was generating some errors and breaking the testsPR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.