Skip to content
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

dont rerender list items #31313

Closed
wants to merge 1 commit into from

Conversation

halaei
Copy link
Contributor

@halaei halaei commented Apr 6, 2021

Summary

Using FlatList components with 50 items, and the default values for initialNumToRender and maxToRenderPerBatch, the following happens:

  • Render first 10 items (Items[0 .. 9]).
  • Sleep.
  • Render first 20 items (Items[0 .. 19]). It should be rendering items [10 .. 19] instead.
  • Sleep.
  • Render first 30 items (Items[0 .. 29]).
  • Sleep.
  • Render first 40 items (Items[0 .. 39]).
  • Sleep.
  • Render all items (Items[0..49]).

So the flat list is rendering the first 10 items 5 times. It is a performance issue when the items are complicated, or the FlatList has many items. Having N items the FlatList will render O(N^2) items over time.

This PR adds a shouldComponentUpdate() for the CellRenderer that always return false to disable redundant renders. It seems it doesn't break anything. For our apps it also reduces about 1 second of CPU usage of calls to React.createElement when rendering some components in our app.

An alternative to this PR is to add the same shouldComponentUpdate() in our own items. However, that is a problem for functional components and not as efficient as checking for update in FlatList itself.
https://reactnative.dev/docs/optimizing-flatlist-configuration#use-shouldcomponentupdate

If this change breaks something that I am not aware of, we can add some flag/function prop to FlagList, so the logic in CellRenderer.shouldComponentUpdate() can be set by the users.

Changelog

[JavaScript] [Fixed] - Don't render list items if not changed.

Test Plan

Current tests should pass.

@facebook-github-bot 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 Apr 6, 2021
@analysis-bot
Copy link

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

Base commit: a15a46c

@halaei halaei force-pushed the dont-rerender-list-items branch 3 times, most recently from 465e679 to 4806b65 Compare April 6, 2021 14:15
@halaei
Copy link
Contributor Author

halaei commented Apr 7, 2021

It still has the same issue with separator. I try to fix it later.

@efstathiosntonas
Copy link

efstathiosntonas commented Apr 8, 2021

This patch could cause issues with some libraries that use a single item passed to data prop, ie react-native-actions-sheet in this line: #L622

The issue I encountered is that the items inside the bottom sheet wouldn't listen to state updates, ie trying to change the text of a button when user taps another button in there.

@halaei
Copy link
Contributor Author

halaei commented Apr 8, 2021

@efstathiosntonas I am opening another PR, #31327. It takes a different approach because this PR really doesn't work.

@halaei halaei closed this Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants