[data grid] After setting loading to false, DataGrid resets to the initial row status

[dstucki]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Current behavior 😯

Calls to apiRef.current.setRows are being reseted when setting loaded to false.

Probably related to #5208?

Expected behavior 🤔

rows set with setRows should be displayed even after call to setLoading

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/data-grid-pro-forked-d34r64

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo

  Chrome, Safari, Firefox.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

31174

[DanailH]

Hi, @dstucki thanks for raising this! Can you explain a bit more what the problem is I checked your reproduction and it seems to be working as expected: you start off with 1 row, then you set the loading to true, call the setRows and provide a different row, and set the loading to false and the row that is being displayed is the one that you provided through the setRows?

We also advise people to memoize their rows to persist the state between renders. There is a warning here https://mui.com/x/react-data-grid/row-definition/#feeding-data

[dstucki]

@DanailH thanks for your questions.

I accidentally changed the codepen example after creating the issue. Now it is back in the original state which represents the error. (Accidental change was wrapping setRows with setTimeout)

For our use case, we use server-side filtering and sorting. Do we still need to memoize the rows?

[DanailH]

Ah yes, now the issue is clearly visible, thanks.

Regarding the server-side filtering and sorting - It's aways adviced to memoize the rows to avoid unnecessary rerenders of the grid when your data changes. That being said it also depends on how you fetch your data. If you use REST then you can update the rows when you receive the data and the grid will respect that. If you use a solution like MobX or observables then it can be a bit more tricky. There is https://github.com/mui/mui-x/pull/5711 that explains the problem. We fixed it so that it won't throw if the rows you are providing can't be frozen due to an external dependency.

[dstucki]

Any update on this issue?

[DanailH]

@dstucki we haven't looked into the problem yet. We will try and prioritize it. I apologize for the inconvenience.

[cherniavskii]

Interesting issue! I expected the same issue to happen when using updateRows instead of setRows, but it works just fine:

• setRows: https://codesandbox.io/s/data-grid-pro-forked-x3udvh?file=/src/App.tsx
• updateRows: https://codesandbox.io/s/data-grid-pro-forked-9sinxn?file=/src/App.tsx

The workaround

Instead of using setRows method you can keep rows in a state and update that state when needed. Here's a demo: https://codesandbox.io/s/data-grid-pro-forked-u7ujol?file=/src/App.tsx

The problem

I realized there's an important difference between those two methods:

• updateRows doesn't change rowsBeforePartialUpdates property in cache https://github.com/mui/mui-x/blob/7aa156e7aad2957ce3f4e678412eb8fcfe5c06e2/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts#L191
• while setRows updates it (through createRowsInternalCache) to match the new rows: https://github.com/mui/mui-x/blob/7aa156e7aad2957ce3f4e678412eb8fcfe5c06e2/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts#L147-L148

The implication of rowsBeforePartialUpdates change is that when the loading prop changes back to false, it triggers this effect: https://github.com/mui/mui-x/blob/7aa156e7aad2957ce3f4e678412eb8fcfe5c06e2/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts#L514-L521 And areNewRowsAlreadyInState will be false, because we compare new rows provided to setRows and props.rows that hasn't changed:

apiRef.current.unstable_caches.rows.rowsBeforePartialUpdates // <== [{ id: 3, username: "okay", age: 33 }] 
props.rows // <== [{ id: 1, username: "fault", age: 1 }]

And then rows are reset to props.rows.

The solution

It looks like rowsBeforePartialUpdates has one purpose - to check whether rows prop has changed: https://github.com/mui/mui-x/blob/7aa156e7aad2957ce3f4e678412eb8fcfe5c06e2/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts#L435-L437 https://github.com/mui/mui-x/blob/7aa156e7aad2957ce3f4e678412eb8fcfe5c06e2/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts#L520-L521

So I think the solution here is to not update rowsBeforePartialUpdates in setRows:

--- a/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts
+++ b/packages/grid/x-data-grid/src/hooks/features/rows/useGridRows.ts
@@ -143,16 +143,16 @@ export const useGridRows = (
   const setRows = React.useCallback<GridRowApi['setRows']>(
     (rows) => {
       logger.debug(`Updating all rows, new length ${rows.length}`);
-      throttledRowsChange(
-        createRowsInternalCache({
-          rows,
-          getRowId: props.getRowId,
-          loading: props.loading,
-        }),
-        true,
-      );
+      const prevCache = apiRef.current.unstable_caches.rows;
+      const newCache = createRowsInternalCache({
+        rows,
+        getRowId: props.getRowId,
+        loading: props.loading,
+      });
+      newCache.rowsBeforePartialUpdates = prevCache.rowsBeforePartialUpdates;
+      throttledRowsChange(newCache, true);
     },
-    [logger, props.getRowId, props.loading, throttledRowsChange],
+    [logger, props.getRowId, props.loading, throttledRowsChange, apiRef],
   );
 
   const updateRows = React.useCallback<GridRowApi['updateRows']>(

@flaviendelangle what do you think?

[dstucki]

@cherniavskii Thanks for the detailed answer. May the difference between setRows and updateRows come from the fix of issue #5208 ? If the fix for #5208 is applied to setRows, would it behave right?

[dstucki]

Any update on this issue?