Skip to content

Commit

Permalink
Make Fabric event receivers init only once (#6907)
Browse files Browse the repository at this point in the history
## Summary

Fixes
#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 (
    <View
      style={{ flex: 1 }}
      onLayout={(evt) => sW(evt.nativeEvent.layout.width)}>
      <Animated.ScrollView
        onScroll={scrollHandler}
        horizontal
        snapToInterval={w}
        decelerationRate="fast">
        {Array.from({ length: 20 })
          .fill(0)
          .map((_, i) => {
            return (
              <View key={i} style={{ width: w }}>
                <Text>{i}</Text>
              </View>
            );
          })}
      </Animated.ScrollView>
    </View>
  );
}

export default EmptyExample;

```
  • Loading branch information
szydlovsky authored Feb 4, 2025
1 parent e8e14f0 commit d3e6ff2
Showing 1 changed file with 14 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit d3e6ff2

Please sign in to comment.