(Android/ScrollView) Fix onMomentumScrollEnd being called multiple times

[AntoineDoubovetzky]

Summary

I noticed that onMomentumScrollEnd is called multiple times on Android.

1. It is always called three times with the last value
2. It is sometimes called many times befire the scrolling stops when the pagingEnabled prop is true

See:

I used the following code to get the logs:

import React from 'react';
import {SafeAreaView, ScrollView, Text, View} from 'react-native';

const App = () => {
  const onMomentumScrollEnd = ({nativeEvent}) => {
    console.log(
      'onMomentumScrollEnd',
      nativeEvent.contentOffset.x,
      nativeEvent.contentOffset.y,
    );
  };

  const onMomentumScrollBegin = ({nativeEvent}) => {
    console.log(
      'onMomentumScrollBegin',
      nativeEvent.contentOffset.x,
      nativeEvent.contentOffset.y,
    );
  };

  return (
    <SafeAreaView>
      <ScrollView
        horizontal
        pagingEnabled
        onMomentumScrollEnd={onMomentumScrollEnd}
        onMomentumScrollBegin={onMomentumScrollBegin}>
        {new Array(10).fill(0).map((_, index) => {
          return (
            <View
              style={{width: 400, height: 400, backgroundColor: 'red'}}
              key={index}>
              <Text>{index}</Text>
            </View>
          );
        })}
      </ScrollView>
    </SafeAreaView>
  );
};

export default App;

From what I understood:

1. We do not check that mStableFrames is >= 3 when emitting the event (line 798) and we keep executing the runnable, so it is emitted until mStableFrames equals 3. When mStableFrames equals 3 we stop executing the runnable (line 809). That's why it gets called 3 times.
2. When pagingEnabled is true, the postOnAnimationDelayed method is called twice (line 794 and line 806). I believe it causes the runnable to be executing too often, and the onMomentumScrollEnd event to be emitted too many times.

I updated the code so:

1. The event is emitted only once, and at the same time we stop executing the runnable
2. The postOnAnimationDelayed method is called at most once per execution of the runnable

Changelog

[Android] [Fixed] - Fix ScrollView's onMomentumScrollEnd being called multiple times on Android

Test Plan

I tested using the code above with every combination of horizontal and pagingEnabled values.

[pull-bot]

PR build artifact for af85c606467a9963e4c5b544c4903c47eb28f485 is ready. To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,820,441	-43
android	hermes	armeabi-v7a	7,214,323	-49
android	hermes	x86	8,134,152	-52
android	hermes	x86_64	8,112,257	-52
android	jsc	arm64-v8a	9,697,923	-80
android	jsc	armeabi-v7a	8,454,039	-88
android	jsc	x86	9,649,713	-87
android	jsc	x86_64	10,247,934	-89

Base commit: 9923ac1b524ae959abdf50a28a3094198015f77e Branch: main

[pull-bot]

PR build artifact for 4f3bdbdc448be5f65c082fd8b27631d71abda24a is ready. To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: d31d83f4109c167ec612058c805fd65f69b82476 Branch: main

[zhiqingchen]

+1

[anton-patrushev]

Any chance it would be merged? The issue is still reproducible on the latest 0.66.4 version. It should be a pretty critical bug. By the way, thanks to @AntoineDoubovetzky for the provided fix.

[anton-patrushev]

Any chance it would be merged? The issue is still reproducible on the latest 0.66.4 version. It should be a pretty critical bug. By the way, thanks to @AntoineDoubovetzky for the provided fix.

Wanna note that this fix is not working for me on the 0.66.4 version 🤷‍♂️

[AntoineDoubovetzky]

@anton-patrushev I'll try to find time to look into it this week

[AntoineDoubovetzky]

I just rebased on main and it is still working for me using the RNTester app. @anton-patrushev How did you apply the fix exactly ?

[anton-patrushev]

I just rebased on main and it is still working for me using the RNTester app. @anton-patrushev How did you apply the fix exactly ?

@AntoineDoubovetzky This is my patch with a workaround on this issue. Here is a gist.

I'm still using RN 66.4, so maybe it's already acceptable on the latest RN version, but I didn't have a chance to verify it. I will be back with the updated results after upgrading to the newer RN versions if this issue would be still opened. But not sure if it would be soon :)

[AntoineDoubovetzky]

I believe you can't simply patch react-native to fix native android code, you have to build it from source (https://github.com/facebook/react-native/wiki/Building-from-source)

[AntoineDoubovetzky]

@ryancat I see you are the last one who worked on ReactScrollView / ReactHorizontalScrollView. Do you have any changes to request ?

[ryancat]

Thanks for the fix, @AntoineDoubovetzky!

Your fix makes sense and cleans up the existing logic a lot. I am aware that this is one of the pain points people have with moment end events:

Stackoverflow: https://stackoverflow.com/questions/53408470/flatlist-onendreached-being-called-multiple-times Github issues: #32696

The only concern I have is if there are people who actually depend on the moment end call to trigger multiple times, through the scroll gliding phase. I'll discuss with our team and prioritize to review & merge this.

[facebook-github-bot]

@ryancat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[github-actions[bot]]

This pull request was successfully merged by @AntoineDoubovetzky in 06668fcbacd750771f1d53cce829dc55e86f3f3c.

^{When will my fix make it into a release? | Upcoming Releases}

[gaearon]

There's a similar problem for onScrollBeginDrag (which gets called twice). Is that also fixed by this PR?