Skip to content

GridCore dataController: fix ESLint errors in the updateItems block - #34754

Open
Alyar666 wants to merge 2 commits into
DevExpress:mainfrom
Alyar666:grid_core_datacontroller_fix_eslint_part_6_26_2
Open

GridCore dataController: fix ESLint errors in the updateItems block#34754
Alyar666 wants to merge 2 commits into
DevExpress:mainfrom
Alyar666:grid_core_datacontroller_fix_eslint_part_6_26_2

Conversation

@Alyar666

Copy link
Copy Markdown
Contributor

No description provided.

@Alyar666 Alyar666 self-assigned this Aug 12, 2026
Copilot AI lite review requested due to automatic review settings August 12, 2026 13:06
@Alyar666
Alyar666 requested a review from a team as a code owner August 12, 2026 13:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the GridCore DataController “partial refresh” path (the applyChangesOnly / row-index correction flow) to address ESLint issues while improving type safety and extracting reusable row-change utilities.

Changes:

  • Renamed internal extensibility hooks from _applyChangesOnly / _correctRowIndices to applyChangesOnly / correctRowIndices and updated all overrides.
  • Refactored DataController.applyChangesOnly into smaller helpers and moved key/index/cell-update logic into utils/row_changes.ts.
  • Expanded Jest coverage for the extracted row_changes utilities and introduced supporting types (RowIndexByKey, ItemChange).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/devextreme/js/__internal/grids/grid_core/keyboard_navigation/m_keyboard_navigation.ts Updates the keyboard-navigation extender override to the renamed correctRowIndices hook.
packages/devextreme/js/__internal/grids/grid_core/editing/m_editing.ts Updates the editing extender override to the renamed applyChangesOnly hook.
packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/row_changes.ts Extracts and adds helper utilities for row keying, indexing, cell updater propagation, and change conversion.
packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/tests/row_changes.test.ts Adds unit tests for the newly extracted row-change helpers.
packages/devextreme/js/__internal/grids/grid_core/data_controller/types.ts Adds new helper types (RowIndexByKey, ItemChange) used by the refactored update logic.
packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts Refactors the partial-update (“repaint changes only”) refresh path, improves typing, and updates insert-index correction logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings August 12, 2026 13:13
@Alyar666
Alyar666 force-pushed the grid_core_datacontroller_fix_eslint_part_6_26_2 branch from dcd8301 to d70a8e5 Compare August 12, 2026 13:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

  • getDataRowIndex allocates intermediate arrays (slice + filter) every time it runs. This method is called from changingHandler for each inserted change and can run on large visible-row sets; a simple counted loop avoids allocations and should be measurably cheaper while still satisfying no-plusplus by using += 1.
export function getDataRowIndex(rows: ProcessedItem[], visibleRowIndex: number): number {
  const previousRows = rows.slice(0, visibleRowIndex);

  return previousRows.filter((row) => row?.rowType === 'data' || row?.rowType === 'group').length;
}

* A store change is indexed by data rows, while an insert index coming from the
* grid counts every visible row — group rows included.
*/
export function getDataRowIndex(rows: ProcessedItem[], visibleRowIndex: number): number {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export function getDataRowIndex(rows: ProcessedItem[], visibleRowIndex: number): number {
export function getDataRowCount(rows: ProcessedItem[], visibleRowIndex: number): number {

return changedRows;
}

export function convertToUpdateChange(change: DataChange, changedRows: ChangedRows): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'convert' reads as we passed smth and return converted one, not mutate.
can we avoid mutation here?

Copilot AI review requested due to automatic review settings August 13, 2026 19:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

  • getDataRowIndex allocates intermediate arrays (slice + filter) on every call. This runs in the dataSource changing handler and can be invoked frequently for large grids, so avoiding allocations here helps prevent unnecessary GC pressure.
export function getDataRowIndex(rows: ProcessedItem[], visibleRowIndex: number): number {
  const previousRows = rows.slice(0, visibleRowIndex);

  return previousRows.filter((row) => row?.rowType === 'data' || row?.rowType === 'group').length;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants