[DataGrid] Add support for replacing rows instead of merging updates - #23323
[DataGrid] Add support for replacing rows instead of merging updates#23323jvskriubakken wants to merge 16 commits into
Conversation
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
JCQuintas
added
scope: data grid
There was a problem hiding this comment.
@jvskriubakken
Thanks for the contribution and the details provided.
I have made some adjustments, so it looks pretty much ok for me now.
The only remaining thing is the type change. Let's see what others think.
| export type GridRowModel<R extends GridValidRowModel = GridValidRowModel> = R; | ||
|
|
||
| export type GridUpdateAction = 'delete'; | ||
| export type GridUpdateAction = 'delete' | 'replace'; |
There was a problem hiding this comment.
This type is not used internally anymore, but it is exported, so we have to keep it.
Not adding 'replace' would be inaccurate, but adding it would create a breaking change for users.
Depending on the usage, additional checks would be needed to confirm that the update is of a specific type.
Suggestions @MBilalShafi @JCQuintas @jvskriubakken?
There was a problem hiding this comment.
This was the original suggestion by Claude on how to extend this type:
type GridRowModelUpdate<R extends GridValidRowModel = GridValidRowModel> =
| (Partial<R> & { id: GridRowId; _action?: never })
| { id: GridRowId; _action: 'delete' }
| (R & { _action: 'replace' }); // NEW — full row required
But I am not sure if that makes it a less "breaking change".
| newRow: R, | ||
| oldRow: R, | ||
| params: { rowId: GridRowId }, | ||
| ) => Promise<R | GridRowModelReplace<R>> | R | GridRowModelReplace<R>; |
There was a problem hiding this comment.
This could also break for people using the prop's return value for further processing, but that is very unlikely.
3 participants
Resolves #22606
Problem
apiRef.current.updateRows() always merges: the stored row is rebuilt as a new object from the old row plus the update. That makes three things impossible:
For apps whose rows are class instances (typically built from a backend JSON response), there was no way to tell the Grid "store this object, as-is".
Solution
A new replace update, passed as an envelope object rather than a marker on the row itself:
The Grid stores row verbatim: apiRef.current.getRow(id) === myInstance. A replace is never a merge — fields missing from row are removed from the stored row.
The envelope was chosen over marking the row (row._action = 'replace', mirroring the existing delete action) so the user's object is never mutated: no hidden-class churn, frozen/sealed instances work, no marker can leak onto a long-lived object and turn a later innocent update into a replace, and it types cleanly as a discriminated union.
Why
processRowUpdate()matters hereThis is the part worth understanding before using the feature.
updateRows()is not the only code path that writes rows. Cell editing, row editing and clipboard paste also persist rows — and they build a plain-object draft of the edited row internally. If you don't intervene, that draft goes through the merge path and your class instance is silently demoted to a copy: prototype survives, identity and #private fields do not. The failure is delayed and confusing — the row looks fine until some method that reads a private field throws.The Grid never persists an edit behind your back, though: it always hands the draft to
processRowUpdate(newRow, oldRow, params)and stores whatever you return. SoprocessRowUpdate()is the single place where you convert the draft back into a real instance, and it can now return the replace envelope:With dataSource, the equivalent hook is
dataSource.updateRow()— its return value is funneled into the same code path, so it can return the envelope too.So, when is processRowUpdate() needed?
One-sentence contract: identity survives everywhere, provided every write goes through either a replace envelope or a
processRowUpdate()/dataSource.updateRow()that returns one.Caveats (documented)
Changes
new GridRowReplaceUpdate<R> ({ _action: 'replace'; row: R });`` GridRowModelUpdate._actionstays 'delete', so the two form a discriminated union.isReplaceUpdate()type guard andgetReplaceRow()unwrapper (throws a documented error if row is missing);updateCacheWithNewRows()unwraps the envelope in the dedup loop and tracks replaced ids in a Set, never on the row; extractedmergeRowUpdate()for the prototype-preserving merge.updateRows(),updateNestedRows()andupdateNonPivotRows()accept the new update type; pinned rows (computeRowsUpdates), pivoting, tree data and dataSource paths all handle it.processRowUpdatereturn type widened toR | GridRowReplaceUpdate<R> | Promise<…>.clipboardPasteEndevent payload exposes the stored row, not the wrapper.Performance
The non-replace hot path is unchanged apart from one
_action === 'replace'comparison. The replace path is cheaper than a merge: it stores a reference instead of allocating and copying every field. The envelope is one short-lived allocation per replace. Dev warnings are NODE_ENV-guarded.Tests