-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Crash with expo custom build (49.x or 50.x, or 51.x) when newArchEnabled set to true on iOS and with expo-dev-client installed #5497
Comments
Native crash report here:
|
Also tried with the 3.7.x version (same issue) and 3.5.4 (does not compile) |
Hi @sync, thanks for submitting this issue! I investigated it and it turns out there's some problem with how React Native's bridge is handled by/with Expo. I forwarded the issue to them. |
@tjzel thanks for this, the issue: expo/expo#25968 |
Hi @tjzel I believe this is actually a race condition happening while the bridge is being invalidated, I can reproduce this same issue in a project without expo by reloading the app by double pressing "r" Screen.Recording.2023-12-20.at.10.38.08.mov |
Indeed doing something like this "fixes" the issue (see attached patch). Obviously not a great patch.
|
but then reanimated animations will crash with a null JSI assertion, so yes definitely not a fix :-) |
Not sure if it is exactly the same issue, or just reproduction steps are the same but I am getting crashes on reloads as well. No expo, RCT_NEW_ARCH_ENABLED=1. Sometimes it does not crash right away and I need to reload more times, so indeed looks like a race condition "react-native": "0.73.1" My crash logs looks a little bit different:
So this in particular looks interesting (Thread 11, where it crashed)
|
Any news here? Should we open an issue for react native perhaps? |
now that expo 50.x is out i'll try again to reproduce the issue and update this ticket, react-native version has also changed to 0.73.2 |
I reproduced with react-native 0.73.2 without expo and new architecture enabled |
I added a repository on how to reproduce the issue in latest RN 0.73.3 and Reanimated 3.6.2 In my case I have the crash when using
|
updated sample reproduction repo with latest expo and reanimated 3.7.0, crashes like before, haven't had a chance to have a deeper look so far |
I've also run into this issue, it's killed my Testflight, I'm looking for a temp solution while this is getting fixed. This works locally just fine for me in my simulator though. Running expo 50, reanimated 3.7.0 is there a downgrading solution. i'm a little perplexed. |
Update: |
@tjzel tested with latest version of expo 51 beta with Version of reanimated:
cc @brentvatne |
Can also confirm that reanimated is fine with latest react-native 0.74 if barebone (without expo), so something inside expo is triggering the issue.
|
If I remove the |
@brentvatne how could i get someone at expo to look at this issue ? thank you and let me know if I can help |
@sync - i tried this on a new project with sdk 51 beta and it works well for me on android, but had the same issue as you on ios: https://github.com/brentvatne/mittaz. @gabrieldonadel will investigate! |
I suspect the issue might be caused by using |
Yeah, the problem is the DevMenu bridge, which gets initialized along with the main app. I just tested an using |
I can confirm that this is fixing the issue indeed! Thank you guys! |
Are we sure about this? I can boot up the app now (which is definitely an improvement), but after a couple of hot reload it is crashing again.
|
@sync It seems to be a different issue, probably on the Expo side this time – we'll investigate this. |
…5953) ## Summary As I pointed out [here](#5901 (comment)), using `[RCTBridge currentBridge]` may lead to the wrong bridge in apps that have multiple React instances, as is the case for expo-dev-client. https://github.com/software-mansion/react-native-reanimated/blob/7e1a19ed8190763456e96939eec71c2528c1eed4/apple/REAModule.mm#L284 Fixes #5497 Instead of using `[RCTBridge currentBridge]` we can just call `self.bridge` and ensure the correct bridge is always used ## Test plan Run FabricExample app on iOS Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
Summary: Based on the discussion starting here: https://discord.com/channels/514829729862516747/1073566663825432587/1237407161991172157, I suggest moving the call to `_notifyEventDispatcherObserversOfEvent_DEPRECATED` straight to `RCTEventDispatcher.mm`. It was previously in `RCTInstance.mm` which is only relevant on bridgeless mode. We want to mimic the behavior of https://github.com/facebook/react-native/blob/06eea61c19cd730cf0c14a436f042d30791c3f4a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm#L75-L78 but without using `currentBridge` since it is considered bad practice: software-mansion/react-native-reanimated#5497 (comment). ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [CHANGED] - Move `notifyObservers` straight to `RCTEventDispatcher.mm`. Pull Request resolved: facebook#44474 Test Plan: See that example with `stickyHeaders` still works correctly on both bridgeless and bridge mode. Videos with it on https://github.com/facebook/react-native/blob/deee037c62a7d62a349d34db427b14d3560ddf83/packages/rn-tester/js/examples/FlatList/FlatList-stickyHeaders.js example with more items for visibility: - bridgeless: https://github.com/facebook/react-native/assets/32481228/8b78104a-226b-466a-9f32-60ba4ec14100 - bridge: https://github.com/facebook/react-native/assets/32481228/f2ca67cb-578f-45d4-954f-3249c6fa9410 - old arch: https://github.com/facebook/react-native/assets/32481228/7d642923-ddda-4dd3-8f14-c9982a03bc2e Differential Revision: D57097880 Reviewed By: javache Pulled By: cipolleschi
Summary: Based on the discussion starting here: https://discord.com/channels/514829729862516747/1073566663825432587/1237407161991172157, I suggest moving the call to `_notifyEventDispatcherObserversOfEvent_DEPRECATED` straight to `RCTEventDispatcher.mm`. It was previously in `RCTInstance.mm` which is only relevant on bridgeless mode. We want to mimic the behavior of https://github.com/facebook/react-native/blob/06eea61c19cd730cf0c14a436f042d30791c3f4a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm#L75-L78 but without using `currentBridge` since it is considered bad practice: software-mansion/react-native-reanimated#5497 (comment). ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [CHANGED] - Move `notifyObservers` straight to `RCTEventDispatcher.mm`. Pull Request resolved: #44474 Test Plan: See that example with `stickyHeaders` still works correctly on both bridgeless and bridge mode. Videos with it on https://github.com/facebook/react-native/blob/deee037c62a7d62a349d34db427b14d3560ddf83/packages/rn-tester/js/examples/FlatList/FlatList-stickyHeaders.js example with more items for visibility: - bridgeless: https://github.com/facebook/react-native/assets/32481228/8b78104a-226b-466a-9f32-60ba4ec14100 - bridge: https://github.com/facebook/react-native/assets/32481228/f2ca67cb-578f-45d4-954f-3249c6fa9410 - old arch: https://github.com/facebook/react-native/assets/32481228/7d642923-ddda-4dd3-8f14-c9982a03bc2e Reviewed By: javache Differential Revision: D57097880 Pulled By: cipolleschi fbshipit-source-id: de1504e90529fe4f001f44f02ace329386cf7727
Summary: Based on the discussion starting here: https://discord.com/channels/514829729862516747/1073566663825432587/1237407161991172157, I suggest moving the call to `_notifyEventDispatcherObserversOfEvent_DEPRECATED` straight to `RCTEventDispatcher.mm`. It was previously in `RCTInstance.mm` which is only relevant on bridgeless mode. We want to mimic the behavior of https://github.com/facebook/react-native/blob/06eea61c19cd730cf0c14a436f042d30791c3f4a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm#L75-L78 but without using `currentBridge` since it is considered bad practice: software-mansion/react-native-reanimated#5497 (comment). ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [CHANGED] - Move `notifyObservers` straight to `RCTEventDispatcher.mm`. Pull Request resolved: facebook#44474 Test Plan: See that example with `stickyHeaders` still works correctly on both bridgeless and bridge mode. Videos with it on https://github.com/facebook/react-native/blob/deee037c62a7d62a349d34db427b14d3560ddf83/packages/rn-tester/js/examples/FlatList/FlatList-stickyHeaders.js example with more items for visibility: - bridgeless: https://github.com/facebook/react-native/assets/32481228/8b78104a-226b-466a-9f32-60ba4ec14100 - bridge: https://github.com/facebook/react-native/assets/32481228/f2ca67cb-578f-45d4-954f-3249c6fa9410 - old arch: https://github.com/facebook/react-native/assets/32481228/7d642923-ddda-4dd3-8f14-c9982a03bc2e Reviewed By: javache Differential Revision: D57097880 Pulled By: cipolleschi fbshipit-source-id: de1504e90529fe4f001f44f02ace329386cf7727
Facing this issue again without expo - new architecture is TRUE and bridgeless is FALSE in my project |
Description
When reloading (using
r
) and sometimes when starting the app, the app crash (on iOS, not Android) as soon as react-native-reanimated is installed, even if not used inside app, with the newArchEnabled.As soon as your remove
react-native-reanimted
andexpo prebuild --clean
andyarn ios
the app will no longer crash on boot or reload.Just an fyi, you no longer need to add 'react-native-reanimated/plugin' in your babel.config.js with expo 50 as per their latest blog post.
Steps to reproduce
Snack or a link to a repository
https://github.com/sync/new-arch-crash
Reanimated version
3.6.1
React Native version
0.73.0
Platforms
iOS
JavaScript runtime
Hermes
Workflow
Expo Dev Client
Architecture
Fabric (New Architecture)
Build type
Debug app & dev bundle
Device
iOS simulator
Device model
iPhone 15 Pro
Acknowledgements
Yes
The text was updated successfully, but these errors were encountered: