SectionList unnecessarily unmounts and mounts items when filtering (in between renders)

[HectorRicardo]

Description

When a SectionList rerenders, sometimes it unmounts and remounts some of its items unnecessarily, even though the items preserve the same "key" prop in between renders. This impacts performance.

A scenario where this could happen is when filtering data. Imagine a SectionList with a search bar (TextInput) above it. Every time the user types something in the search bar, the section list gets filtered so it shows the items that contain the text in the search bar. Sometimes, the items get remounted unnecessarily when typing (Why remount an item when it was already there? It passed the filter, it should stay there.)

React Native version:

System: OS: Windows 10 10.0.18363 CPU: (8) x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz Memory: 6.52 GB / 15.87 GB Binaries: Node: 12.14.0 - C:\Program Files\nodejs\node.EXE Yarn: Not Found npm: 6.13.4 - C:\Program Files\nodejs\npm.CMD Watchman: Not Found SDKs: Android SDK: Not Found IDEs: Android Studio: Version 3.6.0.0 AI-192.7142.36.36.6392135 Languages: Java: Not Found Python: 2.7.17 - C:\Python27\python.EXE npmPackages: @react-native-community/cli: Not Found react: 16.11.0 => 16.11.0 react-native: 0.62.2 => 0.62.2 npmGlobalPackages: react-native: Not Found

Steps To Reproduce

1. Create a new react-native project. npx react-native init sectionlisttest
2. Modify the App.js file so it looks like the following.

import React, {useEffect, useState} from 'react';
import {SectionList, Text, View} from 'react-native';

const originalData = [{
  key: 'header1',
  header: 'Header 1',
  data: [{key: 'firstItem'}, {key: 'byebye1'}],
}];

const filteredData = [{
  key: 'header1',
  header: 'Header 1',
  data: [{key: 'firstItem'}],
}];

export default function App() {
  useEffect(() => {
    setTimeout(() => setSections(filteredData), 5000);
  }, []);

  const [sections, setSections] = useState(originalData);

  return (
    <SectionList
      sections={sections}
      renderItem={renderItem}
      renderSectionHeader={renderSectionHeader}
      ItemSeparatorComponent={Separator}
    />
  );
}

const renderItem = ({item}) => <Item item={item} />;

const Item = ({item}) => {
  useEffect(() => {
    console.log('item mounted');
  }, []);
  return <Text>{item.key}</Text>;
};

const renderSectionHeader = ({section}) => <Text>{section.header}</Text>;

const Separator = () => <View style={{height: 1, backgroundColor: 'black'}} />;

3. Run the app. I ran it in Android (npx react-native run-android).

IMPORTANT: Fully reload the app, don't depend on hot reloading.

Expected Results

Expected result: 2 lines saying "Item mounted" get printed to the logs.

Actual Result: 2 lines saying "Item mounted" get printed to the logs. . But then, 5 seconds later, it prints a third line also saying "Item mounted".

Snack, code example, screenshot, or link to a repository:

Snack: https://snack.expo.io/@hectorricardo/sectionlist-bug

[stale[bot]]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

[safaiyeh]

Thanks for the issue @HectorRicardo, seems to only happen on Android through the snack. Is this fine on iOS for you?

[HectorRicardo]

Hello, I am on a trip right now and don't have access to a Mac or iphone to test. Can I get back to you next week to confirm if this also happens on iOS?

[safaiyeh]

@HectorRicardo no worries! I'll keep this open until then

[HectorRicardo]

Hello @safaiyeh I was able to reproduce this same bug in iOS. The code is exactly the same as in the description.

[jehartzog]

I think this has something to do with stickyHeaderIndices in VirtualizedSectionList.

In _computeState if you set stickyHeaderIndices to always at least have a single entry ([0]) this remount no longer occurs. But obviously that's not a valid way to fix this.

For the meantime we'll continue our workaround of using a special case check on renderItem to render our data given a single special section, versus relying on the ListEmptyComponent and passing an empty sections array.

Edit: Actually the above hack seems to be working for our implementation. Here is the patch-package we applied.

[fabOnReact]

@HectorRicardo @jehartzog I would like to receive some inputs from you as I don't know well this component. Thanks :peace_symbol: Sorry for disturbing you :pray:

https://github.com/facebook/react-native/blob/ff3c1307ff38ee586171c40c8512ec4bb07b0bf0/Libraries/Lists/VirtualizedSectionList.js#L176-L182

could this be caused by the change in itemCount when you use the filter?

 LOG  Running "RNTesterApp" with {"rootTag":91}
 LOG  itemCount 4
 LOG  item mounted
 LOG  item mounted
 LOG  itemCount 3
 LOG  item mounted

https://github.com/facebook/react-native/blob/ff3c1307ff38ee586171c40c8512ec4bb07b0bf0/Libraries/Lists/VirtualizedSectionList.js#L194-L211

itemCount updates the state which re-render the list? but item with the same key are unmounted/remounted PureComponent should not re-render in this cases

https://github.com/facebook/react-native/blob/ff3c1307ff38ee586171c40c8512ec4bb07b0bf0/Libraries/Lists/VirtualizedSectionList.js#L170-L172

I'm trying to understand the functionality, I'm reading https://github.com/facebook/react-native/commit/26a1eba1cef853b0dab7aad5731699c06d36b781 and https://github.com/facebook/react-native/pull/20787 to better understand this problem

[jehartzog]

@fabianeichinger I did not find that itemCount was a driver for this behavior, and it was something I looked into. Are you using any stickyHeaderIndices in your example? Based on my testing, I suspect your itemCount changes are actually affecting stickHeaderIndices which is causing the issue.

This is speculation, I didn't really figure out the underlying issue here. My fix in the above comment didn't work, so I ended up hacking around it by making sure my list of data provided is never empty, and using a custom key to show the needed zero state, rather than using the provided props. It now works correctly, was just awkward to code around.

[gabriel-moresco]

do you have any solution for this problem?

it's happening to me too.

@HectorRicardo how did you handle with this problem?

[SimpleCreations]

Hi everyone! I've been facing the same issue and wanted to share what I've found out.

As @jehartzog hinted, the issue can be reproduced by changing the value of ScrollView's stickyHeaderIndices from undefined or an empty array to a non-empty array, or vice versa. This causes all the elements in the scroll view to remount. Since SectionList uses VirtualizedList, which in turn uses ScrollView, changing the amount of sticky headers also updates the stickyHeaderIndices prop from ScrollView.

Upon investigating further, I've found out that the remount is caused by this line: https://github.com/facebook/react-native/blob/8993ffc82e8d4010d82dcb1d69c33a609bb2771a/Libraries/Components/ScrollView/ScrollView.js#L1641

Apparently, React differentiates between the value of the children prop and an array created from it, and considers them to be different nodes in the tree, therefore remounting the children when switching from one representation to the other; and currently the condition on which representation to use is whether the list of sticky header indices is empty or not. My solution is to always turn the children prop into an array:

diff --git a/node_modules/react-native/Libraries/Components/ScrollView/ScrollView.js b/node_modules/react-native/Libraries/Components/ScrollView/ScrollView.js
index 7cee521..a4f3e61 100644
--- a/node_modules/react-native/Libraries/Components/ScrollView/ScrollView.js
+++ b/node_modules/react-native/Libraries/Components/ScrollView/ScrollView.js
@@ -1635,12 +1635,10 @@ class ScrollView extends React.Component<Props, State> {
           };
 
     const {stickyHeaderIndices} = this.props;
-    let children = this.props.children;
+    let children = React.Children.toArray(this.props.children);
 
     if (stickyHeaderIndices != null && stickyHeaderIndices.length > 0) {
-      const childArray = React.Children.toArray(this.props.children);
-
-      children = childArray.map((child, index) => {
+      children = children.map((child, index) => {
         const indexOfIndex = child ? stickyHeaderIndices.indexOf(index) : -1;
         if (indexOfIndex > -1) {
           const key = child.key;
@@ -1653,7 +1651,7 @@ class ScrollView extends React.Component<Props, State> {
               nativeID={'StickyHeader-' + key} /* TODO: T68258846. */
               ref={ref => this._setStickyHeaderRef(key, ref)}
               nextHeaderLayoutY={this._headerLayoutYs.get(
-                this._getKeyForIndex(nextIndex, childArray),
+                this._getKeyForIndex(nextIndex, children),
               )}
               onLayout={event => this._onStickyHeaderLayout(index, event, key)}
               scrollAnimatedValue={this._scrollAnimatedValue}

Maybe someone here with a more in-depth knowledge of React can suggest a better fix?

Edit: typo

[edsztukowski]

I just chased down an issue with this where I was dynamically adding items to a list on button press, and the previously added item was re-rendering each time.

This turns out to be related to the ItemSeparatorComponent prop. When the number of list items change, it's re-rendering the previous item in the list as well.

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[SimpleCreations]

Not stale

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[SimpleCreations]

This needs to be retested, because a different package is now used for virtualized lists, so my patch won't apply to more recent RN versions

[react-native-bot]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[SimpleCreations]

Again - someone needs to check whether this is still an issue

[RohovDmytro]

Not stale.

[react-native-bot]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[SimpleCreations]

Not stale

[react-native-bot]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[SimpleCreations]

Not stale

[alex-mironov]

having the same issue