Grids: encapsulate FilterSyncController's cross-module surface

[anna-shakhova]

No description provided.

[Copilot]

Pull request overview

This PR refactors Grid Core’s filter-sync implementation to reduce cross-module coupling by extracting shared filter-sync logic into dedicated utilities/types and by narrowing/clarifying internal surfaces used by state storing and other modules.

Changes:

• Exposed a typed FilterSyncController.getFilterValueFromColumns() for state storing, and extracted filter-sync logic into filter_sync/utils.ts.
• Split the ColumnHeadersView filter-sync extender into its own module and updated filter-sync module wiring.
• Introduced internal FilterValue* type aliases and narrowed InternalGridOptions.filterValue to the internal expression shape.

Reviewed changes

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

Show a summary per file

File	Description
packages/devextreme/js/__internal/grids/grid_core/state_storing/types.ts	Reuses ColumnUserState from columns controller instead of redefining it locally.
packages/devextreme/js/__internal/grids/grid_core/state_storing/state_storing_controller.ts	Uses typed FilterSyncController surface for deriving filterValue from persisted columns.
packages/devextreme/js/__internal/grids/grid_core/m_types.ts	Narrows internal filterValue option typing to internal FilterValue.
packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts	Centralizes filter-sync calculations (conditions, header/filter-row sync, validation).
packages/devextreme/js/__internal/grids/grid_core/filter_sync/types.ts	Adds shared filter-sync type aliases for columns and state fragments.
packages/devextreme/js/__internal/grids/grid_core/filter_sync/m_filter_sync.ts	Refactors controller to use extracted utils and exposes a typed cross-module method.
packages/devextreme/js/__internal/grids/grid_core/filter_sync/m_filter_custom_operations.ts	Adds return typing for custom operations (anyof/noneof).
packages/devextreme/js/__internal/grids/grid_core/filter_sync/filter_sync_module.ts	Rewires module to use the extracted ColumnHeadersView extender.
packages/devextreme/js/__internal/grids/grid_core/filter_sync/extenders/filter_sync_data_controller.ts	Introduces a small public extension interface and aligns calls with the refactor.
packages/devextreme/js/__internal/grids/grid_core/filter_sync/extenders/filter_sync_column_headers_view.ts	New dedicated ColumnHeadersView extender for filter-sync behavior.
packages/devextreme/js/__internal/grids/grid_core/data_controller/types.ts	Adds internal FilterValue* type aliases to formalize filter-sync expressions.
packages/devextreme/js/__internal/grids/grid_core/columns_controller/types.ts	Exposes ColumnUserState type and adds defaultFilterOperation to internal column options.

---

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:132

• FilterValueCondition may be [field, value] (2 items). In that case condition?.[1] is the value, not the operation, and condition?.[2] is undefined, so the filter row state gets cleared instead of applying an implicit '=' operation. Normalize 2-item conditions before deriving operation/filterValue.

  const operation = condition?.[1] as Column['selectedFilterOperation'];
  const filterValue = condition?.[2];
  // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing

packages/devextreme/js/__internal/grids/grid_core/filter_sync/types.ts:4

• The doc comment says FilterSyncColumn combines Column, FilterField, and ColumnUserState, but the actual type is just Partial<FilterField>. This is misleading for consumers trying to understand what properties are required/available.

/** Alias combines `Column`, `FilterField` and the persisted `ColumnUserState`. */
export type FilterSyncColumn = Partial<FilterField>;

[Copilot]

Pull request overview

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

Suppressed comments (2)

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:131

• getFilterRowOptionsFromCondition reads condition?.[1] as the operation and condition?.[2] as the value. For 2-item filter conditions ([field, value]), this treats the value as an operation and clears the filter row state. Normalize 2-item conditions to [field, '=', value] before extracting operation/filterValue.

  const operation = condition?.[1] as Column['selectedFilterOperation'];
  const filterValue = condition?.[2];

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:111

• FilterValueCondition can be a 2-item condition ([field, value] where = is implied). getHeaderFilterFromCondition indexes condition[2] / condition[1] assuming a 3-item tuple, which will mis-handle 2-item conditions returned by getMatchedConditions and can clear header filter state unexpectedly. Normalize 2-item conditions to 3-item form before reading operation/value.

  const value = condition[2];
  const hasArrayValue = Array.isArray(value);

  if (!hasArrayValue && !canSyncHeaderFilterWithFilterRow(column)) {
    return getEmptyFilterValues();

[Copilot]

Pull request overview

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

Suppressed comments (2)

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:131

• FilterValueCondition can be a 2-item shorthand [field, value], but operation/filterValue are read as condition[1]/condition[2] (3-item shape). For shorthand conditions, this causes the filter row options to be reset instead of being restored. Normalize the condition before extracting operation/value.

  const operation = condition?.[1] as Column['selectedFilterOperation'];
  const filterValue = condition?.[2];

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:110

• FilterValueCondition includes the 2-item shorthand form [field, value], but this function assumes a 3-item condition and reads condition[2] / condition[1] as (value/operation). For shorthand conditions this clears header-filter state instead of syncing it. Consider normalizing 2-item conditions to [field, '=', value] before reading indices.

  const value = condition[2];
  const hasArrayValue = Array.isArray(value);

  if (!hasArrayValue && !canSyncHeaderFilterWithFilterRow(column)) {

[Alyar666]

The argument is missing a type

[Alyar666]

And can we remove the underscore from the method name?

[anna-shakhova]

this is columnHeadersView extender, I've extracted it but not typed intentionally as base is not typed

[Copilot]

🟡 Changes recommended

Filter sync utilities currently mis-handle the implicit 2-item filter condition form ([field, value]), which can prevent header/filter-row state from syncing correctly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:138

• getFilterRowOptionsFromCondition reads operation from condition[1] and filterValue from condition[2], which breaks for the implicit filter form [field, value] (implied =). This can prevent filter row state from syncing when filterValue contains 2-item conditions.

  const operation = condition?.[1] as Column['selectedFilterOperation'];
  const filterValue = condition?.[2];
  // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing

packages/devextreme/js/__internal/grids/grid_core/filter_sync/tests/utils.test.ts:348

• Add Jest coverage for the implicit filter condition form [field, value] (implied =) in getFilterRowOptionsFromCondition, since it should behave like the explicit ['field', '=', value] form.

  it('applies a built-in operation', () => {
    expect(getFilterRowOptionsFromCondition(condition('=', 1), { dataField: 'field' }))
      .toStrictEqual({
        filterValue: 1,
        selectedFilterOperation: '=',

• Files reviewed: 13/13 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are confirmed issues that can cause lint failures and runtime exceptions when filterValue is undefined in active filter-sync flows.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:194

• syncFilters assumes the input filter is an array or null and accesses filter.length; FilterValue allows undefined, so this path can throw at runtime. Coerce undefined to null before calling syncFilters.

  const condition = getConditionFromHeaderFilter(column);

  if (condition) {
    return syncFilters(filterValue, condition) as FilterValue;
  }

• Files reviewed: 13/13 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new FilterValueCondition typing is currently too permissive for the 2-item shorthand form and can allow values that filter-builder utilities won’t treat as valid conditions.

Review details

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/types.ts:227

• FilterValueCondition currently allows the 2-item shorthand form [field, value] to use FilterValueOperand, which includes arrays. In filter-builder utils, a 2-item condition whose second item is an array is not recognized as a condition (isCondition requires criteria[1] to not be an array), so values like ["field", [1, 2]] would be type-accepted but behave inconsistently at runtime. The 2-item shorthand should be restricted to scalar values only (implied =), matching BinaryDataFilterExpression.

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The new filter-sync utilities can produce or manipulate filter conditions with an undefined field when a column has no identifier, which can break filterValue synchronization/removal logic at runtime.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:91

• getConditionFromHeaderFilter also casts the column identifier to string without checking it is defined, which can create invalid filter conditions (field becomes undefined) for partial column shapes. Guard against missing name/dataField and return null in that case.

  const field = getColumnIdentifier(column) as string;
  const isExcluded = column.filterType === FILTER_TYPES_EXCLUDE;
  const isSingleValue = filterValues.length === 1
    && canSyncHeaderFilterWithFilterRow(column)
    && !Array.isArray(filterValues[0]);

packages/devextreme/js/__internal/grids/grid_core/filter_sync/utils.ts:183

• getFilterValueWithFilterRow/getFilterValueWithHeaderFilter call removeFieldConditionsFromFilter(filterValue, getColumnIdentifier(column)) even when the identifier is missing, which ends up syncing/removing conditions for an undefined field. Short-circuit when there is no identifier.

export const getFilterValueWithFilterRow = (
  filterValue: FilterValue,
  column: FilterSyncColumn,
): FilterValue => {
  const condition = getConditionFromFilterRow(column);

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Lite