Fix `onEndReached` bugs for FlatList #26444

ifsnow · 2019-09-15T14:39:47Z

Summary

Related to : #18887 #16067 #25656 #14015 (There're more issues.)

I'm sure FlatList's onEndReached doesn't work as expected. It's one of the headaches that hasn't been solved for quite some time. We had to make it work by modifying onEndReachedThreshold several times, but eventually it doesn't work efficiently.

Problems

Unnecessary calls on first render
In most cases, onEndReached is called one or more times after the initial rendering.
Inefficient calls during scrolling
If scrolling is fast, it will be called again after the onEndReached call.
Inefficient logic to check
There're some logic implementations that are not needed.

Solutions

Removed unnecessary calls on first render
Removed inefficient calls during scrolling
Improved and clarified logic to check

Changelog

[General] [Fixed] - Fix onEndReached bugs for FlatList

Test Plan

Test App : https://github.com/ifsnow/FlatListImprovementTest

The list has 10 items at the beginning.
When onEndReached is called, 10 items are added.

1. After the initial rendering

Current FlatList

OnEndReached is called twice in a short period and has 30 items.

Patched FlatList

OnEndReached is not called and has 10 items.

2. After scrolling to the 11th item

Current FlatList

OnEndReached is called twice in a short period, having 50 items.

Patched FlatList

OnEndReached is called twice when required, having 30 items.

roshangm1 · 2019-09-15T14:55:04Z

This is amazing!!

vonovak · 2019-09-16T22:57:31Z

some automated test would be highly desirable

ifsnow · 2019-09-17T05:31:50Z

@vonovak Thanks for your feedback. I added a test case to check if onEndReached is working properly. :)

Libraries/Lists/__tests__/VirtualizedList-test.js

ifsnow · 2019-09-20T15:10:06Z

https://github.com/ifsnow/react-native-infinite-flatlist-patch

Use the patch library above to try it out before this PR merges.

tanoabeleyra · 2019-09-21T14:25:56Z

Thanks!, I'm starting a project with RN and was going to add infinite scrolling today. I didn't know they were so many problems with it since so long ago.

Now I just hope the elevation bug gets fixed 🤞

Libraries/Lists/__tests__/VirtualizedList-test.js

vonovak · 2019-09-22T20:16:00Z

@ifsnow I left some comments that can improve the tests, but please note I'm not a person who can merge the PR, I'm just trying to make it look good.
btw you linked several bugs that you're trying to solve by the PR. If those bugs are reproducible by tests (I don't know) it would be nice to have the tests included (and mention in their description what bugs those tests are covering).

ifsnow · 2019-09-23T08:42:37Z

@vonovak I agree on your comments. I'm sure it can improve the tests, so I modified the test as you said. It's ok even if you don't have merging role. Thank you for your interest in my PR.

sahrens

I appreciate you trying to optimize this, and love how you beefed up the tests, but I don't think you're considering all possible use cases this needs to work for, especially cases with rows of unpredictable render heights and data changes (e.g. the same 3 items could be updated to include a piece of metadata that shrinks their height, which would no longer fill the screen and should trigger an onEndReached).

Do you mind providing more context on the exact cases you're seeing where onEndReached is called when it shouldn't be? As I commented inline, it sounds like you probably just need to tweak your configuration or adjust how you're thinking about optimal performance.

Libraries/Lists/VirtualizedList.js

Libraries/Lists/__tests__/VirtualizedList-test.js

ifsnow · 2019-10-02T05:25:50Z

@sahrens Thank you for trying to explain the shortcomings of my implementation.

I didn't think of a situation where the item layout would shrink dynamically. I've tried to improve the logic so that it works more clearly in the various cases you've mentioned. You can see how the improved logic works in the test app.

The first goal is to come up with an easy way to use infinite scrolling in FlatList without worrying about the suitable value of onEndReachedThreshold and the number of initial items.

Please let me know if there is a better approach or something I'm missing.

kristerkari · 2019-10-11T12:52:27Z

This looks very cool!

Does this PR also solve the issue whereonEndReached is not getting called consistently between Android and iOS?

I can test these changes in my project to see if it helps with having to use different onEndReachedThreshold value for Android and iOS.

ifsnow · 2019-10-11T15:10:36Z

@kristerkari I think this patch will work the same on both Android and iOS. Please test it on your project and let me know if you have any problems. Thanks.

ghost · 2019-10-20T23:34:29Z

We really need this merged, awesome work.

ifsnow · 2019-10-20T23:43:04Z

@emilioheinz You can use this improvement feature without merged RN. Check it out here https://github.com/ifsnow/react-native-infinite-flatlist-patch

luancurti · 2019-10-28T15:15:19Z

Any updates on this?

kristerkari · 2019-10-29T12:17:29Z

I did a quick test using patch-package on top of a 0.61.2 project.

It does not seem to fix the issue with onEndReachedThreshold: 0 + onEndReached working on iOS, but not getting called on Android. Of course there are plenty of more test cases in this issue that could be tested: #16067 (comment)

I'm currently using const onEndReachedThreshold = Platform.OS === "android" ? 0.1 : 0; as a workaround.

fabOnReact · 2020-07-14T08:39:41Z

use the package provided by the author

TNBoiBoi · 2021-01-05T23:20:23Z

Bump, we really need this.

vastamaki · 2021-06-14T15:58:06Z

Any updates on this? Would be nice to get this merged instead of using patch :)

lunaleaps

Thanks @ifsnow for the contribution!

Indeed this is an issue we've seen in internal usage of FlatList as well. I've been hesitant about pulling this change in because of lack of testing in our list components and any potential side effects this may have in our view logging. We are actively investing in this area but still have a ways to go.

In the meantime, for this fix, could we add an opt-in option to the fix so we can run experimentation around it? That'd help de-risk this change for us as we continue to improve testing and clarify expectations in VirtualizedList's behavior.

KingAmo · 2021-12-23T03:29:21Z

any news here?

dylmye · 2022-07-19T11:22:11Z

I've been discussing this ticket on the road to 0.7.0 thread, I'd love to pick this up.

I don't have much in the way of internal React / RN development but I reckon to resolve @lunaleaps request for an opt-in we would need a prop, right?

(Sorry for necro'ing the PR, by the way)

lunaleaps · 2022-07-25T23:12:26Z

@dylmye Sounds great! Let me know if you need any help

vksgautam1 · 2022-09-07T10:45:02Z

Any solution of this

dylmye · 2022-09-11T00:41:28Z

I didn't get round to this @vksgautam1 because I don't have write access to ifsnow's repo and (honestly) forgot about this. There's also conflicts that need to be fixed that I don't have the knowledge to fix.

adding logic from PR https://github.com/facebook/react-native/pull/26444/files#diff-7481ec8ed5532798b075df637e805a8e439807aa2ce671208c24068c286361e8L1374-R1413

- Import improvements from PR facebook#26444 _maybeCallOnEndReached: https://github.com/facebook/react-native/pull/26444/files#diff-7481ec8ed5532798b075df637e805a8e439807aa2ce671208c24068c286361e8L1374-R1413 https://github.com/facebook/react-native/blob/2d3f6ca9801ef92b446458a2efc795db4ec17021/Libraries/Lists/VirtualizedList.js#L1372-L1414 - Additional check _hasDoneFirstScroll for maybeCallOnEndReached (added in onScroll callback) - Add improved logic start/endPositionReached and isScrollingForward - Add threeshold instead of 2 - Use default threeshold 30 for iOS - Add other improvements from method _maybeCallOnEndReached

moving onEndReached logic for inverted infinite list with TalkBack enabled to rn-tester example. To add in upcoming PR Notes of tasks done with this and previous commits: - Import improvements from PR facebook#26444 _maybeCallOnEndReached: https://github.com/facebook/react-native/pull/26444/files#diff-7481ec8ed5532798b075df637e805a8e439807aa2ce671208c24068c286361e8L1374-R1413 https://github.com/facebook/react-native/blob/2d3f6ca9801ef92b446458a2efc795db4ec17021/Libraries/Lists/VirtualizedList.js#L1372-L1414 - Additional check _hasDoneFirstScroll for maybeCallOnEndReached (added in onScroll callback) - Add improved logic start/endPositionReached and isScrollingForward - Add threeshold instead of 2 - Use default threeshold 30 for iOS - Add other improvements from method _maybeCallOnEndReached - Complete improvements required for iOS and Android: - Test iOS solution onEndReached correctly works with an infinite list - Test Android Solution onEndReached correctly works and list starts from the end - Try to remove setTimeout and test if bug is reintroduced - On Android re-introduce java logic to start the list from the bottom - On iOS use initialScrollIndex={0} with inverted flatlist and VoiceOver - Code review: - Test setEnabledTalkbackCompatibleInvertedList - Remove check on lastItem - Remove setTimeout Try to add more of the logic from _maybeCallOnEndReached as it may be the cause of the issue _maybeOnCallReached could be causing the flatlist to scroll in the opposite direction later .. check if the callback is called after the scrollTo - Review Diff

github-actions · 2023-03-01T18:01:05Z

This PR is waiting for author's feedback since 24 days. Please provide the requested feedback or this will be closed in 7 days

github-actions · 2023-03-08T18:01:23Z

This PR was closed because the author hasn't provided the requested feedback after 7 days.

fabOnReact · 2024-01-25T02:09:23Z

https://github.com/ifsnow/react-native-infinite-flatlist-patch/tree/master/patches does not support rn versions > 0.68.
Do you still experience this issue? Should I work on this? Thanks

ifsnow/react-native-infinite-flatlist-patch#11

fabOnReact · 2024-01-26T09:23:10Z

anybody was able to reproduce this issue on 0.72 or 0.73? Thanks

fabOnReact · 2024-01-29T23:55:25Z

@dylmye @vksgautam1 @KingAmo @roshangm1 @tanoabeleyra @luancurti @nnnnnoel @ghasemikasra39 @vastamaki @dylmye @Smolyan91

anybody was able to reproduce this issue on 0.72 or 0.73? Thanks

Fix onEndReached for FlatList

65d32fa

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 15, 2019

react-native-bot added Component: FlatList Bug labels Sep 15, 2019

Improve logic more easily

6600cbb

vonovak reviewed Sep 18, 2019

View reviewed changes

Libraries/Lists/__tests__/VirtualizedList-test.js Outdated Show resolved Hide resolved

Add a test for onEndReached

4374497

ifsnow force-pushed the fix/flatlist-onendreached-bug branch from 8dfc262 to 4374497 Compare September 18, 2019 14:33

vonovak requested changes Sep 22, 2019

View reviewed changes

Libraries/Lists/__tests__/VirtualizedList-test.js Outdated Show resolved Hide resolved

Libraries/Lists/__tests__/VirtualizedList-test.js Outdated Show resolved Hide resolved

Libraries/Lists/__tests__/VirtualizedList-test.js Outdated Show resolved Hide resolved

Improve a test for onEndReached

5451f8e

cpojer requested a review from sahrens September 30, 2019 01:22

sahrens suggested changes Sep 30, 2019

View reviewed changes

Libraries/Lists/VirtualizedList.js Show resolved Hide resolved

Libraries/Lists/VirtualizedList.js Outdated Show resolved Hide resolved

Libraries/Lists/__tests__/VirtualizedList-test.js Show resolved Hide resolved

Libraries/Lists/__tests__/VirtualizedList-test.js Show resolved Hide resolved

ifsnow added 2 commits October 2, 2019 13:44

Modify logic to work more clearly

a4ab116

Improve testing to check for more cases

2d3f6ca

ifsnow requested a review from sahrens October 8, 2019 13:37

ghost approved these changes Oct 13, 2019

View reviewed changes

lunaleaps assigned lunaleaps and unassigned lunaleaps Jan 25, 2021

elicwhite added the Needs: React Native Team Attention label Jun 15, 2021

lunaleaps requested changes Jun 15, 2021

View reviewed changes

lunaleaps self-assigned this Jun 15, 2021

lunaleaps added the Needs: Author Feedback label Aug 2, 2021

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Aug 12, 2021

github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Mar 1, 2023

github-actions bot closed this Mar 8, 2023

fabOnReact mentioned this pull request Jan 26, 2024

RN 0.69 support ifsnow/react-native-infinite-flatlist-patch#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `onEndReached` bugs for FlatList #26444

Fix `onEndReached` bugs for FlatList #26444

ifsnow commented Sep 15, 2019 •

edited

Loading

roshangm1 commented Sep 15, 2019

vonovak commented Sep 16, 2019

ifsnow commented Sep 17, 2019

ifsnow commented Sep 20, 2019

tanoabeleyra commented Sep 21, 2019

vonovak commented Sep 22, 2019

ifsnow commented Sep 23, 2019

sahrens left a comment

ifsnow commented Oct 2, 2019

kristerkari commented Oct 11, 2019

ifsnow commented Oct 11, 2019

ghost commented Oct 20, 2019

ifsnow commented Oct 20, 2019

luancurti commented Oct 28, 2019

kristerkari commented Oct 29, 2019 •

edited

Loading

fabOnReact commented Jul 14, 2020

TNBoiBoi commented Jan 5, 2021

vastamaki commented Jun 14, 2021

lunaleaps left a comment •

edited

Loading

KingAmo commented Dec 23, 2021

dylmye commented Jul 19, 2022

lunaleaps commented Jul 25, 2022

vksgautam1 commented Sep 7, 2022

dylmye commented Sep 11, 2022

github-actions bot commented Mar 1, 2023

github-actions bot commented Mar 8, 2023

fabOnReact commented Jan 25, 2024

fabOnReact commented Jan 26, 2024

fabOnReact commented Jan 29, 2024

Fix onEndReached bugs for FlatList #26444

Fix onEndReached bugs for FlatList #26444

Conversation

ifsnow commented Sep 15, 2019 • edited Loading

Summary

Problems

Solutions

Changelog

Test Plan

1. After the initial rendering

2. After scrolling to the 11th item

roshangm1 commented Sep 15, 2019

vonovak commented Sep 16, 2019

ifsnow commented Sep 17, 2019

ifsnow commented Sep 20, 2019

tanoabeleyra commented Sep 21, 2019

vonovak commented Sep 22, 2019

ifsnow commented Sep 23, 2019

sahrens left a comment

Choose a reason for hiding this comment

ifsnow commented Oct 2, 2019

kristerkari commented Oct 11, 2019

ifsnow commented Oct 11, 2019

ghost commented Oct 20, 2019

ifsnow commented Oct 20, 2019

luancurti commented Oct 28, 2019

kristerkari commented Oct 29, 2019 • edited Loading

fabOnReact commented Jul 14, 2020

TNBoiBoi commented Jan 5, 2021

vastamaki commented Jun 14, 2021

lunaleaps left a comment • edited Loading

Choose a reason for hiding this comment

KingAmo commented Dec 23, 2021

dylmye commented Jul 19, 2022

lunaleaps commented Jul 25, 2022

vksgautam1 commented Sep 7, 2022

dylmye commented Sep 11, 2022

github-actions bot commented Mar 1, 2023

github-actions bot commented Mar 8, 2023

fabOnReact commented Jan 25, 2024

fabOnReact commented Jan 26, 2024

fabOnReact commented Jan 29, 2024

Fix `onEndReached` bugs for FlatList #26444

Fix `onEndReached` bugs for FlatList #26444

ifsnow commented Sep 15, 2019 •

edited

Loading

kristerkari commented Oct 29, 2019 •

edited

Loading

lunaleaps left a comment •

edited

Loading