Open ddiachkov opened 2 months ago
Upon further investigation, I've found that the bug is not reproducible on 0.74 (or at least not as easy).
I've added minimal repro for 0.73. I think this corner case (mishandling events) is valid.
Hi @ddiachkov! Seems like you got down to the bottom of the problem 😄 . I encourage you to create a PR with your fix proposal - then the team will be able to see themselves whether it risks any bugs or not. When you've created the PR, make sure to link it to this issue
Description
We have an app that has a react-navigation entering animation for one of the screens. This screen has a fullscreen animation (Rive) playing in the background. Recently we've moved to the new arch and discovered that if you click on the animation it will crash the app because Reanimated tries to consume an event that was never meant to be consumed by it.
Stack Trace
Root Cause
Reanimated installs custom
EventDispatcherListener
to listen to animation events, but it fails to validate that the incoming event does belong to the node it animates. That causes double consumption ofWritableMap
and app crash.If you look at vanilla
NativeAnimatedNodesManager
, you can see that it callsevent.getEventAnimationDriverMatchSpec()
andmatchSpec.match(driver.mViewTag, driver.mEventName)
to filter the event.NodesManager
does something similar, but only when the event is dispatched from non-UI thread.Potential Fix
I moved the check to the top of the
handleEvent
:This seems to fix the issue. However, I am not 100% sure it won't cause some other bugs.
Steps to reproduce
<Stack.Screen options={{ animation: "slide_from_right" }} />
Snack or a link to a repository
https://github.com/ddiachkov/reanimated-0.73-crash-repro
Reanimated version
3.11.0
React Native version
0.73.5
Platforms
Android
JavaScript runtime
Hermes
Workflow
React Native
Architecture
Fabric (New Architecture)
Build type
Release app & production bundle
Device
Real device
Device model
No response
Acknowledgements
Yes