GridCore data: Merge `DataHelperMixin` into `DataController` by bit-byte0 · Pull Request #34938 · DevExpress/DevExtreme

bit-byte0 · 2026-08-26T19:54:22Z

What

Removes the grid_core-only DataHelperMixin and folds its data-source lifecycle logic directly into DataController, so the controller no longer carries unused generic mixin machinery. Also fixes a pre-existing case where the shared-data-source flag stayed set after switching to a non-shared source

How

Inlined the live parts (postInit, _refreshDataSource, ready-watcher wiring) into data_controller.ts and dropped the dead generic paths that grid_core never exercised. The raw DataSource is now built by a local _createRawDataSource helper so _dataSource only ever holds the adapter, and _isSharedDataSource is now assigned on every path

Copilot

Pull request overview

This PR removes the grid_core-specific DataHelperMixin and folds its remaining data-source lifecycle responsibilities into grid_core’s DataController, reducing reliance on generic mixin plumbing and making DataController the single place that wires data-source readiness/disposal behavior.

Changes:

Removed DataHelperMixin and stopped extending it from DataController.
Inlined data-source lifecycle hooks into DataController (init/refresh + ready-watcher wiring) and introduced _createRawDataSource() to construct the raw DataSource before adapter wrapping.
Updated search code to read langParams via getDataSource() directly.

Reviewed changes

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

File	Description
packages/devextreme/js/__internal/grids/grid_core/search/m_search.ts	Adjusts how `langParams` is retrieved (via `getDataSource()`), aligning with the removal of the mixin-held controller path.
packages/devextreme/js/__internal/grids/grid_core/data_controller/data_helper_mixin.ts	Removes the mixin implementation entirely.
packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts	Integrates the mixin’s data-source lifecycle setup into `DataController` and adds `_createRawDataSource()` + ready-watcher wiring.

Suppressed comments (1)

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

_createRawDataSource() disposes the previous adapter but doesn’t unsubscribe readyWatcher from the previous raw DataSource, and it never clears _isSharedDataSource after disposal. If the previous dataSource was shared and the new one is not, the stale _isSharedDataSource flag can cause future adapters to be disposed as “shared” (skipping dispose) and the loadingChanged subscription can leak. Detach the listener before disposing and reset _isSharedDataSource after disposal.

  private _createRawDataSource(): DataSource | undefined {
    const dataSourceOptions = this._getSpecificDataSourceOption();

    this._disposeDataSource();

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

…t shared

Copilot

Pull request overview

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

Tucchhaa · 2026-08-27T11:02:40Z

+  protected readyWatcher?: (isLoading: boolean) => void;
+
+  protected _ready?: (value?: boolean) => void;


Looks like the 'ready' callback is never set and used. If that's true, let's remove readyWatcher and _ready

Tucchhaa · 2026-08-27T11:03:40Z

+    const isSharedDataSource = dataSourceOptions instanceof DataSourceClass;
+
+    this._isSharedDataSource = isSharedDataSource;


optional code-style suggestion, use just this._isSharedDataSource prop and delete var:

this._isSharedDataSource = dataSourceOptions instanceof DataSourceClass;

Tucchhaa · 2026-08-27T11:06:40Z

+  public postInit(): void {
+    this.on('disposing', () => {
+      this._disposeDataSource();
+    });
+  }


Do we really need postInit() method? I see that dataController.dispose() already calls _disposeDataSource()

Tucchhaa · 2026-08-27T11:10:34Z

+  /**
+   * @extended: state_storing, virtual_scrolling
+   */
+  protected _refreshDataSource(): DeferredObj<unknown> | undefined {


What do you think about renaming the method to resetDataSource()?
I think that name expresses what this function does more clearly than the current name

i don't object about it

Tucchhaa · 2026-08-27T11:13:12Z


  protected _calculateAdditionalFilter(): DataFilter {
-    const dataSource = this._dataController?.getDataSource?.();
+    const dataSource = this.getDataSource?.();


this.getDataSource();

…ename to resetDataSource

Copilot

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

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

When the dataSource option is cleared (null/undefined/0/''), _createRawDataSource returns early without updating _isSharedDataSource, so the flag can retain the previous value even though there is no current data source. This makes _isSharedDataSource reflect stale state and can lead to confusing behavior for any code/tests that reads it after clearing the dataSource.

    if (!dataSourceOptions) {
      return undefined;
    }

anna-shakhova · 2026-08-27T11:53:18Z

    }
  }

+  private _createRawDataSource(): DataSource | undefined {


lets remove _ for added fields and methods (except _dataSource may be - if it assumes multiple changes through code base)

anna-shakhova · 2026-08-27T12:04:12Z

+    const dataSource = (isSharedDataSource
+      ? dataSourceOptions
+      : new DataSourceClass(
+        extend(true, {}, normalizeDataSourceOptions(dataSourceOptions, { fromUrlLoadMode: false })),


a bit difficult to read, after readyWatcher removed, then lets use early return

{ fromUrlLoadMode: false } is excessive, empty options object can be passed

Suggested change

extend(true, {}, normalizeDataSourceOptions(dataSourceOptions, { fromUrlLoadMode: false })),

extend(true, {}, normalizeDataSourceOptions(dataSourceOptions, {})),

anna-shakhova · 2026-08-27T12:08:31Z

    }
  }

+  private _createRawDataSource(): DataSource | undefined {


incorrect naming - this method not only creates new raw data source, but also disposes existing one

…ore, early return

Copilot

Pull request overview

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

Suppressed comments (1)

packages/devextreme/js/__internal/grids/grid_core/data_controller/tests/data_controller.data_source.test.ts:55

After changing the dataSource option, the controller update/load cycle is async in these tests. Add await flushAsync() before asserting on internal controller state to avoid flaky timing-dependent failures.

      instance.option('dataSource', DATA);

      expect(getIsSharedDataSource(instance)).toBe(false);

…re on isSharedDataSource, reset flag on every path

…d types

…atacontroller-merge-datahelper-mixin-26_2

Copilot

Pull request overview

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

…stead of unknown

Copilot

Pull request overview

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

…a source test

Copilot

Pull request overview

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

refactor(dataController): inline DataHelperMixin into DataController

0e12120

Copilot AI lite review requested due to automatic review settings August 26, 2026 19:54

bit-byte0 requested a review from a team as a code owner August 26, 2026 19:54

bit-byte0 added the 26_2 label Aug 26, 2026

bit-byte0 self-assigned this Aug 26, 2026

Copilot started reviewing on behalf of bit-byte0 August 26, 2026 19:54 View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

Comment thread packages/devextreme/js/__internal/grids/grid_core/data_controller/data_controller.ts Outdated

fix(dataController): reset _isSharedDataSource when data source is no…

3874a4d

…t shared

Copilot AI review requested due to automatic review settings August 27, 2026 08:29

Copilot started reviewing on behalf of bit-byte0 August 27, 2026 08:29 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Tucchhaa reviewed Aug 27, 2026

View reviewed changes

refactor(dataController): drop dead ready-watcher, inline postInit, r…

6c59ade

…ename to resetDataSource

Copilot AI review requested due to automatic review settings August 27, 2026 12:00

Copilot started reviewing on behalf of bit-byte0 August 27, 2026 12:01 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

anna-shakhova reviewed Aug 27, 2026

View reviewed changes

bit-byte0 added 2 commits August 27, 2026 17:02

test(dataController): rename _refreshDataSource calls to resetDataSource

f2f41d1

refactor(dataController): keep createRawDataSource pure, drop undersc…

42ec047

…ore, early return

Copilot AI review requested due to automatic review settings August 27, 2026 13:08

Copilot started reviewing on behalf of bit-byte0 August 27, 2026 13:08 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

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

Comment thread .../js/__internal/grids/grid_core/data_controller/__tests__/data_controller.data_source.test.ts

Tucchhaa previously approved these changes Aug 27, 2026

View reviewed changes

refactor(dataController): rename to recreateDataSource, drop undersco…

445c3f3

…re on isSharedDataSource, reset flag on every path

Copilot AI review requested due to automatic review settings August 27, 2026 14:22

bit-byte0 dismissed Tucchhaa’s stale review via 445c3f3 August 27, 2026 14:22

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 27, 2026

Tucchhaa previously approved these changes Aug 28, 2026

View reviewed changes

bit-byte0 added this pull request to the merge queue Aug 28, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 28, 2026

bit-byte0 added this pull request to the merge queue Aug 28, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 28, 2026

bit-byte0 added 2 commits August 28, 2026 11:53

fix(dataController): cast raw DataSource through unknown for tightene…

e75720f

…d types

Merge remote-tracking branch 'upstream/main' into refactor/gridcore-d…

dab679e

…atacontroller-merge-datahelper-mixin-26_2

Copilot AI review requested due to automatic review settings August 28, 2026 07:55

bit-byte0 dismissed stale reviews from Tucchhaa and anna-shakhova via dab679e August 28, 2026 07:55

Copilot started reviewing on behalf of bit-byte0 August 28, 2026 07:55 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread .../js/__internal/grids/grid_core/data_controller/__tests__/data_controller.data_source.test.ts Outdated

test(dataController): type getIsSharedDataSource helper as boolean in…

bbbfb16

…stead of unknown

Copilot AI review requested due to automatic review settings August 28, 2026 08:06

Copilot started reviewing on behalf of bit-byte0 August 28, 2026 08:07 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Tucchhaa previously approved these changes Aug 28, 2026

View reviewed changes

anna-shakhova previously approved these changes Aug 28, 2026

View reviewed changes

test(dataController): use ExposedDataController instead of any in dat…

7eda658

…a source test

Copilot AI review requested due to automatic review settings August 28, 2026 08:33

bit-byte0 dismissed stale reviews from anna-shakhova and Tucchhaa via 7eda658 August 28, 2026 08:33

Copilot started reviewing on behalf of bit-byte0 August 28, 2026 08:33 View session

anna-shakhova approved these changes Aug 28, 2026

View reviewed changes

Copilot AI reviewed Aug 28, 2026

View reviewed changes

bit-byte0 added this pull request to the merge queue Aug 28, 2026

Merged via the queue into DevExpress:main with commit a5d793b Aug 28, 2026
116 of 117 checks passed

bit-byte0 deleted the refactor/gridcore-datacontroller-merge-datahelper-mixin-26_2 branch August 28, 2026 10:19

		protected readyWatcher?: (isLoading: boolean) => void;

		protected _ready?: (value?: boolean) => void;

		const isSharedDataSource = dataSourceOptions instanceof DataSourceClass;

		this._isSharedDataSource = isSharedDataSource;

	extend(true, {}, normalizeDataSourceOptions(dataSourceOptions, { fromUrlLoadMode: false })),
	extend(true, {}, normalizeDataSourceOptions(dataSourceOptions, {})),

Uh oh!

Conversation

bit-byte0 commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Tucchhaa Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tucchhaa Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tucchhaa Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tucchhaa Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bit-byte0 Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Tucchhaa Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

anna-shakhova Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

anna-shakhova Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anna-shakhova Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

bit-byte0 commented Aug 26, 2026 •

edited

Loading

Tucchhaa Aug 27, 2026 •

edited

Loading

Tucchhaa Aug 27, 2026 •

edited

Loading

Tucchhaa Aug 27, 2026 •

edited

Loading

Tucchhaa Aug 27, 2026 •

edited

Loading

anna-shakhova Aug 27, 2026 •

edited

Loading