Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

3.0.1

Current Behavior

When a component is defined with a Host Listener that species a target that isn't the element itself (e.g. Listen({ target: 'window'}), if an instance of that component is created but not attached to the DOM then the event listener will not be removed, meaning that the component and the underlying Stencil runtime objects cannot be garbage collected. Overtime a memory/resource leak like this will cause the application to run out of resources.

Expected Behavior

The event listener should not be leaked (either it should be removed, or it should never have been created), allowing the GC to release the DOM element and the associated Stencil runtime objects, and therefore not leaking memory.

System Info

No response

Steps to Reproduce

Create a simple component with a host listener targetting the window object

import { Component, h, Listen } from '@stencil/core';

@Component({
  tag: 'cmp-a'
})
export class CmpA {

  @Listen('testEvent', { target: 'window' }) testListener(ev: Event) {
    // Does nothing
  }

  render() {
    return <Host/>;
  }
}

Elsewhere in your code, create an instance of this element, but don't attach it to the DOM

const elm = document.createElement('cmp-a')

If you now inspect the heap (DevTools > Memory > Heap Snapshot) you will see a "Detached HTMLElement" corresponding to the component. If you are able to go into Stencils internals, then the hostRefs WeakMap will also contain entries for this component that will never be released.

Reproduction Repository

The reproduction repository provided illustrates the above. The root component creates a child component that has a host listener targetting root once every second. If you run the reproduction app and monitor the "Performance Monitor" tab in Chrome Dev Tools you'll see "DOM Nodes" and "JS event listeners" increase indefinitely. Forcing a manual GC will not bring the numbers down. Examining the heap you'll see an increasing number of Deetached HTMLElements. Examining one of those elements, under "Retainers" you'll see e.g. elm in system / Context@313813 which is the context for the event listener.

Code Reproduction URL

https://github.com/joewoodhouse/stencil-hostlistener-leak

Additional Information

Background

This may seem like a niche issue, however I came across this whilst investigating memory/resource leaks in Ionic to do with overlays. This is all tied to a suite of issues in the Stencil runtime and vdom that cause our application to regularly crash for our users after a moderate amount of use. I've discussed this with @alicewriteswrongs and others elsewhere in #3607

Root Cause and Potential Solutions

HostListeners are attached to the element in the constructor of the CustomElement (addHostEventListeners is called by registerHost which is called by the constructor). However, they are detached in the disconnectedCallback of the element, which is not guaranteed to be called.

In the above case, disconnectedCallback is not called because the element is never connected. However there are other scenarios where both the constructor and connectedCallback can be called but disconnectedCallback is not called (these are other bugs in Stencil that I will raise shortly).

The presence of target is important to this scenario, because adding event listeners to the element itself (i.e. the default when you don't provide a target) does not create the issue, because the GC will automatically disregard these when destroying the element. It's when the event listener is attached to something else that causes the problem.

As CustomElement (and Javascript) have no destroy sort of concept (the inverse of the constructor) there is no obvious other place to move the disconnection of the host listeners to.

The most obvious fix would be to move the attachment of the host listeners into the connectedCallback of the element. However this would obviously introduce a slight change in behaviour - host listeners would only then be "active" when an element is connected to the DOM. This may in fact be the correct/desired/intended behaviour, but I'm not sure.

One other thing to note is that the @Listen function itself is never called, as the Stencil runtime proxies the events. So in the runtime you'll see HostRefs $queuedListeners$ array will be storing all the incoming events indefinitely (another resource/memory leak).

The safest but obvious very breaking change to make would be to remove the target option all together, and force the user to do that manually in their user-space connected and disconnected callbacks (or elsewhere).