[DataGrid] Changing pages causes onSelectionModelChange prop to be invoked

[willsoto]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Current behavior 😯

The current behavior makes it so that a controlled selection is getting "blown away" when the page changes thus making retaining the selectionModel difficult / impossible. I am following the example in the documentation and am on version 5.10.0

Demo taken from the documentation with a console.log added to show state is not being retained when changing pages:
https://codesandbox.io/s/controlledselectionserverpaginationgrid-material-demo-forked-6whmr8?file=/demo.tsx

Seems like it may be a regression or related to #2136

Expected behavior 🤔

When the page changes, the selection model callback should either:

1. Be invoked with the provided selection model
2. Not invoked at all (but I do understand why it's being invoked)

Steps to reproduce 🕹

https://codesandbox.io/s/controlledselectionserverpaginationgrid-material-demo-forked-6whmr8?file=/demo.tsx

Follow the steps / code described in the documented example to see that it no longer works when opened separately (although it is unclear to me why it seems to be working on the documentation itself)

1. Make a selection on page 1

2. Go to page 2 and notice that the bottom does not show any rows being selected. Selecting all rows on page will show 5 rows selected (the demo shows the total selected across all pages)

3. Go back to page 1 and notice no selection have persisted.

I have added console.log statements so you can see the onSelectionModelChange calls

Context 🔦

I am trying to retain selections when changing pages using server side pagination.

Your environment 🌎

`npx @mui/envinfo`

  System:
    OS: macOS 12.3.1
  Binaries:
    Node: 16.13.2 - ~/.volta/tools/image/node/16.13.2/bin/node
    Yarn: 1.22.17 - ~/.volta/tools/image/yarn/1.22.17/bin/yarn
    npm: 7.24.2 - ~/code/project/node_modules/.bin/npm
  Browsers:
    Chrome: 100.0.4896.127
    Edge: Not Found
    Firefox: 99.0.1
    Safari: 15.4
  npmPackages:
    @emotion/react: ~11.8.1 => 11.8.1
    @emotion/styled: ~11.8.1 => 11.8.1
    @mui/base:  5.0.0-alpha.78
    @mui/icons-material: ~5.6.2 => 5.6.2
    @mui/lab: ~5.0.0-alpha.79 => 5.0.0-alpha.79
    @mui/material: ~5.6.3 => 5.6.3
    @mui/private-theming:  5.6.2
    @mui/styled-engine:  5.6.1
    @mui/styles: ~5.6.2 => 5.6.2
    @mui/system:  5.6.3
    @mui/types:  7.1.3
    @mui/utils:  5.6.1
    @mui/x-data-grid: ~5.10.0 => 5.10.0
    @mui/x-date-pickers:  5.0.0-alpha.0
    @types/react: ~17.0.39 => 17.0.39
    react: ~17.0.2 => 17.0.2
    react-dom: ~17.0.2 => 17.0.2
    typescript: ~4.5.5 => 4.5.5

Using Firefox: 99.0.1

Order ID 💳 (optional)

No response

[m4theushw]

I tried to reproduce the steps in the example you provided but I ended on a different result. When switching back to the first page, it shows "10 rows selected" but called onSelectionModelChange with 5 items only. Could you verify if you also see this?

I found some other weird bugs related to the selected row count. It seems that it's using some cached value and doesn't update when selectionModel changes.

onSelectionModelChange is called with an empty array but display 5 rows selected:

onSelectionModelChange is called with 5 items but shows 10 rows selected (this is what I refer in the first paragraph):

[willsoto]

I reproduced the behavior I saw in the OP. Decided to make a video:

Kapture.2022-04-29.at.08.02.35.mp4

Are we seeing a difference in behavior because it's pulling in different versions? What I see locally matches the behavior I see in the codesandbox.

[willsoto]

@m4theushw I am happy to contribute a fix for this. We would just need to discuss what the correct behavior should be before I begin implementation.

[m4theushw]

If I go a couple more pages then I see the empty row count. This bug happens in part because the way this specific demo was built but also due to the fact that the grid always tries to sanitize the selection model, keeping in it only the rows that exist in the rows prop.

In our demo, there's a setTimeout that updates the selection model with the previous model every time that the page is changed. We added it to fight the default sanitization and keep the selected rows from the previous page. However, at some point, the previous model becomes [] and cleans the current one. Logging the current model helps to understand the problem.

One way to fix this would be introduce a prop to allow in selectionModel rows that might not exist in the rows prop. Basically we do:

diff --git a/packages/grid/x-data-grid/src/hooks/features/selection/useGridSelection.ts b/packages/grid/x-data-grid/src/hooks/features/selection/useGridSelection.ts
index a2bb5eac4..7c661724d 100644
--- a/packages/grid/x-data-grid/src/hooks/features/selection/useGridSelection.ts
+++ b/packages/grid/x-data-grid/src/hooks/features/selection/useGridSelection.ts
@@ -265,6 +265,10 @@ export const useGridSelection = (
    * EVENTS
    */
   const removeOutdatedSelection = React.useCallback(() => {
+    if (props.keepNonExistentRowsSelected) {
+      return;
+    }
+
     const currentSelection = gridSelectionStateSelector(apiRef.current.state);
     const rowsLookup = gridRowsLookupSelector(apiRef);

Then it would be up to you to clean selectionModel when it doesn't make more sense to keep previously selected rows. What do you think?

[willsoto]

That seems like a reasonable change. Would you like me to open a PR with those changes?

[m4theushw]

Feel free to open a PR proposing the change, or we can leave for someone from the X team or community. To test the new prop we only need to create a test case based on the below one but with the behavior inverted:

mui-x/packages/grid/x-data-grid/src/tests/selection.DataGrid.test.tsx

Lines 560 to 561 in e7f9f56

	describe('prop: rows', () => {
	it('should remove the outdated selected rows when rows prop changes', () => {

[willsoto]

Opened #4786

[ozanmanav]

Hi @willsoto, How can we fix this issue for MUI v4? So is there any workaround with refs?

[HusnainRafiq12]

Hi @ozanmanav, Did you get the solution for MUI V4?