Grids - DataController: Refactoring the _partialUpdateRow method

packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts

@@ -50,13 +50,15 @@ import type {
 import { resolvePaginate, syncPaging } from './utils/paging';
 import { getRefreshOptions } from './utils/refresh';
 import {
+  canDiffColumns,
   convertToUpdateChange,
   getChangedRowIndices,
   getDataRowIndex,
+  getGroupColumnIndices,
   getRowKey,
   getRowOperation,
   indexRowsByKey,
-  isSameGroupRowState,
+  partialUpdateRow,
   pushChangedRow,
   resetChangedRows,
   updateKeptRows,
@@ -832,13 +834,17 @@ export class DataController extends DataHelperMixin(modules.Controller) {
       };
     }
 
+    const columnIndices = isPartialUpdate
+      ? this.getUpdatedColumnIndices(oldItem, newItem, visibleRowIndex)
+      : undefined;
+
+    partialUpdateRow(oldItem, newItem, columnIndices);
+
     return {
       changeType: 'update',
       rowIndex: visibleRowIndex,
       item: newItem,
-      columnIndices: isPartialUpdate
-        ? this._partialUpdateRow(oldItem, newItem, visibleRowIndex)
-        : undefined,
+      columnIndices,
     };
   }
 
@@ -942,36 +948,34 @@ export class DataController extends DataHelperMixin(modules.Controller) {
   /**
    * @extended: editing_row_based, editing, editing_form_based
    */
-  protected _getChangedColumnIndices(
+  protected getChangedColumnIndices(
     oldItem: ProcessedItem,
     newItem: ProcessedItem,
     visibleRowIndex: number,
     isLiveUpdate?: boolean,
   ): number[] | undefined {
-    if (oldItem.rowType !== newItem.rowType) {
+    if (!canDiffColumns(oldItem, newItem)) {
       return undefined;
     }
 
-    if (newItem.rowType === 'group') {
-      if (!oldItem.cells || !isSameGroupRowState(oldItem, newItem)) {
-        return undefined;
-      }
-
-      return oldItem.cells
-        .map((cell, index) => (cell.column?.type !== 'groupExpand' ? index : -1))
-        .filter((index) => index >= 0);
-    }
-
-    if (newItem.rowType === 'groupFooter') {
-      return undefined;
+    switch (newItem.rowType) {
+      case 'group':
+        return getGroupColumnIndices(oldItem, newItem);
+      case 'detail':
+        return [];
+      default:
+        return this.getChangedColumnIndicesCore(oldItem, newItem, visibleRowIndex, isLiveUpdate);
     }
+  }
 
+  private getChangedColumnIndicesCore(
+    oldItem: ProcessedItem,
+    newItem: ProcessedItem,
+    visibleRowIndex: number,
+    isLiveUpdate?: boolean,
+  ): number[] {
     const columnIndices: number[] = [];
 
-    if (newItem.rowType === 'detail') {
-      return columnIndices;
-    }
-
     for (let columnIndex = 0; columnIndex < oldItem.values.length; columnIndex += 1) {
       if (this._isCellChanged(oldItem, newItem, visibleRowIndex, columnIndex, isLiveUpdate)) {
         columnIndices.push(columnIndex);
@@ -981,43 +985,21 @@ export class DataController extends DataHelperMixin(modules.Controller) {
     return columnIndices;
   }
 
-  private _partialUpdateRow(
+  private getUpdatedColumnIndices(
     oldItem: ProcessedItem,
     newItem: ProcessedItem,
     visibleRowIndex: number,
     isLiveUpdate?: boolean,
   ): number[] | undefined {
-    const changedColumnIndices = this
-      ._getChangedColumnIndices(
-        oldItem,
-        newItem,
-        visibleRowIndex,
-        isLiveUpdate,
-      );
-    const columnIndices = changedColumnIndices?.length && this.option('dataRowTemplate')
-      ? undefined
-      : changedColumnIndices;
-
-    if (columnIndices) {
-      oldItem.cells?.forEach((cell, columnIndex) => {
-        const isCellChanged = columnIndices.includes(columnIndex);
-        if (!isCellChanged && cell?.update) {
-          cell.update(newItem);
-        }
-      });
-
-      newItem.update = oldItem.update;
-      newItem.watch = oldItem.watch;
-      newItem.cells = oldItem.cells;
-
-      if (isLiveUpdate) {
-        newItem.oldValues = oldItem.values;
-      }
-
-      oldItem.update?.(newItem);
-    }
+    const changedColumnIndices = this.getChangedColumnIndices(
+      oldItem,
+      newItem,
+      visibleRowIndex,
+      isLiveUpdate,
+    );
+    const hasDataRowTemplate = !!this.option('dataRowTemplate');
 
-    return columnIndices;
+    return changedColumnIndices?.length && hasDataRowTemplate ? undefined : changedColumnIndices;
   }
 
   /**
@@ -1036,13 +1018,15 @@ export class DataController extends DataHelperMixin(modules.Controller) {
     switch (itemChange.type) {
       case 'update': {
         const newItem = itemChange.data;
-        const columnIndices = this._partialUpdateRow(
+        const columnIndices = this.getUpdatedColumnIndices(
           itemChange.oldItem,
           newItem,
           index,
           isLiveUpdate,
         );
 
+        partialUpdateRow(itemChange.oldItem, newItem, columnIndices, isLiveUpdate);
+
         this._items[index] = newItem;
 
         return {

packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/__tests__/row_changes.test.ts

@@ -3,17 +3,20 @@ import {
 } from '@jest/globals';
 
 import type {
-  ChangedRows, DataChange, ItemChange, ProcessedItem, UpdateChange,
+  ChangedRows, DataChange, ItemChange, ProcessedItem, RowWatch, UpdateChange,
 } from '../../types';
 import {
+  canDiffColumns,
   convertToUpdateChange,
   getChangedRowIndices,
   getDataRowIndex,
+  getGroupColumnIndices,
   getRowKey,
   getRowOperation,
   indexRowsByKey,
   isSameGroupRowState,
   isSameItem,
+  partialUpdateRow,
   pushChangedRow,
   resetChangedRows,
   updateKeptRows,
@@ -125,6 +128,48 @@ describe('isSameGroupRowState', () => {
   });
 });
 
+describe('canDiffColumns', () => {
+  it('should allow the diff for the rows of the same type', () => {
+    expect(canDiffColumns(row({ rowType: 'data' }), row({ rowType: 'data' }))).toBe(true);
+  });
+
+  it('should forbid the diff for the rows of different types', () => {
+    expect(canDiffColumns(row({ rowType: 'data' }), row({ rowType: 'detail' }))).toBe(false);
+  });
+
+  it('should forbid the diff for group footers', () => {
+    expect(canDiffColumns(row({ rowType: 'groupFooter' }), row({ rowType: 'groupFooter' })))
+      .toBe(false);
+  });
+});
+
+describe('getGroupColumnIndices', () => {
+  const groupRow = (partial: Partial<ProcessedItem>): ProcessedItem => row({
+    rowType: 'group',
+    isExpanded: true,
+    data: { isContinuation: false, isContinuationOnNextPage: false },
+    ...partial,
+  });
+
+  it('should skip the group expand cell', () => {
+    const oldItem = groupRow({
+      cells: [{ column: { type: 'groupExpand' } }, {}, { column: { dataField: 'name' } }],
+    });
+
+    expect(getGroupColumnIndices(oldItem, groupRow({}))).toEqual([1, 2]);
+  });
+
+  it('should return undefined when the old row has no cells', () => {
+    expect(getGroupColumnIndices(groupRow({}), groupRow({}))).toBeUndefined();
+  });
+
+  it('should return undefined when the group state has changed', () => {
+    const oldItem = groupRow({ cells: [{}] });
+
+    expect(getGroupColumnIndices(oldItem, groupRow({ isExpanded: false }))).toBeUndefined();
+  });
+});
+
 describe('getRowKey', () => {
   it('should tell apart the rows of different types with the same key', () => {
     expect(getRowKey(row({ key: 1, rowType: 'data' })))
@@ -463,3 +508,70 @@ describe('pushChangedRow', () => {
     expect(changedRows.columnIndices).toEqual([undefined]);
   });
 });
+
+describe('partialUpdateRow', () => {
+  it('should pass the new row to the updaters of the cells the change did not touch', () => {
+    const newItem = row({ key: 1 });
+    const cellUpdates = [jest.fn(), jest.fn(), jest.fn()];
+    const oldItem = row({ key: 1, cells: cellUpdates.map((update) => ({ update })) });
+
+    partialUpdateRow(oldItem, newItem, [1]);
+
+    expect(cellUpdates[0]).toHaveBeenCalledWith(newItem);
+    expect(cellUpdates[1]).not.toHaveBeenCalled();
+    expect(cellUpdates[2]).toHaveBeenCalledWith(newItem);
+  });
+
+  it('should update every cell when no column has changed', () => {
+    const newItem = row({ key: 1 });
+    const cellUpdate = jest.fn();
+
+    partialUpdateRow(row({ key: 1, cells: [{ update: cellUpdate }] }), newItem, []);
+
+    expect(cellUpdate).toHaveBeenCalledWith(newItem);
+  });
+
+  it('should move the updaters and the cells to the new row', () => {
+    const update = jest.fn();
+    const watch: RowWatch = () => () => {};
+    const cells = [{}];
+    const newItem = row({ key: 1 });
+    const oldItem = row({
+      key: 1, update, watch, cells,
+    });
+
+    partialUpdateRow(oldItem, newItem, [0]);
+
+    expect(newItem.update).toBe(update);
+    expect(newItem.watch).toBe(watch);
+    expect(newItem.cells).toBe(cells);
+    expect(update).toHaveBeenCalledWith(newItem);
+  });
+
+  it('should keep the old values only on a live update', () => {
+    const values = [1, 2];
+    const liveItem = row({ key: 1 });
+    const item = row({ key: 1 });
+
+    partialUpdateRow(row({ key: 1, values }), liveItem, [0], true);
+    partialUpdateRow(row({ key: 1, values }), item, [0]);
+
+    expect(liveItem.oldValues).toBe(values);
+    expect(item.oldValues).toBeUndefined();
+  });
+
+  it('should do nothing when the whole row is repainted', () => {
+    const update = jest.fn();
+    const cellUpdate = jest.fn();
+    const oldItem = row({ key: 1, update, cells: [{ update: cellUpdate }] });
+    const newItem = row({ key: 1 });
+
+    partialUpdateRow(oldItem, newItem, undefined, true);
+
+    expect(update).not.toHaveBeenCalled();
+    expect(cellUpdate).not.toHaveBeenCalled();
+    expect(newItem.update).toBeUndefined();
+    expect(newItem.cells).toBeUndefined();
+    expect(newItem.oldValues).toBeUndefined();
+  });
+});

packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/row_changes.ts

@@ -32,6 +32,23 @@ export function isSameGroupRowState(item1: ProcessedItem, item2: ProcessedItem):
     && item1.data?.isContinuationOnNextPage === item2.data?.isContinuationOnNextPage;
 }
 
+export function canDiffColumns(oldItem: ProcessedItem, newItem: ProcessedItem): boolean {
+  return oldItem.rowType === newItem.rowType && newItem.rowType !== 'groupFooter';
+}
+
+export function getGroupColumnIndices(
+  oldItem: ProcessedItem,
+  newItem: ProcessedItem,
+): number[] | undefined {
+  if (!oldItem.cells || !isSameGroupRowState(oldItem, newItem)) {
+    return undefined;
+  }
+
+  return oldItem.cells
+    .map((cell, index) => (cell.column?.type !== 'groupExpand' ? index : -1))
+    .filter((index) => index >= 0);
+}
+
 /**
  * Rows of different types may share a key, so the row type is a part of the key
  * the diff is built on.
@@ -211,3 +228,31 @@ export function pushChangedRow(changedRows: ChangedRows, changedRow: UpdateRowCh
   changedRows.changeTypes.push(changeType);
   changedRows.columnIndices.push(columnIndices);
 }
+
+export function partialUpdateRow(
+  oldItem: ProcessedItem,
+  newItem: ProcessedItem,
+  columnIndices: number[] | undefined,
+  isLiveUpdate?: boolean,
+): void {
+  if (!columnIndices) {
+    return;
+  }
+
+  oldItem.cells?.forEach((cell, columnIndex) => {
+    const isCellChanged = columnIndices.includes(columnIndex);
+    if (!isCellChanged && cell?.update) {
+      cell.update(newItem);
+    }
+  });
+
+  newItem.update = oldItem.update;
+  newItem.watch = oldItem.watch;
+  newItem.cells = oldItem.cells;
+
+  if (isLiveUpdate) {
+    newItem.oldValues = oldItem.values;
+  }
+
+  oldItem.update?.(newItem);
+}

packages/devextreme/js/__internal/grids/grid_core/editing/extenders/editing_data_controller.ts

@@ -115,7 +115,7 @@ export const editingDataControllerExtender = (
     return super.isSameRowState(item1, item2);
   }
 
-  protected _getChangedColumnIndices(
+  protected getChangedColumnIndices(
     oldItem: ProcessedItem,
     newItem: ProcessedItem,
     visibleRowIndex: number,
@@ -125,7 +125,7 @@ export const editingDataControllerExtender = (
       return undefined;
     }
 
-    return super._getChangedColumnIndices(oldItem, newItem, visibleRowIndex, isLiveUpdate);
+    return super.getChangedColumnIndices(oldItem, newItem, visibleRowIndex, isLiveUpdate);
   }
 
   protected _isCellChanged(

packages/devextreme/js/__internal/grids/grid_core/editing/extenders/editing_form_based_data_controller.ts

@@ -24,7 +24,7 @@ export const editingFormBasedDataControllerExtender = (
     }
   }
 
-  protected _getChangedColumnIndices(
+  protected getChangedColumnIndices(
     oldItem: ProcessedItem,
     newItem: ProcessedItem,
     visibleRowIndex: number,
@@ -34,6 +34,6 @@ export const editingFormBasedDataControllerExtender = (
       return undefined;
     }
 
-    return super._getChangedColumnIndices(oldItem, newItem, visibleRowIndex, isLiveUpdate);
+    return super.getChangedColumnIndices(oldItem, newItem, visibleRowIndex, isLiveUpdate);
   }
 };