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

Scheduled client side calls can get lost after detach/attach #18069

Open DiegoCardoso opened 11 months ago

DiegoCardoso commented 11 months ago

Description of the bug

If the component schedules calls to execute JS commands on the client side, they can sometimes be lost in certain scenarios, namely:

This issue is the root cause of this Grid ticket (https://github.com/vaadin/flow-components/issues/5645). The user initially sets the secondary grid (componentB) to not visible and later, the code sets it to visible and calls SplitLayout#setSecondary, which internally removes all children to re-attach again. That makes the scheduled call to set the selection mode to be lost and never be sent to the client.

Expected behavior

Scheduled calls are sent after being delayed, even after an element is detached and later re-attached.

Minimal reproducible example

Clone the https://github.com/DiegoCardoso/flow-scheduled-js-call-issue project and start it with ./mvnw

On the page, there are two <my-component> elements: the first one is visible and the second one is not.

Versions

mcollovati commented 11 months ago

Pending Javascript invocations are purged when the component is detached to prevent memory leaks. So point 2 and 3 are unfortunately somehow expected.

Javadoc on executeJs says If the element is not attached or not visible, the function call will be deferred until the element is attached and visible. but it effectively does not mention what happens if the elements get detached.

Point 2 could potentially be handled somehow, since detach and attach occur during the same request. Point 3 could be more difficult.

For the specific grid issue, would it make sense to call updateSelectionModeOnClient() also on attach? Or will this break some other thing?

mshabarov commented 11 months ago

This is expected behaviour, but I propose to focus on the use case and figure out a way how to make it work with current logic in Flow, e.g. adding JS commands within onAttach hooks.

DiegoCardoso commented 11 months ago

For the specific grid issue, would it make sense to call updateSelectionModeOnClient() also on attach? Or will this break some other thing?

That's possible to use as a fix. I just wanted to confirm whether this was expected to happen in the case described mainly in point 2.

Point 3 is not a problem for Grid, because the updateSelectionModeOnClient is called from the ArrayUpdater#initialize method that is called when the Grid is detached and re-attached in different round-trips (which doesn't occur when these events happen in the same round-trip).

mshabarov commented 11 months ago

The latest patch for runWhenAttached has been reverted in https://github.com/vaadin/flow/pull/18131 due to regressions in components. This issue might be related and might be fixed after revert.