[data grid] Cells are rerendered on every click

[rgavrilov]

Order ID 💳

52426

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

The problem in depth 🔍

Same as:
#3480 (comment)

I just need you to confirm that a trivial UI operation is still causing unnecessary rerendering of all the cells (which is ridiculous).

Sandbox:
https://codesandbox.io/s/datagrid-rerendering-issue-tzdri2

just click around and watch Value column being rerendered.

The issue:
In my actual use case cells are rendered with 4 buttons inside. Even scrolling becomes too sluggish. I'm questioning whether this component would be usable in the UI.

Your environment 🌎

`npx @mui/envinfo`

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

[rgavrilov]

At the very least, I need an example of how to cache a rendered cell. I have tried caching a result of renderCell:

            renderCell: (params: GridRenderCellParams<Response, Row, any>) => {
                const response = params.value!;

                if (cache[params.rowNode.id]) {
                    return cache[params.rowNode.id];
                } else {
                    console.log("missed cache");
                    return cache[params.rowNode.id] = <CheckboardCell response={ response }/>;
                }

            },

but that didn't help much. Even assuming that response doesn't change, the CheckboardCell is being rerendered.

[yaredtsy]

I have checked it with the profiler and everything is being rendered for some reason.

bandicam.2023-02-03.12-01-46-957.mp4

I think it should have been like this.

bandicam.2023-02-03.12-13-58-314.mp4

[m4theushw]

I just need you to confirm that a trivial UI operation is still causing unnecessary rerendering of all the cells (which is ridiculous).

Yes, this happens because each cell is responsible for calling .focus() when it should be focused. AG-Grid handles this differently. In their case, the grid itself finds which element should have focus and calls .focus(). The DataGrid implements a declarative approach, while AG-Grid uses an imperative one. The problem with switching to the imperative approach now is that we need to ensure that the next cell to be focused is in the DOM (ReactDOM.flushSync could be used) and, when providing custom cell renderers containing interactive elements, users will have to handle the focus outside the cell component. Also, the management of the tabIndex attribute is tied to the focus logic, so it will also be impacted.

At the very least, I need an example of how to cache a rendered cell.

You could try wrapping CheckboardCell in React.memo but I don't believe this will bring any relevant improvement.

[yaredtsy]

hey @m4theushw, anything you do in the datagrid causes every componet(ColumnHeader, footer, row, cell) to rerender, even moving the mouse in and out of the datagrid is causing rerender.

bandicam.2023-02-05.21-18-50-797.mp4

[m4theushw]

At the current state we can't easy solve this issue because the state of the grid is all stored in the same place, so if the parent component re-renders, their children will also re-render. Also, we can't wrap all components in React.memo because it will cause other problems like cells displaying outdated values. However, one workaround is to memoize the custom cell renderers, like I showed in #7821 (comment)

[m4theushw]

I've been working in a way to allow to use React.memo to memoize rows, cells and column headers in #7846. By doing so, the number of re-renders reduced drastically. Could you check if performance has improved? Here's the CodeSandbox using the code from the PR: https://codesandbox.io/s/datagrid-rerendering-issue-forked-vizc7w?file=/src/DataTable.js

[yaredtsy]

it's perfect now, the only problem I had was, with the custom cell, I had to memorize it myself.

[yelodevopsi]

@m4theushw : Really appreciation if memorize of rows, cells and column can get in place!
The model should be to create custom Components like you do in the PR link?

Will test your commit and report back: https://pkg.csb.dev/mui/mui-x/commit/794239b6/@mui/x-data-grid <3

Note: Already memoized my custom renderCell, but it seem that the apiRef used through the passed props makes this problematic.

[m4theushw]

The model should be to create custom Components like you do in the PR link?

@rompeldunk Almost yes, I'm actually only wrapping the default components in React.memo.

The model should be to create custom Components like you do in the PR link?

Only memoizing renderCell doesn't bring much benefit. What really needs to be done is to memoize the component that renders the cell and only with the change from this PR it's possible.

[yelodevopsi]

@m4theushw Would it be possible to test an equivalent PR to x-data-grid-pro?
I tried mixing the commit with my existing code and memoize it, but it it broke, as expected.

We're trying to render many hundreds, up to many thousand of rows. One checkboxSelection event renders the whole stack. Also tested with renderCell disabled, but as you mentioned, it doesn't matter much.

<DataGridPro
  rows={rows}
  columns={browseColumns}
  columnBuffer={3}
  rowHeight={32}
  autoHeight
  autoPageSize
  components={{
    // Memoized comp's here
  }}
  disableSelectionOnClick={true}
  checkboxSelection
  rowBuffer={3}
  onSelectionModelChange={handleSetSelectedRowIds}
  onCellEditCommit={handleCellEditCommit}
  onColumnHeaderClick={handleColumnHeaderClick}
  />

[cherniavskii]

@rompeldunk

Would it be possible to test an equivalent PR to x-data-grid-pro?

Yes, you can find links to the packages in the Packages section of Codesandbox CI build: https://ci.codesandbox.io/status/mui/mui-x/pr/7846/builds/344182

[yelodevopsi]

@rompeldunk

Would it be possible to test an equivalent PR to x-data-grid-pro?

Yes, you can find links to the packages in the Packages section of Codesandbox CI build: https://ci.codesandbox.io/status/mui/mui-x/pr/7846/builds/344182

@cherniavskii
I notice that there's a lot of breaking changes. Maybe I've picked version 6?
Extranote: By this I mean other dependencies in other files (outside this scope/example) which have changed name, thus if this is not v5.**.* build

I tested this one:
https://pkg.csb.dev/mui/mui-x/commit/794239b6/@mui/x-data-grid-pro

Currently using "@mui/x-data-grid-pro": "5.15.1",

[yelodevopsi]

Just to clarify: (in my use-case at least)

• It is the params from the renderCell (params: GridRenderCellParams) which cause the problems, since i.e. the api reference which changes for every render?

• Wouldn't using a callback instead (to a state, setState) with none-changing ref instead do the trick?

• Would an alternative optimization-technique be to use the components and componentsProps props?

[cherniavskii]

I notice that there's a lot of breaking changes. Maybe I've picked version 6?

Yes, it's v6. We do not plan to release this improvement on v5 since we are going to release v6 stable during the next 3-4 weeks.

[m4theushw]

It is the params from the renderCell (params: GridRenderCellParams) which cause the problems, since i.e. the api reference which changes for every render?

@rompeldunk Partially yes, but the problem here appears before reaching renderCell. It's the props passed to the component responsible for rendering the rows that are changing on every render.

[cherniavskii]

Is there a specific use case where you can observe the data grid performance degradation?
We need a real-world example to take action on this.
It's not clear whether it's a UX or DX issue.

[github-actions]

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

[LinnaViljami]

Still facing the problem. The whole DataGrid is rerendered on every cell or row click, all rows and cells are rendered. It causes a significant performance issue with a table over about hundred cells