React-window: All items re-mount on list change

Created on 4 Feb 2020  路  15Comments  路  Source: bvaughn/react-window

I have an issue that seems identical to this. Each time one of the items in my list is modified, added, or removed, itemData changes. This results in all previously rendered items being re-mounted (not just re-rendered). My code is essentially identical to the examples in terms of form (data and list item differ, but the project is structured the same).

Conceptually, using memoize and React.memo should only prevent re-renders of currently mounted items. Thus, neither of those should have an effect on whether components are re-mounted. What other variables that are factored into the list's decision of whether to re-mount the items? I'm converting this from a project that used to render all items in a list, in which case the items are _not_ re-mounted. So the only difference in this case is the virtual list. I've been stuck on this for a while and feel like I'm missing something...

Most helpful comment

This happens because react-window treats children as a component which I believe is an anti-pattern for render-props as there are only a very limited number of cases where you want to pass a component. Most of the time you want to instead use an arrow function to pass additional props to your component from the parent closure.

The fix for this issue is really simple: just replace React.createElement(children, props) with direct children(props). Unfortunately it would be a breaking change: some existing code would rely on the fact that the passed function is a component and use something like useState inside.

@bvaughn as a quick fix before your v2 is out, can we opt out of this behaviour with a switch maybe? introduce noComponent prop to all the lists and pass it into createListComponent():

--- a/src/createListComponent.js
+++ b/src/createListComponent.js
@@ -145,6 +145,7 @@ export default function createListComponent({
   initInstanceProps,
   shouldResetStyleCacheOnItemSizeChange,
   validateProps,
+  noComponent,
 }: {|
   getItemOffset: GetItemOffset,
   getEstimatedTotalSize: GetEstimatedTotalSize,
@@ -321,15 +322,15 @@ export default function createListComponent({
       const items = [];
       if (itemCount > 0) {
         for (let index = startIndex; index <= stopIndex; index++) {
-          items.push(
-            createElement(children, {
-              data: itemData,
-              key: itemKey(index, itemData),
-              index,
-              isScrolling: useIsScrolling ? isScrolling : undefined,
-              style: this._getItemStyle(index),
-            })
-          );
+          const props = {
+            data: itemData,
+            key: itemKey(index, itemData),
+            index,
+            isScrolling: useIsScrolling ? isScrolling : undefined,
+            style: this._getItemStyle(index),
+          }
+
+          items.push(noComponent ? children(props) : createElement(children, props));
         }
       }

All 15 comments

I'd love to see this issue get some love. I've got a FixedSizeList with items that contain controlled text inputs such that the input's onChange affects some state outside of the FixedSizeList component. Every text input key press updates the state and triggers a re-render of the List; nothing out of the ordinary, just typical React controlled inputs. But because of the unmount/remount, the text input loses focus immediately. There's no scrolling going on here. Here's a trivial one-item example:

const ListTest: FC = () => {
  const [value, setValue] = useState('')

  return (
    <FixedSizeList height={150} itemCount={1} itemSize={35} width={300}>
      {({ style }) => (
        <input
          type="text"
          style={style}
          value={value}
          onChange={event => setValue(event.currentTarget.value)}
        />
      )}
    </FixedSizeList>
  )
}

Surely, only re-rendering should happen in this case and not fully unmounting and remounting. Any ideas?

What you're asking about is slightly different, @pgostovic, and seems like a duplicate of #420.

Don't define your item renderer inline. It will get recreated each time the parent component re-renders which will mess with e.g. focus

Oh man that totally works -- I feel kind of stupid now 馃槃. Thanks @bvaughn!

No worries. It's a common mistake. So much so that I plan to change the API for v2 (if I ever finish v2) so that the way you defined it above would not cause this problem.

@bvaughn My situation is similar to that of the landlord. I use event delegation here to click each list element to produce some effects! However, after scrolling, it is necessary to monitor the value from Redux to judge something. Because the parent component listens, and then distributes it to the child component, it will render again. This problem is that the list item in the current viewport is missing, but the DOM is still there. When scrolling in the blank area again, the list item will appear, but it is not the scrolling item before the blank! I want to listen to the changed values so as to make some interaction effects. If I listen to the values, I have to trigger re rendering again, resulting in incorrect list items. How to solve it?

@ryanmcclure4 Did you find a solution for it? I'm facing the same issue and really prevents me from providing a snappy experience.

@bvaughn Thoughts?

@b52 I didn't. I ended up implementing a non-virtual list that just paginates and adds more once you scroll to the bottom. Not as nice of a solution as virtual list would be, but it works.

@bvaughn hi, do you have any plan for this issue? Our product faces the same issue that any change in a node will cause all nodes in a tree get unmount and remount. This will cause bad accessibility issue that we can truly focus on a node, for example, voice-over cannot read the select(focused) node.

@bvaughn Hi, do you have any plan for this issue? My product faces the same issue that any change in a node will cause all nodes in a tree get unmount and remount.

This happens because react-window treats children as a component which I believe is an anti-pattern for render-props as there are only a very limited number of cases where you want to pass a component. Most of the time you want to instead use an arrow function to pass additional props to your component from the parent closure.

The fix for this issue is really simple: just replace React.createElement(children, props) with direct children(props). Unfortunately it would be a breaking change: some existing code would rely on the fact that the passed function is a component and use something like useState inside.

@bvaughn as a quick fix before your v2 is out, can we opt out of this behaviour with a switch maybe? introduce noComponent prop to all the lists and pass it into createListComponent():

--- a/src/createListComponent.js
+++ b/src/createListComponent.js
@@ -145,6 +145,7 @@ export default function createListComponent({
   initInstanceProps,
   shouldResetStyleCacheOnItemSizeChange,
   validateProps,
+  noComponent,
 }: {|
   getItemOffset: GetItemOffset,
   getEstimatedTotalSize: GetEstimatedTotalSize,
@@ -321,15 +322,15 @@ export default function createListComponent({
       const items = [];
       if (itemCount > 0) {
         for (let index = startIndex; index <= stopIndex; index++) {
-          items.push(
-            createElement(children, {
-              data: itemData,
-              key: itemKey(index, itemData),
-              index,
-              isScrolling: useIsScrolling ? isScrolling : undefined,
-              style: this._getItemStyle(index),
-            })
-          );
+          const props = {
+            data: itemData,
+            key: itemKey(index, itemData),
+            index,
+            isScrolling: useIsScrolling ? isScrolling : undefined,
+            style: this._getItemStyle(index),
+          }
+
+          items.push(noComponent ? children(props) : createElement(children, props));
         }
       }

Does anyone know if react-virtualized has this issue as well? I'm having the same issue as OP which was a bit frustrating and it doesn't seem like it's a concern to the maintainer(s).

EDIT: Just tried react-virtualized and it has the same issue unfortunately.

I was hitting my head around the whole evening trying to understand why everything keeps re-rendering until i find this thread which explains exactly my problem. I want to point out that this problem is specifically visible when you have image inside the list. The unmount-mount will cause flickering issue whenever a single prop change for the list.

I've created a demo showed this re-mount behavior based on memorized-list example, you could see multiple(seven times) memo logs in the console when click one of the items, which means all items got rerendered, it's indeed terrible, even worse when we have avatars in the list.

EDIT: would provide repro when I have time

@bvaughn Wondering when could this be fixed ?

@ZYSzys not sure what you are trying to do here. I replaced FixedSizeList with a usual map and got the same result https://codesandbox.io/s/vigorous-archimedes-cvv1c?file=/index.js

I don't think your issue lies within react-window

I'm also facing the re-rendering issue I have tried memo, created separate rows component approach nothing works for me.

https://user-images.githubusercontent.com/48905195/115861068-c46e5680-a44b-11eb-8c77-985b1da3d2ac.mp4

I have an array of objects which I am passing to renders the list. Now I have to select an item for the cart to proceed with checkout whenever I select the item it re-renders the whole list which is not good.

array = [{item:'abc'},{item:'def'},....]; obj={abc:{ isChecked:true },def:{ isChecked:true },...}

So what I am doing is onChange handler of input it adds a new object in obj if the item already exists it just change it's value, and I used that value to check if the item is checked or not
like
checked={obj[map.item]?obj[map.item]:false}
where checked is an attribute of the input element.

@bvaughn
if you have any solution to fix it, It will be really helpful for me. thanks

Was this page helpful?
0 / 5 - 0 ratings