bvaughn / react-window

React components for efficiently rendering large lists and tabular data
https://react-window.now.sh/
MIT License
15.54k stars 779 forks source link

Unnecessary unmounting and remounting of grid items #698

Closed epurban closed 1 year ago

epurban commented 1 year ago

Hi @bvaughn and company, first of all would like to thank you for creating this awesome library as well as react-virtualized. My team is using both libraries, but looking to move over to react-window for our grid use case. However, we're running into an issue that we've solved in react-virtualized, but cannot seem to overcome here.

We use a resizeObserver implementation to listen for the window width and resize our grid based on that. We memoize our grid cell component that is rendered for each cell so that it doesn't re-render when the window resizes, but what I'm experiencing with react-window is that every time a resize takes place and our width changes thats passed into FixedSizeGrid, we get a massive amount of unmounts and remounts of our grid cells.

With react-virtualized this seems to be avoidable by passing down the key prop that comes from the cellRenderer function to the grid cell itself. I noticed this key prop is absent in react-window. Perhaps this is already possible with react-window's API and I am missing something? I'm sure that others have experienced this issue before but I was really unable to find anything in the issues here.

bvaughn commented 1 year ago

but what I'm experiencing with react-window is that every time a resize takes place and our width changes thats passed into FixedSizeGrid, we get a massive amount of unmounts and remounts of our grid cells.

Sounds like you're re-creating the components themselves. Where's your code?

bvaughn commented 1 year ago

I'm guessing it's the same as this? https://github.com/bvaughn/react-window/issues/59#issuecomment-422422350

epurban commented 1 year ago

I thought it may be related to my grid cell component, so I swapped it out for a very simple implementation here and am still getting that behavior:

const CardItem = React.memo<CardItemProps>(({ style, rowIndex, columnIndex }) => {
  useEffect(() => {
    console.log('MOUNT CARD');
    return () => console.log('UNMOUNT CARD');
  }, []);

  return (
    <div style={{ ...style, border: '1px solid black' }}>
      Item {rowIndex},{columnIndex}
    </div>
  );
}, areEqual);

export const CardGrid: React.FC<CardGridProps> = ({
  cards,
  constructLink,
  minCardsInRow = 2,
  maxCardsInRow,
  type,
}) => {

...

return (
    <div ref={ref} style={{ flex: '1 1 auto', overflow: 'hidden' }}>
      <FixedSizeGrid
        rowCount={rowCount}
        columnCount={itemsPerRow}
        rowHeight={cardWidth + cardContentHeight + GRID_GAP}
        columnWidth={cardWidth}
        height={height || window.innerHeight}
        width={gridWidth}
        overscanRowCount={10}
      >
        {CardItem}
      </FixedSizeGrid>
    </div>
  );
epurban commented 1 year ago

Alright, I've clarified some things, the code above does actually work as expected and stop unmounting/remounting. Here is my actual issue:

The more advanced cell component that I must omit needs additional props that are calculated from the rowIndex and columIndex that come in, like this:

const props = cards[rowIndex * itemsPerRow + columnIndex];

However, I'm unable to access the cards object above because it is a prop of CardGrid, and now that CardItem is a component outside of CardGrid, it does not have access to that object.

How can I pass down additional props to the CardItem cell component in addition to rowIndex, columnIndex which are needed for properly rendering? I tried the following and it did not stop unmounting:

<FixedSizeGrid ...>
{({ columnIndex, rowIndex, style }) => {
          const props = cards[rowIndex * itemsPerRow + columnIndex];
          return (
            <CardItem
              props={props}
              constructLink={constructLink}
              style={style}
              rowIndex={rowIndex}
              columnIndex={columnIndex}
            />
          );
        }} 
</FixedSizeGrid>
bvaughn commented 1 year ago

That's why the itemData property exists.

https://react-window.vercel.app/#/examples/list/memoized-list-items

epurban commented 1 year ago

Ah, I missed this! That worked perfectly. If anyone else is having this issue, then this will help them too 🔥 Thanks Brian!