[mwc-dialog] events are fired after disconnection from DOM

[43081j]

Describe the bug

Due to this:

https://github.com/material-components/material-web/blob/23ed27ac770da1799dac1ee547aa5ba6248d5e3d/packages/dialog/mwc-dialog-base.ts#L363-L373

Specifically the close line, means that we will later emit a closed event among other things even though our dialog is no longer in a DOM tree.

This results in really funky behaviour, and stale references to things being held around because of the foundation's setTimeout call (to fire closed when the animation ends).

To Reproduce

• Setup an mwc-dialog with the closed event bound to a console.log or similar
• Disconnect it from DOM
• Observe that your handler is called

---

Happy to fix this if you can point me in the right direction.

Maybe we can have a check for isConnected in our notify* functions we pass into the foundation?

Although in particular we really shouldn't be calling this when disconnected:

https://github.com/material-components/material-components-web/blob/ec54d904621360bcb27e6d3cd1f33112e2a54aac/packages/mdc-dialog/foundation.ts#L197-L201

And that wouldn't be fixed by early exiting the notify functions.

Maybe this is what it should be?

      notifyClosed: (action) => {
        if (!this.closingDueToDisconnect) {
          this.emitNotification('closed', action),
        }
      },
      notifyClosing: (action) => {
        if (!this.closingDueToDisconnect) {
          // Don't set our open state to closed just because we were
          // disconnected. That way if we get reconnected, we'll know to
          // re-open.
          this.open = false;
          this.emitNotification('closing', action);
        }
      },

I noticed we already do a closingDueToDisconnect check. We then keep the dialog open if the close was triggered by a disconnect... but we then go on to fire closing/closed logic anyway. That part doesn't feel right if we actually didn't close it, we left it open for when it is reconnected.

[asyncliz]

What is the use case for removing the dialog from the DOM while it is open? I would expect a close event to be dispatched (since it is closing by leaving the DOM).

[43081j]

Except your current logic means it doesn't close when disconnected from dom. It explicitly tells it not to flip the open flag in the case, so when it gets reconnected, it will still be open.

That doesn't match up with the events it fires.

In that case it'll actually stay open as far as state is concerned but will fire a closed event. That itself sounds like a bug.

As for a use case for disconnecting it. Any time we disconnect a component from DOM that contains a dialog. For example, changing page where each page is a component and the one you were on had a dialog open.

But really it is possible to do so we should assume sometime somewhere is doing it.

[asyncliz]

Thanks for the explanation! We'll add it to our backlog. It probably just needs to set this.open = false in disconnectedCallback.

[asyncliz]

Obsolete with M3