CardView: track items dependencies without bare signal reads (T1334012) - #34860
CardView: track items dependencies without bare signal reads (T1334012)#34860anna-shakhova wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the new-grid/CardView reactive layer against minification (compress.pure_getters) removing “load-bearing” signal .value reads, by introducing a track() helper and updating several effects/computeds to use explicit dependency tracking. It also adds regression tests (including a build-time guard) to ensure runtime option/data changes continue to propagate into derived UI state (toolbar buttons, accessibility status/description, items rendering, etc.).
Changes:
- Add
track(...values)to the state manager primitives and re-export it through the dev/prod barrels. - Replace “bare”
.valuereads used only for dependency subscription withtrack(...)in multiple controllers. - Add Jest coverage for
track, selection toolbar button updates, action option rebinding, remoteOperations reloads, and CardView a11y/status updates; plus a minification guard test.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/js/__internal/grids/new/grid_core/selection/controller.ts | Uses track() in an effect to retain data-driven dependencies under minification. |
| packages/devextreme/js/__internal/grids/new/grid_core/selection/controller.test.ts | Adds coverage for selection toolbar button updates when paging changes. |
| packages/devextreme/js/__internal/grids/new/grid_core/search/controller.ts | Makes highlighted-text computation reactive to highlight options changes (.value vs .peek()). |
| packages/devextreme/js/__internal/grids/new/grid_core/options_controller/options_controller_base.ts | Uses track() so actions recompute when their backing option changes even after minification. |
| packages/devextreme/js/__internal/grids/new/grid_core/options_controller/options_controller_base.test.ts | Adds test ensuring actions rebind to the latest option handler after execution. |
| packages/devextreme/js/__internal/grids/new/grid_core/items_controller/utils.test.ts | Adds unit tests for getColumnLayoutKey behavior (layout vs excluded properties). |
| packages/devextreme/js/__internal/grids/new/grid_core/items_controller/items_controller.ts | Refactors CardView items dependencies to reduce unnecessary recomputes while keeping needed reactivity. |
| packages/devextreme/js/__internal/grids/new/grid_core/items_controller/items_controller.test.ts | Adds tests proving items recomputes when columns layout or highlight options change. |
| packages/devextreme/js/__internal/grids/new/grid_core/data_controller/data_controller.ts | Uses track() to ensure remote options changes trigger reload logic under minification. |
| packages/devextreme/js/__internal/grids/new/grid_core/data_controller/data_controller.test.ts | Adds regression test for runtime remoteOperations updates reloading the store. |
| packages/devextreme/js/__internal/grids/new/grid_core/accessibility/render.test.ts | Adds DOM-level test ensuring status text updates after description changes. |
| packages/devextreme/js/__internal/grids/new/grid_core/accessibility/controller.ts | Uses track() to preserve the description dependency for status announcement logic. |
| packages/devextreme/js/__internal/grids/new/grid_core/accessibility/controller.test.ts | Adds controller-level test for componentStatus updating after a column visibility change. |
| packages/devextreme/js/__internal/core/state_manager/reactive_primitives.test.ts | Adds tests validating track() drives recomputation for effects and computeds. |
| packages/devextreme/js/__internal/core/state_manager/prod/reactive_primitives/index.ts | Introduces the track() primitive implementation to survive pure_getters. |
| packages/devextreme/js/__internal/core/state_manager/prod/index.ts | Re-exports track() from the prod barrel. |
| packages/devextreme/js/__internal/core/state_manager/dev/reactive_primitives/index.ts | Re-exports track() for dev builds. |
| packages/devextreme/js/__internal/core/state_manager/dev/index.ts | Re-exports track() from the dev barrel. |
| packages/devextreme/build/pure-getters-guard.test.ts | Adds a build-level Jest guard to detect .value reads lost specifically due to pure_getters. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d6d269d to
f9f11d2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/devextreme/js/__internal/grids/new/grid_core/items_controller/items_controller.test.ts:153
- The comment says
itemsreads highlight options non-reactively, butSearchController.getHighlightedTextnow usesthis.highlightTextOptions.value(reactive) soitemsdoes subscribe to highlight option changes reactively. The comment is misleading and should be updated to match the current dependency mechanism.
// `items` reads columns and highlight options non-reactively, so the subscriptions are separate.
// Each test changes only one of those signals, so a recompute proves the subscription.
packages/devextreme/build/pure-getters-guard.test.ts:101
- This test minifies every module in the corpus just to assert the corpus contains at least one
.valueread. That adds an extra full minification pass over the whole directory on top of the per-module assertions below, which can significantly slow the Jest suite.
const totalReads = (
await Promise.all(
MODULES.map(async (m) => ((await minify(m, false)).match(VALUE_READ) ?? []).length),
)
).reduce((total, n) => total + n, 0);
| ): HighlightedTextItem[] | null => splitHighlightedText( | ||
| text, | ||
| this.highlightTextOptions.peek(), | ||
| this.highlightTextOptions.value, |
There was a problem hiding this comment.
reads .value, not .peek(), on purpose.
Callers inside a computed - ItemsController renders every field through here - then subscribe to the highlight options naturally, instead of the caller having to read the signal separately just to register the dependency.
No description provided.