flow icon indicating copy to clipboard operation
flow copied to clipboard

executeJs from detach listener gets run for detached element but without reference to the element

Open Legioth opened this issue 2 years ago • 0 comments

Description of the bug / feature

If executeJs is used for an element that is currently being detached, then that executeJs invocation will be sent to the client and executed immediately. The JS expression is run without a defined this value, which means that window will be used instead of the expected element. Furthermore, any executeJs that has been enqueued for the element before the detach will also be sent, whereas any executeJs run after detaching will remain in the queue and sent only later if the element is attached again.

If no executeJs is run from inside the detach handler, then any previously enqueued invocation will remain there until the element is attached again, regardless of whether that executeJs was run before or after the actual detach.

Originally reported through https://github.com/vaadin/cookbook/pull/158#issuecomment-868516835.

Minimal reproducible example

@Route("")
public class DetachedExecute extends VerticalLayout {
    public DetachedExecute() {
        Span spanWithDetachListener = new Span("With detach");
        spanWithDetachListener.addDetachListener(event -> {
            executeLog("Detach listener", spanWithDetachListener);
        });
        Span spanWithoutDetachListener = new Span("Without detach");

        add(spanWithoutDetachListener, spanWithDetachListener);

        add(new Button("Remove element with detach listener", event -> {
            executeLog("Before remove", spanWithDetachListener);
            remove(spanWithDetachListener);
            executeLog("After remove", spanWithDetachListener);
        }));

        add(new Button("Remove element without detach listener", event -> {
            executeLog("Before remove", spanWithoutDetachListener);
            remove(spanWithoutDetachListener);
            executeLog("After remove", spanWithoutDetachListener);
        }));

        add(new Button("Add elements", event -> {
            addComponentAsFirst(spanWithDetachListener);
            addComponentAsFirst(spanWithoutDetachListener);
        }));
    }

    private static void executeLog(String phase, Component target) {
        target.getElement().executeJs("console.log($0, this)", phase);
    }
}
  1. Open the view and the browser console
  2. Click the buttons in sequence from top to bottom and observe the Before remove, Detach listener and After remove messages in the browser console.

Expected behavior

I would spontaneously expect that no executeJs would be sent to the browser for an element that wasn't attached when the response was sent. This would mean that nothing is logged until clicking the last button to add the elements back.

I can, however, see the benefit of giving the element a chance to do client-side cleanup when it's detached. In that case, I would however expect that this references the element since there would otherwise not be any practical way to actually perform any cleanup targeted to this particular element. In that case, I would also expect the executeJs that was run before detaching to always be run since it's quite weird that adding a specific detach listener changes the behaviour of things not related to that detach listener.

Actual behavior

  1. The first button removes the element with a detach listener, and logs both Before remove and Detach listener with this referencing window.
  2. The second button removes the element without a detach listener. Nothing is logged.
  3. The third button adds back both elements and causes all the remaining executions to be run: Before remove for the element without a detach listener and After remove for both elements.

Versions:

- Vaadin / Flow version: Tested with 14.6.1, 20.0.5 and 21.0.0.alpha10

Legioth avatar Jul 28 '21 12:07 Legioth