Newly generated keys no longer match expandedItems after the old keys are removed from KeyMapper

[vursen]

Description of the bug

As @tomivirkki discovered in vaadin/web-components#7097 (comment), Flow HierarchicalCommunicationController can passivate some of the item keys when the items are expanded recursively and request KeyMapper to remove the keys for the passivated items. Since TreeGrid no longer uses DataProvider.getId() for the item identity after vaadin/flow-components#3251, the items get fresh new keys that no longer match the old ones in the expandedItems array.

Expected behavior

Not sure if it would be better to revert the vaadin/flow-components#3251 in TreeGrid (to have it use DataProvider.getId() for the item identity again) or fix the passivating logic in HierarchicalCommunicationController.

Minimal reproducible example

@Route("repro")
public class Repro extends Div {

    public Repro() {
        var grid = new TreeGrid<String>();
        grid.setPageSize(10);
        grid.addHierarchyColumn(String::toString).setHeader("Item");

        var data = new TreeData<String>();
        data.addItems(null, "Granddad");
        for (int i = 0; i < 20; i++) {
            data.addItems("Granddad", "Dad " + i);
            for (int j = 0; j < 20; j++) {
                data.addItems("Dad " + i, "Son " + i + "/" + j);
            }
        }

        grid.setDataProvider(new TreeDataProvider<>(data));

        add(grid);

        add(new NativeButton("Expand all",
            e -> grid.expandRecursively(data.getRootItems(), 1)));

        add(new NativeButton("Print \"Dad 0\" key",
            e -> System.out.println("Dad 0 key: " + grid.getDataCommunicator().getKeyMapper().key("Dad 0"))));
    }
}

...and the stack trace after clicking "Expand all":

at com.vaadin.flow.data.provider.KeyMapper.remove(KeyMapper.java:140)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.lambda$doUnregister$4(HierarchicalCommunicationController.java:324)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.doUnregister(HierarchicalCommunicationController.java:320)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.unregisterPassivatedKeys(HierarchicalCommunicationController.java:312)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.flush(HierarchicalCommunicationController.java:145)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalDataCommunicator.lambda$requestFlush$e15592a2$1(HierarchicalDataCommunicator.java:118)

This is followed by an additional request from Grid due to HierarchicalCommunicationController clearing one of the parent items:

at com.vaadin.flow.component.treegrid.TreeGrid$TreeGridUpdateQueue.clear(TreeGrid.java:125)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.clear(HierarchicalCommunicationController.java:219)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.collectChangesToSend(HierarchicalCommunicationController.java:173)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalCommunicationController.flush(HierarchicalCommunicationController.java:135)
at com.vaadin.flow.data.provider.hierarchy.HierarchicalDataCommunicator.lambda$requestFlush$e15592a2$1(HierarchicalDataCommunicator.java:118)

Versions

• Vaadin / Flow version: 24.4 and earlier
• Java version: 17
• OS version: Mac OS