fix(runtime): don't register listener before connected to DOM (#5844)

* fix(runtime): don't register listener before connected to DOM

* prettier

* register host listeners in connectedCallback

* consolify

* don't register twice

* prettier

* remove unnecessary import

* more explicit tests

Author: christian-bromann

src/client/client-host-ref.ts

@@ -1,5 +1,4 @@
 import { BUILD } from '@app-data';
-import { addHostEventListeners } from '@runtime';
 
 import type * as d from '../declarations';
 
@@ -68,7 +67,6 @@ export const registerHost = (hostElement: d.HostElement, cmpMeta: d.ComponentRun
     hostElement['s-p'] = [];
     hostElement['s-rc'] = [];
   }
-  addHostEventListeners(hostElement, hostRef, cmpMeta.$listeners$, false);
   return hostRefs.set(hostElement, hostRef);
 };
 

src/hydrate/platform/hydrate-app.ts

@@ -1,5 +1,5 @@
 import { globalScripts } from '@app-globals';
-import { doc, getHostRef, loadModule, plt, registerHost } from '@platform';
+import { addHostEventListeners, doc, getHostRef, loadModule, plt, registerHost } from '@platform';
 import { connectedCallback, insertVdomAnnotations } from '@runtime';
 
 import type * as d from '../../declarations';
@@ -183,6 +183,9 @@ async function hydrateComponent(
 
     if (cmpMeta != null) {
       waitingElements.add(elm);
+      const hostRef = getHostRef(this);
+      addHostEventListeners(this, hostRef, cmpMeta.$listeners$, false);
+
       try {
         connectedCallback(elm);
         await elm.componentOnReady();

src/hydrate/platform/index.ts

@@ -1,5 +1,4 @@
 import { BUILD } from '@app-data';
-import { addHostEventListeners } from '@runtime';
 
 import type * as d from '../../declarations';
 
@@ -146,7 +145,6 @@ export const registerHost = (elm: d.HostElement, cmpMeta: d.ComponentRuntimeMeta
   hostRef.$onReadyPromise$ = new Promise((r) => (hostRef.$onReadyResolve$ = r));
   elm['s-p'] = [];
   elm['s-rc'] = [];
-  addHostEventListeners(elm, hostRef, cmpMeta.$listeners$, false);
   return hostRefs.set(elm, hostRef);
 };
 

src/runtime/bootstrap-custom-element.ts

@@ -1,5 +1,5 @@
 import { BUILD } from '@app-data';
-import { forceUpdate, getHostRef, registerHost, styles, supportsShadow } from '@platform';
+import { addHostEventListeners, forceUpdate, getHostRef, registerHost, styles, supportsShadow } from '@platform';
 import { CMP_FLAGS } from '@utils';
 
 import type * as d from '../declarations';
@@ -72,6 +72,9 @@ export const proxyCustomElement = (Cstr: any, compactMeta: d.ComponentRuntimeMet
       registerHost(this, cmpMeta);
     },
     connectedCallback() {
+      const hostRef = getHostRef(this);
+      addHostEventListeners(this, hostRef, cmpMeta.$listeners$, false);
+
       connectedCallback(this);
       if (BUILD.connectedCallback && originalConnectedCallback) {
         originalConnectedCallback.call(this);

src/runtime/bootstrap-lazy.ts

@@ -1,5 +1,6 @@
 import { BUILD } from '@app-data';
 import { doc, getHostRef, plt, registerHost, supportsShadow, win } from '@platform';
+import { addHostEventListeners } from '@runtime';
 import { CMP_FLAGS, queryNonceMetaTagContent } from '@utils';
 
 import type * as d from '../declarations';
@@ -96,6 +97,7 @@ export const bootstrapLazy = (lazyBundles: d.LazyBundlesRuntimeData, options: d.
       const HostElement = class extends HTMLElement {
         ['s-p']: Promise<void>[];
         ['s-rc']: (() => void)[];
+        hasRegisteredEventListeners = false;
 
         // StencilLazyHost
         constructor(self: HTMLElement) {
@@ -138,6 +140,19 @@ export const bootstrapLazy = (lazyBundles: d.LazyBundlesRuntimeData, options: d.
         }
 
         connectedCallback() {
+          const hostRef = getHostRef(this);
+
+          /**
+           * The `connectedCallback` lifecycle event can potentially be fired multiple times
+           * if the element is removed from the DOM and re-inserted. This is not a common use case,
+           * but it can happen in some scenarios. To prevent registering the same event listeners
+           * multiple times, we will only register them once.
+           */
+          if (!this.hasRegisteredEventListeners) {
+            this.hasRegisteredEventListeners = true;
+            addHostEventListeners(this, hostRef, cmpMeta.$listeners$, false);
+          }
+
           if (appLoadFallback) {
             clearTimeout(appLoadFallback);
             appLoadFallback = null;

src/runtime/test/listen.spec.tsx

@@ -201,4 +201,36 @@ describe('listen', () => {
     await waitForChanges();
     expect(a).toEqualHtml(`<cmp-a>1 7</cmp-a>`);
   });
+
+  it('disconnects target listeners when element is not connected to DOM', async () => {
+    let events = 0;
+    @Component({ tag: 'cmp-a' })
+    class CmpA {
+      @Listen('testEvent', { target: 'document' })
+      buttonClick() {
+        events++;
+      }
+
+      render() {
+        return '';
+      }
+    }
+
+    const { doc, waitForChanges } = await newSpecPage({
+      components: [CmpA],
+    });
+
+    jest.spyOn(doc, 'addEventListener');
+    jest.spyOn(doc, 'removeEventListener');
+
+    doc.createElement('cmp-a');
+    await waitForChanges();
+
+    // Event listener will never be called
+    expect(events).toEqual(0);
+
+    // no event listeners have been added as the element is not connected to the DOM
+    expect(doc.addEventListener.mock.calls.length).toBe(0);
+    expect(doc.removeEventListener.mock.calls.length).toBe(0);
+  });
 });

src/testing/platform/testing-host-ref.ts

@@ -1,4 +1,3 @@
-import { addHostEventListeners } from '@runtime';
 import type * as d from '@stencil/core/internal';
 
 import { hostRefs } from './testing-constants';
@@ -57,6 +56,5 @@ export const registerHost = (elm: d.HostElement, cmpMeta: d.ComponentRuntimeMeta
   hostRef.$onReadyPromise$ = new Promise((r) => (hostRef.$onReadyResolve$ = r));
   elm['s-p'] = [];
   elm['s-rc'] = [];
-  addHostEventListeners(elm, hostRef, cmpMeta.$listeners$, false);
   hostRefs.set(elm, hostRef);
 };