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
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:
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 theconnectedCallback
, there should be a correspondingremoveEventListener
call in thedisconnectedCallback
.Thus the listener (e.g.
this.listener
) and the host reference (e.g.this.host
) from theconnectedCallback
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