mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.07k stars 1.26k forks source link

[DataGrid] Cells are rerendered at every event which drastically decreases performance #3480

Closed igor-serzhan closed 2 years ago

igor-serzhan commented 2 years ago

Duplicates

Latest version

Current behavior 😯

Grid cells rerender at every event (for example focus, keydown, click, etc.). Even in cell editing mode all cells in the grid will be rerendered with every keydown event. Also it's not possiibe to disable event listeners with passing null as props to event callback (for example, onCellKeyDown={null} It makes impossible to use Data Grid with large tables (at least 50 row and 5 columns).

Expected behavior 🤔

Cells should not be rendered at every event.

Steps to reproduce 🕹

Steps:

  1. Open https://codesandbox.io/s/datagrid-performance-issue-zqn91
  2. Open console
  3. Enable edit mode for the first column
  4. Type something
  5. Take a look at the count of 'Render' outputs

Context 🔦

I'm trying to build a large table with 50 visible rows and 8 columns with 4 custom components. Currently performance is too low to be able to use this library.

Your environment 🌎

see code sandbox

Order ID 💳 (optional)

No response

m4theushw commented 2 years ago

The grid rerenders at every click because it needs to update the selected rows count and the focused cell. Could you detail what you're trying to build whose performance is affected by this behavior?

Even in cell editing mode all cells in the grid will be rerendered with every keydown event.

This can be fixed by debouncing the keydown events.

igor-serzhan commented 2 years ago

The grid rerenders at every click because it needs to update the selected rows count and the focused cell.

But what if I don't need selection, keydown events, etc.? There is should be possibility to disable rerendering whole grid. I have custom input components with inner logic and inner state (not that this custom component is not rerending, just cell wrapper). And this input are freezing on each keydown event.

I updated sandbox example with input components. Try to type something fast at this input to see these freezing.

Screen Recording 2021-12-20 at 22 19 15

michael-land commented 2 years ago

Are you in production build? things are much slower in dev mode.

I also hope the editing mode can be uncontrolled, delay the validation on submit.

igor-serzhan commented 2 years ago

Are you in production build? things are much slower in dev mode.

The same thing happens in production. Rendering more than 500 components requires a lot of resources

m4theushw commented 2 years ago

I opened #3484 with a small change to avoid rerendering the grid when typing into a cell that already has focus. Notice that clicking into other cells still causes a new render because we need to update the internal state and propagate that change to the other components. The following CodeSandbox demonstrates the change: https://codesandbox.io/s/datagrid-performance-issue-forked-mhlpg?file=/src/App.tsx

Does it work for you?

igor-serzhan commented 2 years ago

@m4theushw thanks, this will partially solve the problem. But in perfect world we should not call any event listeners and trigger rerender if we don't use focus/selection/other functionalities (like in my current project).

m4theushw commented 2 years ago

Only in DataGridPro is possible to completely disable the default event listeners. The rerender caused when clicking a row can be avoided using disableSelectionOnClick. But it doesn't disable selecting rows via keyboard.

Could you detail more what you're trying to build where rerenders is a bottleneck? Besides the lack of debouncing when typing, which will be fixed, sorting and scroll works smoothly.

justin-hoops commented 2 years ago

You're kidding, right? A grid with 100 rows and 50 columns, and I am seeing on a click on a cell every cell is rendered twice, and you are asking what the performance problem is?

You're saying, "Oh, yeah, if you click a cell, we will do 10,000 renders, but what's the problem?"

xepozz commented 1 year ago

Still happening...

akash87 commented 1 year ago

@justin-hoops @xepozz found any workaround / solution ?

s-ueno commented 1 year ago

The problem is that when custom rendering is implemented, such as in the Row of compornents, the creation and destruction of objects occurs repeatedly.

The states on the screen all revert to their initial values for a moment, and the values are reflected at the time of re-rendering.

This is very unsightly.

Can we consider a minimalist approach, for example, suppressing re-rendering in the case of disableVirtualization?

xepozz commented 1 year ago

@akash87 Tuned a few on* handlers that definitely decreased lot of rerenders, but they are still happening. Tried many ways but this way is one that disables triggering events just by clicking inside a data grid :) https://github.com/xepozz/yii-dev-panel/blob/3a1bd85538c4f98af70f6c02255e4e367a0fa5ed/src/Component/Grid.tsx#L31-L38

akash87 commented 1 year ago

@xepozz and everyone else, I was very frustrated by this issue, but I found a fix which works for me. I had autoheight set to true, which was causing all rows (100+) to render everytime. I made autoheight: false, which causes the virtualization to work. It renders only those cells which are visible, and others are rendered only on scroll. This fixes the slowness caused by state change (row selection etc) . Let me know if anyone needs more information or just check mui virtualization and autoheight documentation, they've everything clearly documented.

xepozz commented 1 year ago

@akash87 I've tried this way, but my content inside the data grid is interactive and it changes the height 1) after render whole page 2) when interact with it. So it didn't work.

michaelperrin commented 11 months ago

It looks like re-renders on all cells are still happening on every single key stroke during inline-editing. This can be verified when using the Highlight updates when components render. option of React DevTools on a very simple grid like this one (code on CodeSandbox).

Do you think a new issue should be opened for it?

romgrk commented 11 months ago

Sure, open a new report with as much details as possible.

michaelperrin commented 11 months ago

Thanks @romgrk. I just opened #10639 as a follow-up.