fix: use targeted item refresh when signal-bound item value changes#23913
Merged
fix: use targeted item refresh when signal-bound item value changes#23913
Conversation
Thread old item identity through DataRefreshEvent so that KeyMapper can remap keys when an item is replaced with a new instance (different identity). This replaces the refreshAll() workaround with a targeted single-item refresh for signal-bound grid items. Fixes #8938
Legioth
reviewed
Mar 17, 2026
...data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalDataCommunicator.java
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataProvider.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/DataCommunicator.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/DataKeyMapper.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/test/java/com/vaadin/flow/data/provider/KeyMapperTest.java
Show resolved
Hide resolved
Legioth
requested changes
Mar 17, 2026
- Rename refreshWithOldData to refresh overload for clarity - Replace "threading" terminology with clearer wording in javadocs - Simplify handleDataRefreshEvent to always use refresh(item, oldItem) - Make DataKeyMapper.refresh(T, T) throw instead of silently discarding - Add item swap in HierarchicalDataCommunicator cache context - Add key reuse assertion in KeyMapperTest - Fix spotless formatting
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/Cache.java
Outdated
Show resolved
Hide resolved
- Move rootCache context update into Cache.refreshItem(T, T) to avoid memory leaks and keep remapping logic encapsulated - Update indexToItemId mapping when item identity changes - Fix AbstractLazyDataViewTest to verify two-arg refresh calls
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/Cache.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractDataView.java
Outdated
Show resolved
Hide resolved
- Remove Cache.refreshItem(T, T) identity-changing overload - HierarchicalDataCommunicator.refresh(T, T) throws when identities differ, deferring TreeGrid bindItems support to a future change - AbstractDataView.replaceItem throws for non-AbstractDataProvider - DataViewUtils throws for non-AbstractDataView signal bindings
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/DataViewUtils.java
Outdated
Show resolved
Hide resolved
- Add DataProvider.replaceItem(T oldItem, T newItem) as default method - Add DataView.replaceItem(T oldItem, T newItem) to the interface - Make DataRefreshEvent.getOldItem() public - AbstractDataProvider overrides replaceItem to fire event with old item - AbstractDataView.replaceItem delegates to DataProvider.replaceItem - Remove all instanceof checks from DataViewUtils and AbstractDataView
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/DataChangeEvent.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
Swap parameter order in DataKeyMapper.refresh, KeyMapper.refresh, and DataCommunicator.refresh to match the (oldItem, newItem) convention used by DataProvider.replaceItem and DataView.replaceItem.
Legioth
reviewed
Mar 17, 2026
...data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalDataCommunicator.java
Show resolved
Hide resolved
Align replaceItem methods with the refresh/DataRefreshEvent convention where the new/primary item is always the first parameter.
Legioth
reviewed
Mar 17, 2026
...data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalDataCommunicator.java
Outdated
Show resolved
Hide resolved
Legioth
reviewed
Mar 17, 2026
flow-data/src/main/java/com/vaadin/flow/data/provider/KeyMapper.java
Outdated
Show resolved
Hide resolved
Align DataKeyMapper.refresh, KeyMapper.refresh, DataCommunicator.refresh, and HierarchicalDataCommunicator.refresh with the convention where the new/primary item is always the first parameter, matching refresh(T data) and replaceItem(T newItem, T oldItem).
Without this override, the default DataProvider.replaceItem falls back to refreshItem, losing the old item identity.
Use refreshItem(T newItem, T oldItem) as an overload of the existing refreshItem(T item), keeping naming consistent within DataProvider and DataView.
Legioth
approved these changes
Mar 17, 2026
|
vaadin-bot
pushed a commit
that referenced
this pull request
Mar 17, 2026
…23913) - Add a two-argument `refreshItem(T newItem, T oldItem)` overload to `DataProvider`, `DataView`, and related classes, enabling targeted refresh when an item's identity changes (e.g., signal-bound items replaced with new instances) - `DataRefreshEvent` now carries an optional old item reference via `getOldItem()`, allowing`DataCommunicator` and `KeyMapper` to remap from the old identity to the new one instead of failing to find the item - `HierarchicalDataCommunicator` throws `UnsupportedOperationException` for identity-changing refreshes, deferring TreeGrid `bindItems` support to a future change Fixes #8938
Artur-
added a commit
that referenced
this pull request
Mar 18, 2026
…23913) (#23922) - Add a two-argument `refreshItem(T newItem, T oldItem)` overload to `DataProvider`, `DataView`, and related classes, enabling targeted refresh when an item's identity changes (e.g., signal-bound items replaced with new instances) - `DataRefreshEvent` now carries an optional old item reference via `getOldItem()`, allowing`DataCommunicator` and `KeyMapper` to remap from the old identity to the new one instead of failing to find the item - `HierarchicalDataCommunicator` throws `UnsupportedOperationException` for identity-changing refreshes, deferring TreeGrid `bindItems` support to a future change Fixes #8938 Co-authored-by: Artur Signell <artur@vaadin.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Thread old item identity through DataRefreshEvent so that KeyMapper can remap keys when an item is replaced with a new instance (different identity). This replaces the refreshAll() workaround with a targeted single-item refresh for signal-bound grid items.
Fixes vaadin/flow-components#8938