[DataGrid] Fix setPage not working (#284)

* fix #229 setPage not reliable

* fix #228 issue with first page trigger a pageChange event

* Added test for #229

* Apply suggestions from code review

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

* move api test to xgrid

* fix prettier

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

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

@@ -58,6 +58,30 @@ describe('<DataGrid />', () => {
           container.firstChild.firstChild.firstChild,
         );
       });
+
+      it('should apply the page prop correctly', () => {
+        const rows = [
+          {
+            id: 0,
+            brand: 'Nike',
+          },
+          {
+            id: 1,
+            brand: 'Addidas',
+          },
+          {
+            id: 2,
+            brand: 'Puma',
+          },
+        ];
+        render(
+          <div style={{ width: 300, height: 300 }}>
+            <DataGrid rows={rows} columns={defaultProps.columns} page={2} pageSize={1} />
+          </div>,
+        );
+        const cell = document.querySelector('[role="cell"][aria-colindex="0"]')!;
+        expect(cell).to.have.text('Addidas');
+      });
     });
   });
 

packages/grid/x-grid-modules/src/hooks/features/usePagination.ts

@@ -61,6 +61,7 @@ export const usePagination = (
     ),
   };
   const stateRef = React.useRef(initialState);
+  const prevPageRef = React.useRef<number>(1);
   const [state, dispatch] = React.useReducer(paginationReducer, initialState);
 
   const updateState = React.useCallback(
@@ -74,16 +75,22 @@ export const usePagination = (
 
   const setPage = React.useCallback(
     (page: number) => {
-      page = stateRef.current.pageCount >= page ? page : stateRef.current.pageCount;
-      apiRef.current.renderPage(
-        stateRef.current.paginationMode === FeatureModeConstant.client ? page : 1,
-      );
-
+      let hasPageChanged = false;
+      if (stateRef.current.rowCount > 0) {
+        page = stateRef.current.pageCount >= page ? page : stateRef.current.pageCount;
+        apiRef.current.renderPage(
+          stateRef.current.paginationMode === FeatureModeConstant.client ? page : 1,
+        );
+        hasPageChanged = true;
+      }
       const params: PageChangeParams = {
         ...stateRef.current,
         page,
       };
-      apiRef.current.publishEvent(PAGE_CHANGED, params);
+      if (hasPageChanged && prevPageRef.current !== page) {
+        apiRef.current.publishEvent(PAGE_CHANGED, params);
+        prevPageRef.current = page;
+      }
       if (stateRef.current.page !== page) {
         updateState({ page });
       }
@@ -147,6 +154,17 @@ export const usePagination = (
     }
   }, [setPageSize, logger, getAutoPageSize]);
 
+  useApiEventHandler(apiRef, PAGE_CHANGED, options.onPageChange);
+  useApiEventHandler(apiRef, PAGESIZE_CHANGED, options.onPageSizeChange);
+
+  const onResize = React.useCallback(() => {
+    if (options.autoPageSize) {
+      resetAutopageSize();
+    }
+  }, [options.autoPageSize, resetAutopageSize]);
+
+  useApiEventHandler(apiRef, RESIZE, onResize);
+
   React.useEffect(() => {
     stateRef.current = state;
   }, [state]);
@@ -157,24 +175,14 @@ export const usePagination = (
     }
   }, [apiRef, stateRef, apiRef.current?.isInitialised]);
 
-  React.useEffect(() => {
-    updateState({ paginationMode: options.paginationMode! });
-  }, [options.paginationMode, updateState]);
-
-  React.useEffect(() => {
-    setPage(options.page != null ? options.page : 1);
-  }, [options.page, setPage]);
-
   React.useEffect(() => {
     const rowCount = options.rowCount == null ? rows.length : options.rowCount;
     if (rowCount !== state.rowCount) {
       logger.info(`Options or rows change, recalculating pageCount and rowCount`);
       const newPageCount = getPageCount(state.pageSize, rowCount);
 
       updateState({ pageCount: newPageCount, rowCount });
-      if (state.page > newPageCount) {
-        setPage(newPageCount);
-      }
+      setPage(state.page);
     }
   }, [
     rows.length,
@@ -187,6 +195,14 @@ export const usePagination = (
     state.page,
   ]);
 
+  React.useEffect(() => {
+    updateState({ paginationMode: options.paginationMode! });
+  }, [options.paginationMode, updateState]);
+
+  React.useEffect(() => {
+    setPage(options.page != null ? options.page : 1);
+  }, [options.page, setPage]);
+
   React.useEffect(() => {
     if (
       !options.autoPageSize &&
@@ -203,17 +219,6 @@ export const usePagination = (
     }
   }, [options.autoPageSize, resetAutopageSize, columns.visible.length]);
 
-  useApiEventHandler(apiRef, PAGE_CHANGED, options.onPageChange);
-  useApiEventHandler(apiRef, PAGESIZE_CHANGED, options.onPageSizeChange);
-
-  const onResize = React.useCallback(() => {
-    if (options.autoPageSize) {
-      resetAutopageSize();
-    }
-  }, [options.autoPageSize, resetAutopageSize]);
-
-  useApiEventHandler(apiRef, RESIZE, onResize);
-
   const paginationApi: PaginationApi = {
     setPageSize,
     setPage,

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

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { screen, createClientRender, act, fireEvent } from 'test/utils';
 import { expect } from 'chai';
-import { XGrid } from '@material-ui/x-grid';
+import { XGrid, useApiRef } from '@material-ui/x-grid';
 import { useData } from 'packages/storybook/src/hooks/useData';
 
 function getActiveCell() {
@@ -212,6 +212,42 @@ describe('<XGrid />', () => {
       expect(row).to.not.have.class('Mui-selected');
       expect(checkbox).to.have.property('checked', false);
     });
+    it('should apply setPage correctly', () => {
+      const rows = [
+        {
+          id: 0,
+          brand: 'Nike',
+        },
+        {
+          id: 1,
+          brand: 'Addidas',
+        },
+        {
+          id: 2,
+          brand: 'Puma',
+        },
+      ];
+      const GridTest = () => {
+        const apiRef = useApiRef();
+        React.useEffect(() => {
+          apiRef.current.setPage(2);
+        });
+        return (
+          <div style={{ width: 300, height: 300 }}>
+            <XGrid
+              rows={rows}
+              apiRef={apiRef}
+              columns={defaultProps.columns}
+              pagination
+              pageSize={1}
+            />
+          </div>
+        );
+      };
+      render(<GridTest />);
+      const cell = document.querySelector('[role="cell"][aria-colindex="0"]')!;
+      expect(cell).to.have.text('Addidas');
+    });
   });
 
   describe('sorting', () => {

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

@@ -264,3 +264,55 @@ export function ServerPaginationWithEventHandler() {
     </div>
   );
 }
+export function Page1Prop() {
+  const data = useData(2000, 200);
+
+  return (
+    <div className="grid-container">
+      <XGrid
+        rows={data.rows}
+        columns={data.columns}
+        pagination
+        pageSize={50}
+        onPageChange={(p) => action('pageChange')(p)}
+      />
+    </div>
+  );
+}
+export function Page2Prop() {
+  const data = useData(2000, 200);
+
+  return (
+    <div className="grid-container">
+      <XGrid
+        rows={data.rows}
+        columns={data.columns}
+        pagination
+        pageSize={50}
+        page={2}
+        onPageChange={(p) => action('pageChange')(p)}
+      />
+    </div>
+  );
+}
+export function Page2Api() {
+  const data = useData(2000, 200);
+  const apiRef = useApiRef();
+
+  React.useEffect(() => {
+    apiRef.current.setPage(2);
+  }, [apiRef]);
+
+  return (
+    <div className="grid-container">
+      <XGrid
+        apiRef={apiRef}
+        rows={data.rows}
+        columns={data.columns}
+        pagination
+        pageSize={50}
+        onPageChange={(p) => action('pageChange')(p)}
+      />
+    </div>
+  );
+}