Grids: convert the pivot grid LocalStore into an ES6 class and remove the Class.inherit leftovers - #35411
Grids: convert the pivot grid LocalStore into an ES6 class and remove the Class.inherit leftovers#35411EugeniyKiyashko wants to merge 2 commits into
Conversation
… the Class.inherit leftovers in grid_core modules, the pivot data source and QUnit tests
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new LocalStore.filter forwarding fails TypeScript overload checking with an unknown[] argument list.
Review effort: Balanced
Findings: 1
What changed in this PR
Modernizes PivotGrid storage and grid module extension code by removing legacy Class.inherit patterns.
Changes:
- Converts PivotGrid
LocalStoreand test stores to ES6 classes. - Detects custom PivotGrid stores by their required methods.
- Removes object-based grid extenders and obsolete test comments.
| File | Description |
|---|---|
packages/devextreme/js/__internal/grids/pivot_grid/local_store/m_local_store.ts |
Converts LocalStore to an ES6 class. |
packages/devextreme/js/__internal/grids/pivot_grid/data_source/m_data_source.ts |
Adds method-based custom store detection. |
packages/devextreme/js/__internal/grids/grid_core/m_modules.ts |
Removes legacy object extender handling. |
packages/devextreme/js/__internal/grids/grid_core/m_types.ts |
Restricts extenders to class-producing functions. |
packages/devextreme/testing/tests/DevExpress.ui.widgets.pivotGrid/dataController.tests.js |
Converts mock stores to ES6 classes. |
packages/devextreme/testing/tests/DevExpress.ui.widgets.dataGrid/grid_core.modules.tests.js |
Removes obsolete legacy-class examples. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| type ControllersExtender = { | ||
| [P in keyof Controllers]: ((Base: ModuleType<Controllers[P]>) => ModuleType<Controllers[P]>) | ||
| | Record<string, any>; | ||
| [P in keyof Controllers]: (Base: ModuleType<any>) => ModuleType<Controllers[P]>; |
There was a problem hiding this comment.
lets return here prev more strict type and add ts-expect-error to a single place where the typing will fail (packages/devextreme/js/__internal/grids/grid_core/adaptivity/adaptivity_module.ts)
| [P in keyof Controllers]: (Base: ModuleType<any>) => ModuleType<Controllers[P]>; | |
| [P in keyof Controllers]: (Base: ModuleType<Controllers[P]>) => ModuleType<Controllers[P]>; |
There was a problem hiding this comment.
Done in ae48aa6: both extender maps are strict again and the single registration that no longer type-checks (adaptivity_module.ts, data) carries a @ts-expect-error. tsc and eslint are clean.
| type ViewsExtender = { | ||
| [P in keyof Views]: ((Base: ModuleType<Views[P]>) => ModuleType<Views[P]>) | ||
| | Record<string, any>; | ||
| [P in keyof Views]: (Base: ModuleType<any>) => ModuleType<Views[P]>; |
There was a problem hiding this comment.
| [P in keyof Views]: (Base: ModuleType<any>) => ModuleType<Views[P]>; | |
| [P in keyof Views]: (Base: ModuleType<Views[P]>) => ModuleType<Views[P]>; |
| const classType = currentType as { inherit: (type: unknown) => unknown }; | ||
| extendTypes[name] = classType.inherit(extender); | ||
| } | ||
| if (currentType && isFunction(extender)) { |
There was a problem hiding this comment.
With the .inherit branch gone, the guard turns a non-function extender into a silent no-op. DataGrid.registerModule / TreeList.registerModule can be called at runtime even though it is not public API
- before this PR:
TypeError: classType.inherit is not a function(since 24.1) - with this PR: the grid is created, the extender is ignored, and the customization quietly does nothing
Without the guard it still fails loudly: TypeError: extender is not a function, thrown synchronously from new DataGrid(...) (_init → processModules → here)
| if (currentType && isFunction(extender)) { | |
| if (currentType) { |
There was a problem hiding this comment.
Done in ae48aa6. The literal if (currentType) does not compile while moduleExtenders is Record<string, unknown> (TS18046), so the parameter is now typed as Record<string, ModuleTypeExtender | undefined>: a non-function extender fails with TypeError: extender is not a function, and an explicitly undefined entry is skipped like a missing key (Partial allows it).
No description provided.