skip to Main Content

Consider a webpage that contains a button which triggers the opening of a modal:

<dialog></dialog>

<button>Open modal</button>
const trigger = document.querySelector(`button`);
const wrapper = document.querySelector(`dialog`);

trigger.addEventListener(`click`, () => {
  wrapper.replaceChildren(new Modal());
  wrapper.showModal();
});

The modal is a custom element:

class Modal extends HTMLElement {
  connectedCallback() {
    this.innerHTML = `<button onclick="this.closest('dialog').close()">Close modal</button>`;
    
    this.closest(`dialog`).addEventListener(`close`, () => {
      console.log(`Modal closed`);
    });
  }
}

customElements.define(`app-modal`, Modal);

Here’s a demo: https://codepen.io/RobertAKARobin/pen/OJGJQro

What I expect is each time the Close modal button is clicked, the console logs 'Modal closed'.

What actually happens is the console logs 'Modal closed' once for each time the modal has been closed so far. So on click 1 it logs the message 1 time, on click 2 it’s 2 times, click 3 is 3 times, click 4 is 4, etc.

This indicates the Modal instance is not being automatically garbage-collected when it is disconnected from the DOM. I suspect this is because of the event listener. However, the listener function does not itself contain any references to the Modal instance (other than via this).

What would be the correct way to set up Modal so that it doesn’t prevent itself from being garbage-collected?

2

Answers


  1. The problem comes from your Modal custom element and has nothing to do with GC.

    You add an event listener in connectedCallback but never remove it.
    The dialog doesn’t leave the DOM, the previous custom elements are not garbage collected (yet) and thus their listeners are still active.

    If you properly disconnect the custom element from the DOM, you can use this:

    class Modal extends HTMLElement {
      constructor() {
        super();
        this.onDialogClose = this.onDialogClose.bind(this);
      }
    
      connectedCallback() {
        this.innerHTML = `<button onclick="this.closest('dialog').close()">Close modal</button>`;
        
        this.closest('dialog').addEventListener('close', this.onDialogClose);
      }
    
      disconnectedCallback() {
        this.closest('dialog').removeEventListener('close', this.onDialogClose);
      }
    
      onDialogClose() {
        console.log('Modal closed');
      }
    }
    
    customElements.define('app-modal', Modal);
    
    Login or Signup to reply.
  2. The event handler is not automatically removed from the host element (the dialog) that stays across open/close cycles. Thus the repeated event handlers are being called on each close event, even from the disconnected custom-elements (the listener is still retained).

    I think it could be done in a better way but anytime there is an addEventListener in the connectedCallback, there should be a corresponding removeEventListener call in the disconnectedCallback.

    Thus the listener (e.g. this.listener) and the host reference (e.g. this.host) from the connectedCallback must be somehow retained to be later released.

    Please take a look at the forked demo with the fix applied: https://codepen.io/Ciunkos/pen/PogoRGV

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search