The more corner cases I fix, the fewer opportunities for the exclude selection model 😅

AFAIU it would only be for "select all", but for large datasets it would improve that interaction substantially.

Correct, but there is a number of conditions for the exclude model to be used:

if (
  !props.isRowSelectable &&
  !props.checkboxSelectionVisibleOnly &&
  applyAutoSelection &&
  !hasFilters
) {
  apiRef.current.selectRows({ type: 'exclude', ids: new Set() }, params.value, true);
} else {
  const rowsToBeSelected = getRowsToBeSelected();
  apiRef.current.selectRows(
    { type: 'include', ids: new Set(rowsToBeSelected) },
    params.value,
  );
}

I would have thought that both models would be equivalent, assuming access to the full row id list (though that implies modifying the row list requires adjusting the selection model).

Ok, so { type: 'exclude', ids: new Set() } could be interpreted differently:

1. All rows are selected (current approach)
2. All selectable and visible rows are selected (this is what you meant, right?)

Do you see benefits in (2) compared to (1)?

On another thought, there's a difference between (1) and (2) when interpreting this model by the developers to perform operations on selected rows.
In (1), an empty exclude model means literally all rows.
In (2), an empty exclude model could mean different things depending on the data grid state. So developers would need to convert empty exclude into include (?) model to be able to perform actions on the dataset.

With the above, I think we should proceed with (1).
What do you think?

Yes, I think (1) is the way to go. Both shapes of the model should be equivalent, which isn't the case for (2).

The more corner cases I fix, the fewer opportunities for the exclude selection model

It's not that it's not usable for the conditions that you've guarded for, it's that some of them might require building a non-empty exclude model. You'd need an equivalent getRowsToBeSelectedAsExclude() function. And there are some cases in which it could make sense to do that work. For example, if there are filters applied that filter a small fraction (or none) of the total rows, using an exclude model would be much faster than building an include one.

In (1), an empty exclude model means literally all rows.

Does this mean that even if some rows are restricted to be selected by let's say isRowSelectable prop, they'd be selected but not de-selectable from the UI, similar to this?

(Based on the assumption that although we are discouraging the usage of exclude mode with isRowSelectable, we aren't forcing it.)

@MBilalShafi Yes, this is the current behavior:

How about being more specific for the naming since we have cell selection too, and we could have column selection in future too.

Side note independent to this PR: Would it make sense to also implement an include/exclude model for the cell selection feature?
I assume a developer learning the interface of one would expect the other to be similar.

It could also make the cell selection faster particularly if we plan to implement this practice of the WAI-ARIA keyboard interaction guidelines:

Control + A: Selects all cells.

Another quick upside would be to make a somewhat expensive function getSelectedCellsAsArray performant.

// Current interface
Record<GridRowId, Record<GridColDef['field'], boolean>>

// With exclude/include: Could help with "Select all" / "Deselect all"
{ coords: Map<GridRowId, Set<GridColDef['field']>>, type: 'include' | 'exclude' }

// Just convert to Map/Set
Map<GridRowId, Set<GridColDef['field']>>

Wdyt?

In (1), an empty exclude model means literally all rows.

Does this mean that even if some rows are restricted to be selected by let's say isRowSelectable prop, they'd be selected but not de-selectable from the UI, similar to this?

(Based on the assumption that although we are discouraging the usage of exclude mode with isRowSelectable, we aren't forcing it.)

Could this be useful as a public export too?
For example, it could be used as a suggestion in migration guide diff (and also in the codemod probably).

-const [rowSelectionModel, setRSM] = React.useState([1, 2, 3]);
+const [rowSelectionModel, setRSM] = React.useState(includeRowSelection([1, 2, 3]));

We should probably disable dev mode (aka strict mode) in testing, as it's not present in production.