DataGrid: extract the row update path from dataController - #34986
DataGrid: extract the row update path from dataController#34986Alyar666 wants to merge 1 commit into
Conversation
Moved updateRow, applyRowOperation and the applyChangeUpdate loop into row_changes.ts as applyRowOperations: it takes the items and the plain values instead of `this` and returns ChangedRows, which the controller attaches to the change. applyChangeUpdate stays a thin delegate, so the editing override keeps composing through super. Covered the engine and the delegate wiring with jest tests.
There was a problem hiding this comment.
Pull request overview
This PR refactors the DataGrid row-diffing “update” path by extracting the row-operation application loop from DataController into data_controller/utils/row_changes.ts as applyRowOperations, while keeping DataController.applyChangeUpdate as a thin delegate that attaches the computed ChangedRows back onto the change object. It also introduces a typed callback (GetUpdatedColumnIndices) and adds Jest coverage for the extracted behavior and wiring.
Changes:
- Added
applyRowOperations(...)plus supporting helpers inrow_changes.ts, and exportedattachChangedRows(...)for reuse by the controller. - Simplified
DataController.applyChangeUpdate(...)to compute changed row indices, delegate the mutation/reporting work toapplyRowOperations, then attach the results. - Added/updated Jest tests covering row-operation classification/application and the
applyChangeUpdatedelegate behavior under key option combinations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/row_changes.ts | Introduces applyRowOperations + exports attachChangedRows to decouple row update application from DataController. |
| packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/tests/row_changes.test.ts | Adds unit tests validating extracted row-operations behavior (index arithmetic, duplicates, visibility-only update, callback contract). |
| packages/devextreme/js/__internal/grids/grid_core/data_controller/types.ts | Adds GetUpdatedColumnIndices and RowOperationOptions types used to pass the hook/options into the extracted engine. |
| packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts | Replaces inline update-loop with applyRowOperations + attachChangedRows; converts getUpdatedColumnIndices into an arrow field for safe passing as a callback. |
| packages/devextreme/js/__internal/grids/grid_core/data_controller/tests/data_controller.row_changes.test.ts | Adds integration-style Jest tests for applyChangeUpdate wiring across repaintChangesOnly, isFullUpdate, and dataRowTemplate. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| } | ||
|
|
||
| export function applyRowOperations( |
There was a problem hiding this comment.
suggestion: return UpdateRowChange[], initiated with plain [] instead of initChangedRows - remove one step in utility reading
in attachChangedRows take as 2nd argument UpdateRowChange[] and convert it inside method (we already have toChangedRows, just move it inside attachChangedRows instead of argument
| oldItem.update?.(newItem); | ||
| } | ||
|
|
||
| function updateRow(rowIndex: number, options: RowOperationOptions): UpdateRowChange { |
There was a problem hiding this comment.
function updates row and create UpdateRowChange instance. Let's split it, updateRow will only update data, symmetrical to applyItemChange
in fact, are those methods differ? may we use single one for both branches?
No description provided.