[DataGrid] Fix crash when updating columns immediately after scrolling #13781
Conversation
cherniavskii
added
component: data grid
|
Deploy preview: https://deploy-preview-13781--material-ui-x.netlify.app/ |
| if (!column) { | ||
| // See https://github.com/mui/mui-x/issues/13719 | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This feels like it hides something.
| if (!column) { | |
| // See https://github.com/mui/mui-x/issues/13719 | |
| return null; | |
| } |
Looking a bit at it, it seems that the issue is that the frozen render context isn't reset when the column changes.
diff --git a/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx b/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
index 334d65759..d87187caa 100644
--- a/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
+++ b/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
@@ -277,6 +277,7 @@ export const useGridVirtualScroller = () => {
const forceUpdateRenderContext = () => {
const inputs = inputsSelector(apiRef, rootProps, enabled, enabledForColumns);
const nextRenderContext = computeRenderContext(inputs, scrollPosition.current, scrollCache);
+ // Reset the frozen context when the render context changes, see the illustration in https://github.com/mui/mui-x/pull/12353
+ frozenContext.current = undefined;
updateRenderContext(nextRenderContext);
};
|
|
||
| await page.click('"Update columns"'); | ||
|
|
||
| await sleep(200); |
There was a problem hiding this comment.
is there a way to use some grid's callback to check if the error happened instead of waiting for some fixed time?
There was a problem hiding this comment.
It feels like an end to end test is a bit too heavy (slow) for this bug. It seems that to reproduce, it's a matter of scrolling down to create a frozen context and update the columns to break part of the frozen context. So I would expect a Karma test to run significantly faster (would be curious to measure the test speed difference, maybe I'm dillusional).
It seems to break instantly as well, without the need to sleep (needs an act()).
| await page.mouse.move(200, 200); | ||
| await page.mouse.wheel(0, 1000); | ||
|
|
||
| let thrownError: Error | null = null; | ||
| context.once('weberror', (webError: WebError) => { | ||
| thrownError = webError.error(); | ||
| console.error(thrownError); | ||
| }); | ||
|
|
||
| await page.click('"Update columns"'); |
There was a problem hiding this comment.
Nitpick but I would group the page interactions together, shows more clearly the sequence of events that trigger the bug.
Fixes #13719
I believe this regression was introduced in #12353
I cannot reproduce the issue if I change this timeout to 100:
mui-x/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
Line 272 in c6d8bf4