[DataGrid] Reset sortedRows state on prop change (#599)

* Fix #575 reset sortedRows state on prop change

* fix typescript issue

* fix lint

* prettier

* update dep array

* polish tests

* Update packages/storybook/src/stories/grid-sorting.stories.tsx

Co-authored-by: Olivier Tassinari <[email protected]>

* Update packages/storybook/src/stories/grid-sorting.stories.tsx

Co-authored-by: Olivier Tassinari <[email protected]>

* avoids confusion, sort is not immutable

* cleanup story

* refactor applySorting

* fix lint

Co-authored-by: Olivier Tassinari <[email protected]>

Author: dtassone

packages/grid/_modules_/grid/GridComponent.tsx

@@ -73,7 +73,7 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps
     useRows(props.rows, apiRef);
     useKeyboard(rootContainerRef, apiRef);
     useSelection(apiRef);
-    useSorting(apiRef);
+    useSorting(apiRef, props.rows);
 
     useContainerProps(windowRef, apiRef);
     const renderCtx = useVirtualRows(columnsHeaderRef, windowRef, renderingZoneRef, apiRef);

packages/grid/_modules_/grid/hooks/features/sorting/useSorting.ts

@@ -13,7 +13,7 @@ import { FeatureModeConstant } from '../../../models/featureMode';
 import { CellParams } from '../../../models/params/cellParams';
 import { ColParams } from '../../../models/params/colParams';
 import { SortModelParams } from '../../../models/params/sortModelParams';
-import { RowModel } from '../../../models/rows';
+import { RowModel, RowsProp } from '../../../models/rows';
 import { FieldComparatorList, SortItem, SortModel } from '../../../models/sortModel';
 import { buildCellParams } from '../../../utils/paramsUtils';
 import { isDesc, nextSortDirection } from '../../../utils/sortingUtils';
@@ -26,10 +26,10 @@ import { columnsSelector } from '../columns/columnsSelector';
 import { GridState } from '../core/gridState';
 import { useGridSelector } from '../core/useGridSelector';
 import { useGridState } from '../core/useGridState';
-import { rowCountSelector, unorderedRowModelsSelector } from '../rows/rowsSelector';
+import { rowCountSelector } from '../rows/rowsSelector';
 import { SortingState } from './sortingState';
 
-export const useSorting = (apiRef: ApiRef) => {
+export const useSorting = (apiRef: ApiRef, rowsProp: RowsProp) => {
   const logger = useLogger('useSorting');
   const allowMultipleSorting = React.useRef<boolean>(false);
   const comparatorList = React.useRef<FieldComparatorList>([]);
@@ -38,7 +38,6 @@ export const useSorting = (apiRef: ApiRef) => {
   const options = useGridSelector(apiRef, optionsSelector);
   const columns = useGridSelector(apiRef, columnsSelector);
   const rowCount = useGridSelector(apiRef, rowCountSelector);
-  const unorderedRows = useGridSelector(apiRef, unorderedRowModelsSelector);
 
   const getSortModelParams = React.useCallback(
     (sortModel: SortModel): SortModelParams => ({
@@ -127,13 +126,14 @@ export const useSorting = (apiRef: ApiRef) => {
   );
 
   const applySorting = React.useCallback(() => {
+    const rowModels = apiRef.current.getRowModels();
     const sortModel = apiRef.current.getState<GridState>().sorting.sortModel;
     logger.info('Sorting rows with ', sortModel);
 
-    let sorted = [...unorderedRows];
+    const sorted = [...rowModels];
     if (sortModel.length > 0) {
       comparatorList.current = buildComparatorList(sortModel);
-      sorted = sorted.sort(comparatorListAggregate);
+      sorted.sort(comparatorListAggregate);
     }
 
     setGridState((oldState) => {
@@ -143,15 +143,7 @@ export const useSorting = (apiRef: ApiRef) => {
       };
     });
     forceUpdate();
-  }, [
-    apiRef,
-    logger,
-    unorderedRows,
-    setGridState,
-    forceUpdate,
-    buildComparatorList,
-    comparatorListAggregate,
-  ]);
+  }, [apiRef, logger, setGridState, forceUpdate, buildComparatorList, comparatorListAggregate]);
 
   const setSortModel = React.useCallback(
     (sortModel: SortModel) => {
@@ -236,6 +228,11 @@ export const useSorting = (apiRef: ApiRef) => {
   const sortApi: SortApi = { getSortModel, setSortModel, onSortModelChange, applySorting };
   useApiMethod(apiRef, sortApi, 'SortApi');
 
+  React.useEffect(() => {
+    // When the rows prop change, we re apply the sorting.
+    apiRef.current.applySorting();
+  }, [apiRef, rowsProp]);
+
   React.useEffect(() => {
     if (rowCount > 0 && options.sortingMode === FeatureModeConstant.client) {
       logger.debug('row changed, applying sortModel');
@@ -268,7 +265,8 @@ export const useSorting = (apiRef: ApiRef) => {
 
   React.useEffect(() => {
     const sortModel = options.sortModel || [];
-    if (sortModel.length > 0) {
+    const oldSortModel = apiRef.current.state.sorting.sortModel;
+    if (sortModel.length > 0 && !isEqual(sortModel, oldSortModel)) {
       // we use apiRef to avoid watching setSortModel as it will trigger an update on every state change
       apiRef.current.setSortModel(sortModel);
     }

packages/grid/data-grid/src/DataGrid.test.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import PropTypes from 'prop-types';
 // @ts-expect-error need to migrate helpers to TypeScript
-import { createClientRender, ErrorBoundary } from 'test/utils';
+import { createClientRender, fireEvent, screen, ErrorBoundary } from 'test/utils';
 import { useFakeTimers } from 'sinon';
 import { expect } from 'chai';
 import { DataGrid } from '@material-ui/data-grid';
@@ -266,6 +266,66 @@ describe('<DataGrid />', () => {
         expect(getColumnValues()).to.deep.equal(['Puma', 'Nike', 'Adidas'].reverse());
       });
     });
+
+    describe('sorting', () => {
+      it('should sort when clicking the header cell', () => {
+        render(
+          <div style={{ width: 300, height: 300 }}>
+            <DataGrid {...defaultProps} />
+          </div>,
+        );
+        const header = screen.getByRole('columnheader', { name: 'brand' });
+        expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
+        fireEvent.click(header);
+        expect(getColumnValues()).to.deep.equal(['Adidas', 'Nike', 'Puma']);
+        fireEvent.click(header);
+        expect(getColumnValues()).to.deep.equal(['Puma', 'Nike', 'Adidas']);
+      });
+
+      it('should keep rows sorted when rows prop change', () => {
+        interface TestCaseProps {
+          rows: any[];
+        }
+        const TestCase = (props: TestCaseProps) => {
+          const { rows } = props;
+          return (
+            <div style={{ width: 300, height: 300 }}>
+              <DataGrid
+                {...defaultProps}
+                rows={rows}
+                sortModel={[
+                  {
+                    field: 'brand',
+                    sort: 'asc',
+                  },
+                ]}
+              />
+            </div>
+          );
+        };
+
+        const { setProps } = render(<TestCase rows={defaultProps.rows} />);
+        expect(getColumnValues()).to.deep.equal(['Adidas', 'Nike', 'Puma']);
+
+        setProps({
+          rows: [
+            {
+              id: 3,
+              brand: 'Asics',
+            },
+            {
+              id: 4,
+              brand: 'RedBull',
+            },
+            {
+              id: 5,
+              brand: 'Hugo',
+            },
+          ],
+        });
+        expect(getColumnValues()).to.deep.equal(['Asics', 'Hugo', 'RedBull']);
+      });
+    });
   });
 
   describe('warnings', () => {

packages/grid/x-grid/src/XGrid.test.tsx

@@ -206,22 +206,6 @@ describe('<XGrid />', () => {
     });
   });
 
-  describe('sorting', () => {
-    it('should sort when clicking the header cell', () => {
-      render(
-        <div style={{ width: 300, height: 300 }}>
-          <XGrid {...defaultProps} />
-        </div>,
-      );
-      const header = screen.getByRole('columnheader', { name: 'brand' });
-      expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']);
-      fireEvent.click(header);
-      expect(getColumnValues()).to.deep.equal(['Adidas', 'Nike', 'Puma']);
-      fireEvent.click(header);
-      expect(getColumnValues()).to.deep.equal(['Puma', 'Nike', 'Adidas']);
-    });
-  });
-
   describe('state', () => {
     it('should trigger on state change and pass the correct params', () => {
       let onStateParams;

packages/storybook/src/stories/grid-sorting.stories.tsx

@@ -1,3 +1,5 @@
+import { Button } from '@material-ui/core';
+import { randomInt } from '@material-ui/x-grid-data-generator';
 import * as React from 'react';
 import {
   ColDef,
@@ -432,3 +434,36 @@ export const ServerSideSorting = () => {
     </div>
   );
 };
+
+export const ResetSortingRows = () => {
+  const columns = [
+    {
+      field: 'name',
+      width: 200,
+    },
+    {
+      field: 'team',
+      width: 200,
+      type: 'number',
+    },
+  ];
+  const [rows, setRows] = React.useState<RowsProp>([]);
+
+  const createRandomRows = () => {
+    const randomRows: any[] = [];
+
+    for (let i = 0; i < 10; i += 1) {
+      const id = randomInt(0, 100000).toString();
+      randomRows.push({ id, name: 'name test', team: id });
+    }
+
+    setRows(randomRows);
+  };
+
+  return (
+    <div className="grid-container" style={{ flexDirection: 'column' }}>
+      <Button onClick={() => createRandomRows()}>Random Rows</Button>
+      <XGrid rows={rows} columns={columns} sortModel={[{ field: 'team', sort: 'asc' }]} />
+    </div>
+  );
+};