Skip to content

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 8, 2025

Fixes #19046

Before: https://stackblitz.com/edit/axg8fqmk?file=src%2FDemo.tsx
After: https://stackblitz.com/edit/54tkqjct?file=src%2FDemo.tsx

  • select all then deselect each row, it should work.

The Bug

When using a controlled exclude model with tree data and row selection propagation, the grid corrupts the model by treating excluded row IDs as selected row IDs.

Step-by-Step Bug Flow

1. User provides controlled exclude model

<DataGridPro
  treeData
  rowSelectionModel={{ type: 'exclude', ids: new Set([1]) }}
  rowSelectionPropagation={{ descendants: true, parents: true }}
  onRowSelectionModelChange={handleChange}
/>

Meaning: "All rows selected EXCEPT row 1 (Thomas)"

2. Grid calls getPropagatedRowSelectionModel

When the controlled prop is provided, syncControlledState (line 849) calls:

const newSelectionModel = apiRef.current.getPropagatedRowSelectionModel(propRowSelectionModel);

3. Bug: Function treats excluded IDs as selected IDs

Before Fix (lines 436-466):

const getPropagatedRowSelectionModel = (inputSelectionModel) => {
  // No check for exclude model type! ❌
  if (!isNestedData || !applyAutoSelection || ...) {
    return inputSelectionModel;
  }

  const propagatedSelectionModel = {
    type: inputSelectionModel.type,  // 'exclude'
    ids: new Set(inputSelectionModel.ids),  // Set([1]) - EXCLUDED rows!
  };

  const selectionManager = createRowSelectionManager(propagatedSelectionModel);

  // Iterate over EXCLUDED row IDs ❌
  for (const id of inputSelectionModel.ids) {  // id = 1 (Thomas)
    findRowsToSelect(  // ❌ Wrong function for excluded rows!
      apiRef, tree, id,
      props.rowSelectionPropagation?.descendants,  // true
      props.rowSelectionPropagation?.parents,  // true
      (rowId) => selectionManager.select(rowId),  // ❌ Removes from exclude set!
    );
  }
};

4. What happens inside the propagation

Tree structure:

Thomas (id: 1) - EXCLUDED
  ├─ Robert (id: 2)
  ├─ Karen (id: 3)
  └─ Nancy (id: 4)

Propagation flow:

  1. findRowsToSelect is called with id = 1 (Thomas - an EXCLUDED row)
  2. It finds children: [2, 3, 4]
  3. For each child, it calls: selectionManager.select(childId)

What select() does for exclude models:

// In gridRowSelectionManager.ts
select(id) {
  if (this.model.type === 'exclude') {
    this.model.ids.delete(id);  // ❌ Removes from exclude set!
  }
}

5. The corruption

Starting model:

{ type: 'exclude', ids: Set([1]) }
// Meaning: "All selected except Thomas"

After propagation:

{ type: 'exclude', ids: Set() }
// Meaning: "All rows selected!" ❌

What happened:

  • Row 1 (Thomas) was in the exclude set
  • select() was called on its children (2, 3, 4)
  • Since they weren't in the exclude set, nothing was deleted
  • But the semantic error is: we iterated over excluded rows and called findRowsToSelect on them!

The Root Cause

Semantic mismatch:

  • findRowsToSelect is designed for SELECTION propagation
  • Exclude model IDs represent UNSELECTED rows
  • Calling findRowsToSelect on unselected rows and using select() to "propagate" them corrupts the model

Why exclude models don't need propagation:

When a user actually deselects a row via UI interaction, propagation ALREADY happens correctly:

// In selectRow (lines 294-302)
} else if (applyAutoSelection) {
  findRowsToDeselect(  // ✅ Correct function
    apiRef, tree, id,
    props.rowSelectionPropagation?.descendants,
    props.rowSelectionPropagation?.parents,
    (rowId) => selectionManager.unselect(rowId),  // ✅ Adds to exclude set
  );
}

Result after UI deselection:

{ type: 'exclude', ids: Set([1, 2, 3, 4]) }  // ✅ Already complete!

The model is semantically complete - no "propagation reapplication" needed.

The Solution

Skip propagation for exclude models in getPropagatedRowSelectionModel

Location: useGridRowSelection.ts:440

Change:

const getPropagatedRowSelectionModel = (inputSelectionModel) => {
  if (
    !isNestedData ||
    !applyAutoSelection ||
    inputSelectionModel.type === 'exclude' ||  // ← NEW: Skip for exclude models
    (inputSelectionModel.ids.size === 0 && inputSelectionModel.type === 'include')
  ) {
    return inputSelectionModel;  // Return unchanged
  }

  // Propagation logic below only runs for include models now
  // ...
};

Test

Added a test to ensure that the exclude model is unchanged when getting from getPropagatedRowSelectionModel


@siriwatknp siriwatknp added type: bug It doesn't behave as expected. scope: data grid Changes related to the data grid. feature: Selection Related to the data grid Selection feature labels Oct 8, 2025
@mui-bot
Copy link

mui-bot commented Oct 8, 2025

Deploy preview: https://deploy-preview-19846--material-ui-x.netlify.app/

Bundle size report

Bundle Parsed size Gzip size
@mui/x-data-grid 🔺+20B(+0.01%) 🔺+1B(0.00%)
@mui/x-data-grid-pro 🔺+20B(0.00%) 🔺+2B(0.00%)
@mui/x-data-grid-premium 🔺+20B(0.00%) 🔺+3B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 440a1be

@siriwatknp siriwatknp marked this pull request as ready for review October 8, 2025 06:37
@siriwatknp siriwatknp requested a review from a team October 8, 2025 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Selection Related to the data grid Selection feature scope: data grid Changes related to the data grid. type: bug It doesn't behave as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Unable to unselect treeData rows after using "Select all" checkbox with controlled row selection
2 participants