vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

Components appended to state tree by ComponentRenderer are removed incorrectly #10126

Open xdenser opened 3 years ago

xdenser commented 3 years ago

Description of the bug / feature

When we reuse(cache) a component instance created for ComponentRenderer in vaadin-grid (maybe not only in grid) and grid is then removed from its parent and reinserted back again we get error.

Minimal reproducible example

See attached zip with skeleton starter, Just start it. Press "Show grid" button 2 times. See error in console. skeleton-starter-flow-master.zip

Expected behavior

Either mark somehow the component that it cannot be reused. Or make it reusable.

Actual behavior

See error in exmaple.

Versions:

- Vaadin / Flow version: 18.0.6
- Java version: 11
-
taefi commented 3 years ago

Hi @xdenser,

Thanks for creating the issue, however, I believe this issue has the same problem as #10002, which is trying to reuse the same component instance (in your case the label in renderer1) while the componentFunction is being called.

ComponentRenderers should be configured in a way that every time they take a model item and output a component instance, not to return the same instance always:

/**
* Creates a new ComponentRenderer that uses the componentFunction to
* generate new {@link Component} instances. The function takes a model item
* and outputs a component instance.
* <p>
* Some components may support several rendered components at once, so
* different component instances should be created for each different item
* for those components.
*
* @param componentFunction
*            a function that can generate new component instances
* @see #ComponentRenderer(SerializableFunction, SerializableBiFunction)
*/
public ComponentRenderer(
        SerializableFunction<SOURCE, COMPONENT> componentFunction) {
    this(componentFunction, null);
}

So I'm closing the issue since I think the above javadoc clearly mentions generate new Component instances which satisfies the Expected Behavior for this issue.

xdenser commented 3 years ago

Still I think there is problem with stateTree when such component is removed. Here is another example: skeleton-starter-flow-master.zip press "Show grid" at least 3 times.

Please note - this is synthentic simplified example. Real application may create some container instead of just label and then update/reuse components inside of container. So java doc should be clear that you cannot reuse not only created component but whole subtree. And you cannot cache any of components from subtree and reuse them or even change their properties after they have been removed. And the only way to detect that the component is unusable anymore is subscribe for "detach" event. Because isAttached - returns true, despite the fact that the component was removed.

taefi commented 3 years ago

Thanks @xdenser, as the issue is reproducible I reopen this issue to investigate more.