From d3e6ff2a1cf033373ace5afb3b39b9fbb19913d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Szyd=C5=82owski?= <9szydlowski9@gmail.com> Date: Tue, 4 Feb 2025 11:42:01 +0100 Subject: [PATCH] Make Fabric event receivers init only once (#6907) ## Summary Fixes https://github.com/software-mansion/react-native-reanimated/issues/6896 When adding support for fabric, we have inadvertently kept previous listeners still active for fabric. This way, we registered twice with the same `NodesManager` for React Native's `FabricEventDispatcher`, resulting in pretty much all the events doubling in number on android. I added a simple check that takes care of it. ## Test plan Paste the following into `EmptyExample` and run `FabricExample`. Check logs making sure that both `start` and `end` are logged only once per gesture. ```TSX import React, { useState } from 'react'; import { View, Text } from 'react-native'; import Animated, { useAnimatedScrollHandler } from 'react-native-reanimated'; function EmptyExample() { const [w, sW] = useState(0); const scrollHandler = useAnimatedScrollHandler({ onBeginDrag() { console.log('start'); }, onMomentumEnd() { console.log('end'); }, }); return ( sW(evt.nativeEvent.layout.width)}> {Array.from({ length: 20 }) .fill(0) .map((_, i) => { return ( {i} ); })} ); } export default EmptyExample; ``` --- .../swmansion/reanimated/NodesManager.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java b/packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java index 56e9637dc78..dcdcd05f200 100644 --- a/packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java +++ b/packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java @@ -187,18 +187,20 @@ protected void doFrameGuarded(long frameTimeNanos) { } }; - // We register as event listener at the end, because we pass `this` and we haven't finished - // constructing an object yet. - // This lead to a crash described in - // https://github.com/software-mansion/react-native-reanimated/issues/604 which was caused by - // Nodes Manager being constructed on UI thread and registering for events. - // Events are handled in the native modules thread in the `onEventDispatch()` method. - // This method indirectly uses `mChoreographerCallback` which was created after event - // registration, creating race condition - EventDispatcher eventDispatcher = - Objects.requireNonNull(UIManagerHelper.getEventDispatcher(context, uiManagerType)); - eventDispatcher.addListener(this); - mUnsubscribe = () -> eventDispatcher.removeListener(this); + if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) { + // We register as event listener at the end, because we pass `this` and we haven't finished + // constructing an object yet. + // This lead to a crash described in + // https://github.com/software-mansion/react-native-reanimated/issues/604 which was caused by + // Nodes Manager being constructed on UI thread and registering for events. + // Events are handled in the native modules thread in the `onEventDispatch()` method. + // This method indirectly uses `mChoreographerCallback` which was created after event + // registration, creating race condition + EventDispatcher eventDispatcher = + Objects.requireNonNull(UIManagerHelper.getEventDispatcher(context, uiManagerType)); + eventDispatcher.addListener(this); + mUnsubscribe = () -> eventDispatcher.removeListener(this); + } mAnimationManager = new AnimationsManager(mContext, mUIManager); }