EventListener does not remove event listeners from the same elements that it registered them on if `getSources` changes

[jrhodkinson]

When EventListener mounts, it attaches event listeners to all of the elements returned by getSources, which is either the collection of its children, or document if it has no children. When it unmounts, instead of removing these specific event listeners, it calls getSources again - but the collection of children may have changed in the meantime.

Usually when EventListener unmounts, it takes its children with it. However, in specific circumstances, this can cause the entire dash application to become non-functional.

• If at the time of the initial call to getSources there were no children, the event listener is attached to the document
• If children are later added dynamically, then when EventListener unmounts it will not remove the event listener from document
• Then, when the event fires, the event handler will try to set the props on a nonexistent object
• This corrupts dash's state and results in the incredibly difficult to diagnose error message

An object was provided as `children` instead of a component, string, or number (or list of those). Check the children property that looks something like:
{
  "props": {
    "n_events": 3
  }
}

• I've seen this propagate into something which can make the entire app cease to function, but I can't pin down the exact steps that make that happen. When it does happen, the above error message seems to contain a much larger object.

I ran into this by putting an EventListener around dash_quill which lazy loads the Quill editor with no placeholder in the DOM. Having understood the problem, I was able to workaround the issue by wrapping the quill editor in a div rather than nesting it directly below the EventListener.

Suggestions

1. Save the list of elements we add event listeners to and use the same list to remove them at unmount
2. logging=True didn't give that much useful information (except show me that the event listener was still being fired even after I expected it to be unmounted, which eventually got me on the right track). Perhaps we can extend the logging to log the list of elements that we're modifying the event listeners of on mount/unmount?
3. I found it very unintuitive that the EventListener defaulted to adding one to the document even though I passed in a non-empty list of children in Python. I know that this is documented, but it still seemed unexpected (I've been using plenty of EventListeners around my application, and never came across this functionality). In an ideal world I think defaulting to document would be an opt-in feature, but obviously this breaks backwards compatibility.

[emilhe]

Do you have an MWE demonstrating the issue? While I can understand the conceptual issue, an MWE would ensure that my proposed solution actually works. So far, I haven't been able to reproduce any errors.

EDIT: I have just pushed a 1.0.20 release with a proposed fix. Could you test it and check it it resolve the issue?

[jrhodkinson]

Here is a MWE:

from dash import html, no_update, Output, Input
from dash_extensions import EventListener

from our_app.app import app


def create_layout():
    return html.Div(
        children=[
            html.Div(
                id="event-listener-container",
            ),
            html.Button(
                id="button-add-event-listener",
                children="Mount event listener"
            ),
            html.Button(
                id="button-add-children",
                children="Add children to event listener"
            ),
            html.Button(
                id="button-remove-event-listener",
                children="Unmount event listener"
            ),
            html.Pre(
                id="event-listener-events",
            ),
        ]
    )


@app.callback(
    Output("event-listener-container", "children"),
    Input("button-add-event-listener", "n_clicks"),
    prevent_initial_call=True
)
def add_event_listener(n_clicks):
    if n_clicks is None:
        return no_update

    return EventListener(
        id="event-listener",
        events=[{"event": "click"}],
        children=None,
    )


@app.callback(
    Output("event-listener-events", "children"),
    Input("event-listener", "n_events"),
    prevent_initial_call=True
)
def update_event_listener_events(n_events):
    if n_events is None:
        return no_update

    return f"Event listener 'click' has been triggered {n_events} times"


@app.callback(
    Output("event-listener", "children"),
    Input("button-add-children", "n_clicks"),
    prevent_initial_call=True
)
def add_children(n_clicks):
    if n_clicks is None:
        return no_update

    return html.Div("This is a child of the event listener")


@app.callback(
    Output("event-listener-container", "children"),
    Input("button-remove-event-listener", "n_clicks"),
    prevent_initial_call=True
)
def remove_event_listener(n_clicks):
    if n_clicks is None:
        return no_update

It was important that the EventListener wasn't in the layout from the outset. I'm not sure why, but the example wouldn't reproduce unless the EventListener was added dynamically.

1. Click 'Mount event listener', this will attach an event listener to document
2. Click around the page, note that the event listener fires
3. Click 'Add children to event listener', this adds a child to the event listener
4. Click around the page, the event still fires - it's not listening on the child, but still on the document
5. Click 'Unmount event listener', this remove the EventListener component from the dom but not the event listener from the document
6. Click anywhere, callback error occurs

[jrhodkinson]

I'll aim to try out the fix within the next few days. I briefly looked at the code and it seems like it should prevent the callback error.

However, now I'm wondering what should happen when EventListener children are changed dynamically. E.g. above in step 4, should the event listener still be capturing events from the document at this point, or should it have switched to only be monitoring events on the children? I think the latter is more intuitive even though it's more complex.