-
-
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
Make Fabric event receivers init only once #6907
Conversation
...ges/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java
Outdated
Show resolved
Hide resolved
...ges/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NodesManager.java
Show resolved
Hide resolved
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.
Code looks good to me, haven't tested it though, let's wait for approvals from other reviewers
Just had that issue pop up on android, so I'm happy for that PR to exist, keeping an eye on it |
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.
I've tested it and it seems to work well 👍
## 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; ```
## 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; ```
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'sFabricEventDispatcher
, 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 runFabricExample
. Check logs making sure that bothstart
andend
are logged only once per gesture.