-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix(crashlytics): Expo SDK 44 compatibility #5947
Conversation
This pull request is being automatically deployed with Vercel (learn more). react-native-firebase – ./🔍 Inspect: https://vercel.com/invertase/react-native-firebase/ANGdEpY2vMM2ESKtV12vv8agbkMz react-native-firebase-next – ./website_modular🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/Bs2fYLAXjVCsy5XTnHTE41xZLreX [Deployment for 504b758 canceled] |
Codecov Report
@@ Coverage Diff @@
## main #5947 +/- ##
=========================================
Coverage 53.04% 53.04%
Complexity 622 622
=========================================
Files 208 208
Lines 10171 10171
Branches 1618 1618
=========================================
Hits 5394 5394
Misses 4523 4523
Partials 254 254 |
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.
Interesting! Looks obviously correct, following the other react imports, and CI will be a clear signal for it. Thanks @Yonom !
Storage.getDownkoadUrl is our last serious flaky test in iOS. Unrelated to this PR if course. Rerun in progress |
Awesome, thank you for the quick review I can definitely tell that the CI is less flaky now fyi, on a previous attempt, the Android e2e test also failed on this PR: https://github.com/invertase/react-native-firebase/runs/4568217848?check_suite_focus=true |
Yes, I slowly catalog and squish CI flakiness, and it slowly creeps back in. Now that the disastrous iOS+Detox+firebase-performance crash is fixed iOS has a storage downloadURL issue, both occasionally have an auth beforeAll issue and there are a couple less frequent ones. Bit by bit - I was completely absorbed with the Detox fix from a flakiness perspective so the others sat. I'll get them all eventually :-) #4058 |
Description
Using
crashlytics
in a Expo SDK 44 (latest version) project fails. This is because expo patches the react podspec for swift compatibility reasons (expo/expo#15299) and that causes old style imports to not work.I fixed the issue by changing the import style.
More information: expo/expo#15622 (comment)
Related issues
Release Summary
fix(crashlytics): Expo SDK 44 compatibility
Checklist
Android
iOS
e2e
tests added or updated inpackages/\*\*/e2e
jest
tests added or updated inpackages/\*\*/__tests__
Test Plan
Tested by adding crashlytics to a brand new expo sdk 44 project and compiling the app using
expo run:ios
Think
react-native-firebase
is great? Please consider supporting the project with any of the below:React Native Firebase
andInvertase
on Twitter