Core(Data): remove m_ prefix and improve typing - #35050
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a few correctness/robustness gaps in newly-touched typed code paths (notably LocalStore JSON parsing and error-id handling) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR modernizes the internal Data layer by removing the legacy m_-prefixed modules, replacing them with typed TypeScript implementations, and updating the public/common reexports and internal call sites accordingly.
Changes:
- Replaced multiple
__internal/data/m_*modules with new typed__internal/data/*TypeScript modules and updated all reexports/imports. - Introduced typed OData request/query infrastructure (
RequestDispatcher,odataquery adapter) and typed data helpers (utils,store_helper,array_utils,query,remote_query). - Reduced/relocated
@ts-expect-errorusage across UI and grid modules, adding more explicit typing where possible.
File summaries
| File | Description |
|---|---|
| packages/devextreme/js/common/data/utils.js | Reexport now points to typed internal data utils module. |
| packages/devextreme/js/common/data/store_helper.js | Reexport now points to typed internal store helper module. |
| packages/devextreme/js/common/data/remote_query.js | Reexport now points to typed internal remote query module. |
| packages/devextreme/js/common/data/query.js | Reexport now points to typed internal query module. |
| packages/devextreme/js/common/data/odata/request_dispatcher.js | Reexport now points to typed internal OData request dispatcher. |
| packages/devextreme/js/common/data/odata/query_adapter.js | Reexport now points to typed internal OData query adapter. |
| packages/devextreme/js/common/data/local_store.js | Reexport now points to typed internal local store. |
| packages/devextreme/js/common/data/errors.js | Reexport now points to typed internal data errors module. |
| packages/devextreme/js/common/data/endpoint_selector.js | Reexport now points to typed internal endpoint selector. |
| packages/devextreme/js/common/data/data_source/utils.js | Reexport now points to typed internal data_source utils. |
| packages/devextreme/js/common/data/data_source/operation_manager.js | Reexport now points to typed internal operation manager. |
| packages/devextreme/js/common/data/array_utils.js | Reexport now points to typed internal array utils. |
| packages/devextreme/js/__internal/ui/shared/ui.editor_factory_mixin.ts | Removes now-unneeded TS suppression around data source normalization. |
| packages/devextreme/js/__internal/ui/selection/selection.strategy.ts | Removes TS suppression around dataQuery(...).toArray() usage. |
| packages/devextreme/js/__internal/ui/selection/selection.strategy.standard.ts | Adds explicit generic array typing + keeps a more specific @ts-expect-error note. |
| packages/devextreme/js/__internal/ui/selection/selection.strategy.deferred.ts | Removes TS suppression around dataQuery(...).toArray() usage. |
| packages/devextreme/js/__internal/ui/hierarchical_collection/data_adapter.ts | Replaces generic TS suppression with a more specific rationale around toArray() typing. |
| packages/devextreme/js/__internal/ui/collection/collection_widget.live_update.ts | Removes TS suppression around deferred/when usage. |
| packages/devextreme/js/__internal/ui/collection/collection_widget.edit.ts | Removes TS suppression around load result normalization. |
| packages/devextreme/js/__internal/scheduler/utils/loader/utils.ts | Updates normalizeDataSourceOptions import + documents internal/public option-type mismatch. |
| packages/devextreme/js/__internal/grids/tree_list/data_source_adapter/m_data_source_adapter.ts | Removes multiple TS suppressions around query and data helpers. |
| packages/devextreme/js/__internal/grids/pivot_grid/remote_store/m_remote_store.ts | Removes TS suppression around load result normalization. |
| packages/devextreme/js/__internal/grids/pivot_grid/local_store/m_local_store.ts | Removes TS suppression around dataQuery(...).toArray() usage. |
| packages/devextreme/js/__internal/grids/pivot_grid/data_source/m_data_source.ts | Removes TS suppression around data source option normalization. |
| packages/devextreme/js/__internal/grids/new/grid_core/filtering/header_filter/legacy_header_filter.ts | Updates utils import path + removes TS suppression around normalization call. |
| packages/devextreme/js/__internal/grids/new/grid_core/data_controller/utils.ts | Updates applyBatch import + documents internal/public option-type mismatch. |
| packages/devextreme/js/__internal/grids/new/grid_core/data_controller/public_methods.ts | Updates keysEqual import to the new non-m_ utils module. |
| packages/devextreme/js/__internal/grids/grid_core/search/m_search.ts | Removes TS suppressions around chained query usage. |
| packages/devextreme/js/__internal/grids/grid_core/m_utils.ts | Removes TS suppression around data source option normalization. |
| packages/devextreme/js/__internal/grids/grid_core/header_filter/m_header_filter.ts | Removes/relocates TS suppressions and adds more specific toArray() typing note. |
| packages/devextreme/js/__internal/grids/grid_core/data_source_adapter/m_data_source_adapter.ts | Removes TS suppressions around applyBatch usage. |
| packages/devextreme/js/__internal/grids/grid_core/columns_controller/m_columns_controller.ts | Removes TS suppression around data source option normalization. |
| packages/devextreme/js/__internal/grids/data_grid/summary/m_summary.ts | Removes TS suppressions around query/group helper usage. |
| packages/devextreme/js/__internal/grids/data_grid/grouping/m_grouping.ts | Adds a specific TS suppression explaining selector-type widening. |
| packages/devextreme/js/__internal/grids/data_grid/grouping/m_grouping_expanded.ts | Removes TS suppression around query usage. |
| packages/devextreme/js/__internal/data/utils.ts | New typed replacement for m_utils (criteria/sorting helpers, Deferred helpers, etc.). |
| packages/devextreme/js/__internal/data/store_helper.ts | New typed replacement for m_store_helper (queryByOptions + grouping/sorting helpers). |
| packages/devextreme/js/__internal/data/remote_query.ts | New typed replacement for m_remote_query (remote task queue + adapter execution). |
| packages/devextreme/js/__internal/data/query.ts | New typed replacement for m_query with overloads for array vs remote query sources. |
| packages/devextreme/js/__internal/data/query_implementation.ts | Swaps internal remote implementation import to the new module. |
| packages/devextreme/js/__internal/data/odata/store.ts | Types _requestDispatcher as RequestDispatcher now that it’s explicitly declared/typed. |
| packages/devextreme/js/__internal/data/odata/request_dispatcher.ts | New typed replacement for m_request_dispatcher. |
| packages/devextreme/js/__internal/data/odata/query_adapter.ts | New typed replacement for m_query_adapter (criteria compilation + request parameterization). |
| packages/devextreme/js/__internal/data/odata/m_request_dispatcher.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/odata/m_query_adapter.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/odata/context.ts | Updates imports from m_errors to errors. |
| packages/devextreme/js/__internal/data/m_utils.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/m_store_helper.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/m_remote_query.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/m_query.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/m_endpoint_selector.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/m_array_utils.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/local_store.ts | Adds typings and safer loading shape checks for localStorage-backed store. |
| packages/devextreme/js/__internal/data/errors.ts | Improves typing for error handler plumbing and debug logging. |
| packages/devextreme/js/__internal/data/endpoint_selector.ts | New typed replacement for m_endpoint_selector. |
| packages/devextreme/js/__internal/data/data_source/utils.ts | New typed replacement for data_source/m_utils and shared DS helper functions. |
| packages/devextreme/js/__internal/data/data_source/operation_manager.ts | Adds explicit field and method typings for operation tracking/cancelation. |
| packages/devextreme/js/__internal/data/data_source/m_utils.ts | Removes legacy m_ implementation. |
| packages/devextreme/js/__internal/data/data_source/data_source.ts | Narrows internal types for _aggregationTimeoutId and _mapFunc; removes one TS suppression. |
| packages/devextreme/js/__internal/data/data_controller/data_controller.ts | Removes TS suppression around normalized data source option construction. |
| packages/devextreme/js/__internal/data/custom_store.ts | Adjusts result typing to align with typed enumerate() return (unknown). |
| packages/devextreme/js/__internal/data/array_utils.ts | New typed replacement for m_array_utils (applyBatch/insert/update/remove, etc.). |
| packages/devextreme/js/__internal/data/array_store.ts | Removes TS suppressions now that array/utils + data/utils are typed. |
| packages/devextreme/js/__internal/data/abstract_store.ts | Adds targeted TS suppression documenting public/internal query interface mismatch. |
Review details
Suppressed comments (2)
packages/devextreme/js/__internal/data/errors.ts:72
- In
handleError(),idcan become a non-string if an external error object provides a non-string__id. Sinceerrors.log()decides severity by matchingidagainst/^E\d+$/and/^W\d+$/, coercing to string here avoids misclassification.
packages/devextreme/js/__internal/data/local_store.ts:90 JSON.parse(raw)can throw if localStorage data is corrupted or manually edited, which would break LocalStore initialization. Since this method is already being adjusted, it would be safer to guard the parse and fall back to an empty array (optionally clearing the bad entry).
- Files reviewed: 64/64 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
37d2190 to
fd54d63
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are at least two confirmed runtime correctness issues (empty-group handling in queryByOptions and 'undefined' URL construction in RequestDispatcher) that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 64/64 changed files
- Comments generated: 2
- Review effort level: Lite
fd54d63 to
bda4be1
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new typed modules introduce at least one TypeScript-breaking type issue (indexing DataSourceOptionsInput) and one runtime footgun (RequestDispatcher producing "undefined" URLs when url is omitted).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
packages/devextreme/js/__internal/data/odata/request_dispatcher.ts:38
RequestDispatcherOptions.urlis optional, but the constructor coerces it withString(dispatcherOptions.url), so omittingurlproduces the literal string "undefined" (and that value is later used to build request URLs). At minimum, coerce missing values to an empty string to avoid accidental "undefined/..." requests.
this._url = String(dispatcherOptions.url).replace(/\/+$/, '');
- Files reviewed: 64/64 changed files
- Comments generated: 1
- Review effort level: Lite
No description provided.