Skip to content

Grids: extract dataController remoteOperations normalization into utils - #34673

Open
bit-byte0 wants to merge 2 commits into
DevExpress:mainfrom
bit-byte0:refactor/gridcore-datacontroller-adapter-utils-26_2
Open

Grids: extract dataController remoteOperations normalization into utils#34673
bit-byte0 wants to merge 2 commits into
DevExpress:mainfrom
bit-byte0:refactor/gridcore-datacontroller-adapter-utils-26_2

Conversation

@bit-byte0

@bit-byte0 bit-byte0 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What

The grids' data controller now resolves the remoteOperations option through a dedicated helper instead of inline logiс

How

The 'auto', true and groupPaging normalization and the store-type checks moved out of _createDataSourceAdapter into a new utils/adapter.ts module that the controller calls

@bit-byte0
bit-byte0 requested a review from a team as a code owner August 6, 2026 08:50
Copilot AI review requested due to automatic review settings August 6, 2026 08:50
@bit-byte0 bit-byte0 added the 26_2 label Aug 6, 2026
@bit-byte0 bit-byte0 self-assigned this Aug 6, 2026

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 Grid Core’s data controller to normalize the remoteOperations option via a dedicated utility helper, aiming to keep behavior unchanged while improving encapsulation and testability.

Changes:

  • Extracted remoteOperations normalization (including 'auto', true, and groupPaging handling) into utils/adapter.ts.
  • Simplified DataController._createDataSourceAdapter by delegating normalization to the helper.
  • Added Jest coverage for the new normalization and store-type helpers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/adapter.ts Introduces store-type checks and remoteOperations normalization helper.
packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/tests/adapter.test.ts Adds Jest tests covering store detection and normalization behavior.
packages/devextreme/js/__internal/grids/grid_core/data_controller/types.ts Adds new types for remoteOperations mode/options used by the helper.
packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts Replaces inline normalization with a call to the extracted helper.

Comment thread packages/devextreme/js/__internal/grids/grid_core/data_controller/types.ts Outdated
Copilot AI review requested due to automatic review settings August 6, 2026 11:33

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 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/adapter.ts:27

  • The groupPaging normalization currently uses object spread ({ ...enabledRemoteOperations, ...remoteOperations }). This subtly changes behavior vs the previous extend(...) logic: spread will overwrite defaults with undefined values (disabling operations that were previously kept enabled) and it ignores enumerable properties coming from the prototype chain. To keep normalization behavior stable, merge via a for..in loop that skips undefined values (and guards against __proto__/constructor).
  // groupPaging only works when every operation runs remotely.
  if (isObject(remoteOperations) && remoteOperations.groupPaging) {
    return { ...enabledRemoteOperations, ...remoteOperations };
  }

@bit-byte0
bit-byte0 force-pushed the refactor/gridcore-datacontroller-adapter-utils-26_2 branch from cfccd4f to 7d9f707 Compare August 6, 2026 11:58
Copilot AI review requested due to automatic review settings August 6, 2026 11:58

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 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/tests/adapter.test.ts:80

  • The new tests cover the main normalization branches, but they don’t assert the legacy behavior that undefined flags should not override the default enabled operations in the groupPaging merge (previously extend skipped undefined). Adding a regression test for this helps prevent reintroducing the subtle undefined→falsy behavior change.
  it('groupPaging object overrides individual operation flags', () => {
    const result = normalizeRemoteOperations({ groupPaging: true, filtering: false }, remoteStore);

    expect(result).toEqual({
      filtering: false,
      sorting: true,
      paging: true,
      grouping: true,
      summary: true,
      groupPaging: true,
    });
  });

packages/devextreme/js/__internal/grids/grid_core/data_controller/utils/adapter.ts:27

  • remoteOperations.groupPaging branch uses object spread to merge defaults, but the previous implementation used extend, which intentionally skips keys whose value is undefined. With the current spread merge, a user-provided object like { groupPaging: true, filtering: undefined } will overwrite the default filtering: true with undefined, which then behaves as false in downstream checks (e.g. !remoteOperations.filtering). Consider merging while ignoring undefined values to preserve the old semantics.
  // groupPaging only works when every operation runs remotely.
  if (isObject(remoteOperations) && remoteOperations.groupPaging) {
    return { ...enabledRemoteOperations, ...remoteOperations };
  }

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants