bvaughn / react-virtualized

React components for efficiently rendering large lists and tabular data
http://bvaughn.github.io/react-virtualized/
MIT License
26.31k stars 3.05k forks source link

Data changes don't propagate to Grid cells to cause a rerender #301

Closed vjancik closed 8 years ago

vjancik commented 8 years ago

My use case broke, with the update to 7.11.0. The FlexTable didn't rerender a row when the data changed, until I started scrolling.

I updated to 7.11.2 and the issue seemed resolved.

However now I started using VirtualScroll, and I am experiencing the same issue with it. I tried updating to 7.11.3, but it seems to be there as well.

Basically, the moment I scroll, the row rerenders with the latest data, but the data is there before, and it should have rerendered the moment the data changed right?

Since this issue appeared with FlexTable and got resolved with a new release, is it possible it's not fixed with VirtualScroll?

Thank you, Viktor

bvaughn commented 8 years ago

I think the underlying issue here is what's mentioned in the docs: https://github.com/bvaughn/react-virtualized#pure-components

If the rows change, but no react-virtualized props change, then things don't get re-rendered.

I backed off of this for FlexTable for simplicity's sake. I just relaxed the pass-thru props on VirtualScroll similar (released as 7.11.4). If this doesn't result your issue, double check your props to ensure that the things RV sees are actually changing (and not just related data).

vjancik commented 8 years ago

Okay, so your update 7.11.3 -> 7.11.4 didn't fix the issue, but your comment made me realize, that with VirtualScroll, unlike with FlexTable, I don't actually pass the data in as a prop. I only specify a rowRenderer, the output of which does change when data changes, but no props on the actual VirtualScroll actually change.

I got around this by passing the data to VirtualScroll through the data prop, even though it doesn't ask for it, and seems React takes care of re-rendering the component, even if the component doesn't use the property I passed it (that changed) anywhere.

Thank you!

vjancik commented 8 years ago

Based on this commit: https://github.com/bvaughn/react-virtualized/commit/8184bcb6fb3aaf89ff58d9fe8877554702882159

I would have guessed my fix wouldn't work prior 7.11.4, but it does (with 7.11.3). Strange?

bvaughn commented 8 years ago

That's a pretty common source of confusion. It's why I added the "pure components" blurb to the main README.

The shallowCompare function doesn't just limit its comparisons to the declared propTypes of a component. Any prop (include one like randomNumber={Math.random()} can trigger a re-render if its value has changed.

nschwan94 commented 7 years ago

It's possible I'm facing a similar issue. I've tried the passing thru props and forceUpdate method on a Grid component and neither seems to update the data displayed. The catch is, it will update the visible rows as soon as a scroll is registered after the data is updated.

bvaughn commented 7 years ago

Share a working repro and I'll take a look.

nschwan94 commented 7 years ago

My mistake, I've never called forceUpdate() on a ref before, so I was trying to call it on the parent component leading to nothing.

hung-phan commented 7 years ago

The simplest solution is that you have a key change:

          <div
            key={sth unique here}
            style={{ minHeight: numOfRows * MainPanelDataTable.rowHeight }}
          >
            <AutoSizer>
              {({ width, height }) =>
                <Grid
                  cellRenderer={this.cellRenderer}
                  rowCount={numOfRows}
                  columnCount={numOfColumns}
                  rowHeight={MainPanelDataTable.rowHeight}
                  columnWidth={MainPanelDataTable.columnWidth}
                  width={width}
                  height={height}
                />}
            </AutoSizer>
          </div>

It will re-render the whole component

jarekb84 commented 7 years ago

I'm encountering the same issue with a MultiGrid with editable cells. @hung-phan key solution works, but it's slow and causes the text input to lose focus when the whole grid is re-rendered.

I also tried using forceUpdateGrids from ref, but I'm dispatching an action to update the redux store on each change, and need to fire forceUpdateGrids after the store updates and passes the updated data down from connected props. Because the force update happens before the store is updated, experience right now is wonky since user see the state of the input alternate as they type.

Are there any alternate solutions or an example of an editable grid that solves this?

bvaughn commented 7 years ago

Setting a new key is kind of a nuclear option because it will cause React to recreate all components beneath it (rather than updating and reusing them). This means you'll lose any data stored in component state (eg things that aren't in Redux, etc).

I'd recommend an approach like I mention in the docs or in this presentation since it's more efficient.

jarekb84 commented 7 years ago

@bvaughn Thanks, that's a much faster/cleaner solution. I've been reading though the individual component docs, but missed this in the main readme.

felix9ia commented 6 years ago

@vjancik @bvaughn https://github.com/bvaughn/react-virtualized/commit/8184bcb6fb3aaf89ff58d9fe8877554702882159 my grid is re-rendered! thanks!

katzman commented 5 years ago

@bvaughn Hey Brian, sorry to bug you about this, i'm having the same issue everyone above has been having with no luck at all getting it to update.

I'm using a List, with AutoSize and CellMeasurer, i have it so the user can drag the columns to reorder the row item cells around, the rowRenderer method builds a grid row based on the column order.

The column data is not being passed as a prop to the List component. I have tried calling both forceUpdateGrid and forceUpdate onComponentDidUpdate, i'm even passing your random number fix as well as a Unix time stamp that gets updated each time any value on the grid changes including columns. I have seen the issue where as i scroll the unrendered components will show with the updated columns. I even tried the key nuclear option with no love.

I can pass some code of my setup, this is for Boeing so i can't share much. Any help would be much appreciated.

Thank you!

jlibrun22 commented 5 years ago

@bvaughn Hey Brian, sorry to bug you about this, i'm having the same issue everyone above has been having with no luck at all getting it to update.

I'm using a List, with AutoSize and CellMeasurer, i have it so the user can drag the columns to reorder the row item cells around, the rowRenderer method builds a grid row based on the column order.

The column data is not being passed as a prop to the List component. I have tried calling both forceUpdateGrid and forceUpdate onComponentDidUpdate, i'm even passing your random number fix as well as a Unix time stamp that gets updated each time any value on the grid changes including columns. I have seen the issue where as i scroll the unrendered components will show with the updated columns. I even tried the key nuclear option with no love.

I can pass some code of my setup, this is for Boeing so i can't share much. Any help would be much appreciated.

Thank you!

@katzman I'm having the same issue did you find a solution?

sfilippenko commented 3 years ago

I solved this way

React.useEffect(() => {
    if (gridInstance.current) {
        cache.current.clearAll();
        gridInstance.current.forceUpdateGrids();
        gridInstance.current.forceUpdate();
    }
}, [data]);

Grid:

<MultiGrid
    ref={gridInstance}
    height={height}
    width={width}
    rowHeight={cache.current.rowHeight}
    rowCount={data.length + 1}
    fixedColumnCount={1}
    fixedRowCount={1}
    columnCount={2}
    columnWidth={columnWidth}
    cellRenderer={cellRenderer}
    cellRangeRenderer={cellRangeRenderer}
/>
worker-py commented 3 years ago

I solved this way

React.useEffect(() => {
    if (gridInstance.current) {
        cache.current.clearAll();
        gridInstance.current.forceUpdateGrids();
        gridInstance.current.forceUpdate();
    }
}, [data]);

Grid:

<MultiGrid
    ref={gridInstance}
    height={height}
    width={width}
    rowHeight={cache.current.rowHeight}
    rowCount={data.length + 1}
    fixedColumnCount={1}
    fixedRowCount={1}
    columnCount={2}
    columnWidth={columnWidth}
    cellRenderer={cellRenderer}
    cellRangeRenderer={cellRangeRenderer}
/>

I solved this way

React.useEffect(() => {
    if (gridInstance.current) {
        cache.current.clearAll();
        gridInstance.current.forceUpdateGrids();
        gridInstance.current.forceUpdate();
    }
}, [data]);

Grid:

<MultiGrid
    ref={gridInstance}
    height={height}
    width={width}
    rowHeight={cache.current.rowHeight}
    rowCount={data.length + 1}
    fixedColumnCount={1}
    fixedRowCount={1}
    columnCount={2}
    columnWidth={columnWidth}
    cellRenderer={cellRenderer}
    cellRangeRenderer={cellRangeRenderer}
/>

Thank you very much @sfilippenko But in case it's not clear you need useRef from "react"

import {useRef} from "react"

export default GridInstance(props){
    const gridInstance = useRef();
    // your MultiGrid component logic
}