Skip to content

Commit

Permalink
fix(runtime): don't register listener before connected to DOM (#5844)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
christian-bromann authored Jun 21, 2024
1 parent a2fa93b commit 9d7021f
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 8 deletions.
2 changes: 0 additions & 2 deletions src/client/client-host-ref.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { BUILD } from '@app-data';
import { addHostEventListeners } from '@runtime';

import type * as d from '../declarations';

Expand Down Expand Up @@ -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);
};

Expand Down
5 changes: 4 additions & 1 deletion src/hydrate/platform/hydrate-app.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 0 additions & 2 deletions src/hydrate/platform/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { BUILD } from '@app-data';
import { addHostEventListeners } from '@runtime';

import type * as d from '../../declarations';

Expand Down Expand Up @@ -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);
};

Expand Down
5 changes: 4 additions & 1 deletion src/runtime/bootstrap-custom-element.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions src/runtime/bootstrap-lazy.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
32 changes: 32 additions & 0 deletions src/runtime/test/listen.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
2 changes: 0 additions & 2 deletions src/testing/platform/testing-host-ref.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { addHostEventListeners } from '@runtime';
import type * as d from '@stencil/core/internal';

import { hostRefs } from './testing-constants';
Expand Down Expand Up @@ -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);
};

0 comments on commit 9d7021f

Please sign in to comment.