search

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/
3.82k stars 1.14k forks source link

[data grid] onMouseUp may be triggered on a cell after the parent row is removed #12888

Closed martinjlowm closed 2 weeks ago

martinjlowm commented 3 weeks ago

Steps to reproduce

https://github.com/mui/mui-x/assets/110860/c46c8fe0-9dd8-416d-8d6e-4a90de339727

Current behavior

getCellParams asserts for the existence of a row, but it may not exist when used in callbacks.

Expected behavior

Throwing an error is perhaps a bit overkill in this situation. The row is expected not to be there, so a defensive return would seem like a more sane solution in this case IMO.

Context

Some of our customers have slow machines where React is a big of a hog to render certain scenarios and they typically operate faster than what the tablet (in this case) is capable of handling. The effect is slow callbacks that are rendered later than expected and, particularly, onMouseUp is scheduled after a row has been removed in many cases. We mostly see the reports in Rollbar and don't experience it much ourselves. The thrown exception is not critical and doesn't break anything, but it's rather noisy.

Your environment

Google Chrome on MacOS

envinfo is N/A (the referenced video is on the current demo site)

Search keywords: row error callback handler onMouseUp cell Order ID: 78367

michelengelen commented 3 weeks ago

Thanks for raising this @martinjlowm ... one question: how did you achieve that result? I did try throttling the CPU in chrome (6x throttling) and although it loads very slowly it still does not throw an error.

@romgrk do you think we could just use the suggestion here?

romgrk commented 2 weeks ago

Not sure why we were throwing an error to start with, maybe @cherniavskii has more context? I think it would be fine to remove the throw.

github-actions[bot] commented 2 weeks ago

The issue has been inactive for 7 days and has been automatically closed.