Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
igor-serzhan opened this issue Dec 20, 2021 · 17 comments · Fixed by #3484
Closed
2 tasks done
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!

Comments

@igor-serzhan
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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

@igor-serzhan igor-serzhan added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 20, 2021
@m4theushw
Copy link
Member

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.

@m4theushw m4theushw added component: data grid This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 20, 2021
@igor-serzhan
Copy link
Author

igor-serzhan commented Dec 20, 2021

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
Copy link

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
Copy link
Author

igor-serzhan commented Dec 20, 2021

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
Copy link
Member

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?

@m4theushw m4theushw added the bug 🐛 Something doesn't work label Dec 20, 2021
@igor-serzhan
Copy link
Author

igor-serzhan commented Dec 21, 2021

@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
Copy link
Member

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
Copy link

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
Copy link

xepozz commented Nov 28, 2022

Still happening...

@akash87
Copy link

akash87 commented Jan 31, 2023

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

@s-ueno
Copy link

s-ueno commented Feb 1, 2023

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
Copy link

xepozz commented Feb 1, 2023

@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
Copy link

akash87 commented Feb 2, 2023

@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
Copy link

xepozz commented Feb 2, 2023

@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
Copy link

michaelperrin commented Oct 11, 2023

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
Copy link
Contributor

romgrk commented Oct 11, 2023

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

@michaelperrin
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants