GridCore - Fix eslint and add types to leaf helpers (m_aggregate_calculator, m_utils, m_accessibility, m_export)

[Tucchhaa]

No description provided.

[Copilot]

Copilot review overview

🟡 Changes recommended

The aggregate calculator’s new row type incorrectly excludes supported primitive data rows.

Review effort: Balanced
Findings: 1

Open (1)

• Aggregate selector types incorrectly exclude primitive rows · New

What changed in this PR

Adds TypeScript typing and ESLint-compliant refactors to GridCore helper modules.

Changes:

• Types aggregate calculation, export, accessibility, and filter helpers.
• Replaces legacy loops/utilities with typed native constructs.
• Updates aggregate test fixtures to object-based rows.

File	Description
aggregateCalculator.tests.js	Updates aggregate fixtures and selectors.
m_export.ts	Types export item preparation.
m_accessibility.ts	Types keyboard action registration.
summary/​types.ts	Makes the custom aggregator group index optional.
summary/​m_summary.ts	Types aggregate input data.
m_utils.ts	Types group-filter construction.
m_aggregate_calculator.ts	Types and modernizes aggregate calculations.

---

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

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are consistent, mechanically preserve existing behavior, and are covered by established aggregate and export tests.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Aggregate selector types incorrectly exclude primitive rows

[anna-shakhova]

can we get better type from usage and docs?

[Tucchhaa]

By checking usage of _data in source code its type should be: RawItemData[] | GroupData<RawItemData>[]

However, I have chosen to type unknown instead, because of this reasons:

1. Data items are just passed to handler: aggregate.selector(item) to extract needed value and their props are never directly read. Only the .items prop is read if data item is actually a group item: line of code

2. aggregateCalculator.tests.ts and dataSource.tests.ts have tests in which _data has number type. Defining type of _data would also require us to update these tests. (This is doable actually, not a lot of such tests)

I'm OK with both approaches. What do you think?

[anna-shakhova]

we already typed data in calculateTotalAggregates that calls constructor of that class, the only issue is tests then, we can add ts-expect-error directive to tests, if there are too many of them, but if not better fix

[anna-shakhova]

can we get better type from usage and docs?

[Tucchhaa]

I think unknown is the accurate type here, because custom accumulator can return any value. We would need to define a broad type: number | string | object | etc for it, but it wouldn't be useful anyways, because grid already treats summary results as unknowns.

[anna-shakhova]

can we get better types from usage and docs?

[Tucchhaa]

results is the same variable as container in this comment: #35464 (comment)
item depends on this resolution of this comment: #35464 (comment)

[anna-shakhova]

can we use here some type from our existing filter types or get one from usage and docs?

[Alyar666]

The existing filter types don't fit here. The group selector can be a function (e.g. calculateGroupValue), and a group key can be any value. DataFilter expects a string selector and a scalar value, so DataFilter[] fails with TS2345. The public FilterDescriptor is just any. combineFilters takes unknown[] anyway, so I'd keep unknown[] here.

[Copilot]

Copilot review overview

🟡 Changes recommended

The new View import references a nonexistent module and prevents TypeScript resolution.

Review effort: Balanced
Findings: 1

Open (1)

• Import View from the correct grid modules path · New

[Copilot]

Copilot review overview

🟡 Changes recommended

The new spread merge changes PivotGrid placeholder output and breaks an existing export assertion.

Review effort: Balanced
Findings: 1

Open (1)

• Preserve undefined-skipping merge semantics for span placeholders · New

Resolved since last review (1)

• Import View from the correct grid modules path

[Copilot]

Copilot review overview

🟡 Changes recommended

Two new type contracts incorrectly permit unsafe export calls and exclude supported primitive summary rows.

Review effort: Balanced
Findings: 1

Open (1)

• Require emptyCell to prevent recursive cloning stack overflow · New

Resolved since last review (1)

• Preserve undefined-skipping merge semantics for span placeholders

Previously missed (1)

In code that hasn't changed since last review

Preserve primitive row support in RawItemData annotation

packages/​devextreme/​js/​__internal/​grids/​data_grid/​summary/​m_summary.ts:354

This annotation excludes supported primitive rows because RawItemData is Record<string, unknown>. The calculator explicitly supports stores of primitives (for example, numeric rows with the this selector), so this private boundary should preserve that input instead of narrowing it to objects.

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are consistent type and lint refinements with no unresolved correctness issues found.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Require emptyCell to prevent recursive cloning stack overflow

[anna-shakhova]

we already typed data in calculateTotalAggregates that calls constructor of that class, the only issue is tests then, we can add ts-expect-error directive to tests, if there are too many of them, but if not better fix

[Copilot]

Copilot review overview

🟡 Changes recommended

Several new contracts exclude supported inputs or promise fields absent at runtime.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Keep emptyCell parameter optional · New
• Handle nullable group items in node types · New

Previously missed (2)

In code that hasn't changed since last review

Allow primitive mapped items in aggregation

packages/​devextreme/​js/​__internal/​grids/​data_grid/​summary/​m_summary.ts:354

This annotation also excludes primitive items produced by DataSource.map, although aggregation supports them (dataSource.tests.js:357-384 exercises a mapped number). Keep this method's data type aligned with the calculator's supported generic/unknown row type rather than restricting it to RawItemData.

NativeEventInfo does not match the callback payload

packages/​devextreme/​js/​__internal/​grids/​grid_core/​m_accessibility.ts:8

fireKeyDownEvent invokes this callback with only { event, handled } (ui/shared/accessibility.ts:71-78), so NativeEventInfo incorrectly promises component and element fields that are not present. Model the actual callback payload instead; otherwise future code can access fields that are always undefined.