[data grid] onPaginationModelChange details property is not defined / useful in v7+

[bjackson42]

Steps to reproduce

Link to live example: (required)

v6 Pagination
v7 Pagination

Steps:

1. Go to v6 MUI DataGridPro pagination page and scroll down to Controlled pagination model
2. Click "Expand Code"
3. Update the onPaginationModel property to the following:

onPaginationModelChange={(model, details) => {
    console.log(details);
            
    setPaginationModel(model)
  }
}

5. Open Developer Console (F12)
6. Click the paging buttons on the example
7. Take note that the console log says { reason: 'setPaginationModel' }
8. Repeat these steps for v7
9. Take note that the console log says { reason: undefined, api: {...} }

Current behavior

The details property is not defined when using the Grid pagination controls.

Expected behavior

The details property should have a string value for reason when using the Grid pagination controls.

Context

In v6 the DataGridPro would give a reason when using the Grid pagination controls. This made it possible to distinguish between a manual pagination model change versus a Grid pagination control model change.

An example of a manual pagination change is an Add button in the Grid toolbar that adds a new row. When pagination is enabled and a new row is added we should take the user to the page where the new row resides. If pageSize is 5 and a 6th row is added we should navigate the user to the second page with the new row. This was possible in v6 because the reason would be undefined and a developer could tell when a manual pagination model change had been made. Otherwise a reason is given and we honor the model suggested by onPaginationModelChange.

In v7 the details property value has changed from { reason: string } to { reason: undefined, api: {...} }. This caused a major breakage in our implementation and we have had to come up with a custom solution to get around the issue. To make the situation worse, there was no notice given about this change and I cannot find any record of this change.

Please advise, is there an alternative method? Should I just use the API even though the documentation suggests not to?

Your environment

npx @mui/envinfo

Chrome (latest)
GitBash (latest)

System:
    OS: Windows 11 10.0.22631
  Binaries:
    Node: 20.9.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.1.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: Not Found
  Browsers:
    Chrome: Not Found
    Edge: Chromium (123.0.2420.97)
  npmPackages:
    @emotion/react: 11.11.4 => 11.11.4
    @emotion/styled: 11.11.5 => 11.11.5
    @mui/base:  5.0.0-beta.40
    @mui/core-downloads-tracker:  5.15.15
    @mui/icons-material: 5.15.15 => 5.15.15
    @mui/material: 5.15.15 => 5.15.15
    @mui/private-theming:  5.15.14
    @mui/styled-engine:  5.15.14
    @mui/system: 5.15.15 => 5.15.15
    @mui/types:  7.2.14
    @mui/utils:  5.15.14
    @mui/x-data-grid:  7.3.0
    @mui/x-data-grid-pro: 7.3.0 => 7.3.0
    @mui/x-date-pickers: 7.1.1 => 7.1.1
    @mui/x-date-pickers-pro: 7.1.1 => 7.1.1
    @mui/x-license:  7.1.1
    @mui/x-license-pro: 6.10.2 => 6.10.2
    @types/react: 18.2.75 => 18.2.75
    react: 18.2.0 => 18.2.0
    react-dom: 18.2.0 => 18.2.0
    typescript: 5.4.4 => 5.4.4

Search keywords: grid pagination details bug
Order ID: 83568

[michelengelen]

Hey @bjackson42 ... this is indeed a slight regression on our side. The responsible commit is this: 7c4ba74

we can easily add this back in. Here is a diff for that change to start off:

diff --git a/packages/x-data-grid/src/hooks/features/pagination/useGridPaginationModel.ts b/packages/x-data-grid/src/hooks/features/pagination/useGridPaginationModel.ts
index c482999e4..57e7deece 100644
--- a/packages/x-data-grid/src/hooks/features/pagination/useGridPaginationModel.ts
+++ b/packages/x-data-grid/src/hooks/features/pagination/useGridPaginationModel.ts
@@ -115,17 +115,20 @@ export const useGridPaginationModel = (
       }
       logger.debug("Setting 'paginationModel' to", paginationModel);

-      apiRef.current.setState((state) => ({
-        ...state,
-        pagination: {
-          ...state.pagination,
-          paginationModel: getDerivedPaginationModel(
-            state.pagination,
-            props.signature,
-            paginationModel,
-          ),
-        },
-      }));
+      apiRef.current.setState(
+        (state) => ({
+          ...state,
+          pagination: {
+            ...state.pagination,
+            paginationModel: getDerivedPaginationModel(
+              state.pagination,
+              props.signature,
+              paginationModel,
+            ),
+          },
+        }),
+        'setPaginationModel',
+      );
     },
     [apiRef, logger, props.signature],
   );

I will put this up for grabs for the community since it is a rather small change.
Feel free to take this up yourself!
Thanks again for raising this with us! 🙇🏼

[bjackson42]

Thank you for the quick reply @michelengelen! I would love for the opportunity to contribute and will definitely make a PR for this. Thank you again for the fast support and details on the issue. I really appreciate the code block showing the diffs, that is awesome.

Have a wonderful and fantastic day!

[bjackson42]

PR created -> #12923