From c4a9c9e7bd168d6e0e6b6d83ad338c0179c1e731 Mon Sep 17 00:00:00 2001 From: Christian Bromann Date: Fri, 10 Jan 2025 14:24:03 -0800 Subject: [PATCH] fix(runtime): clear up rootAppliedStyles (#6087) * fix(runtime): clear up detached hostRefs and rootAppliedStyles * fix build * prettier * fix after rebase * more reverts * more cleanups * remove hostrefs manually instead of cleanups * prettier * fix build * found the leak * found it without errors * delete the right element * clean up after animation frame * prettier * use isConnected to check whether node is in DOM * prettier * solve leak for dist custom elements * prettier * fix: PR comment * more rigurous cleanup * delete the lazy instance after a timeout to ensure that any pending state updates have been processed * prettier * fix unit test * chore(ci): trigger build * disable test * logging * tweak * tweak * clean up * minor comment update * more cleanups --- src/client/client-host-ref.ts | 9 +++++++++ src/hydrate/platform/index.ts | 1 + src/runtime/bootstrap-custom-element.ts | 25 +++++++++++++++++++++++- src/runtime/bootstrap-lazy.ts | 13 ++++++++++++ src/runtime/disconnected-callback.ts | 15 ++++++++++++++ src/runtime/styles.ts | 2 +- src/runtime/vdom/update-element.ts | 5 ++--- src/testing/platform/index.ts | 2 +- src/testing/platform/testing-host-ref.ts | 9 +++++++++ src/utils/constants.ts | 6 ------ 10 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/client/client-host-ref.ts b/src/client/client-host-ref.ts index e96c26a02ca..bb098cc7987 100644 --- a/src/client/client-host-ref.ts +++ b/src/client/client-host-ref.ts @@ -22,6 +22,15 @@ const hostRefs: WeakMap = /*@__PURE__*/ BUILD.hotModule ? ((window as any).__STENCIL_HOSTREFS__ ||= new WeakMap()) : new WeakMap(); +/** + * Given a {@link d.RuntimeRef} remove the corresponding {@link d.HostRef} from + * the {@link hostRefs} WeakMap. + * + * @param ref the runtime ref of interest + * @returns — true if the element was successfully removed, or false if it was not present. + */ +export const deleteHostRef = (ref: d.RuntimeRef) => hostRefs.delete(ref); + /** * Given a {@link d.RuntimeRef} retrieve the corresponding {@link d.HostRef} * diff --git a/src/hydrate/platform/index.ts b/src/hydrate/platform/index.ts index e4467db21d0..fcf8122342c 100644 --- a/src/hydrate/platform/index.ts +++ b/src/hydrate/platform/index.ts @@ -129,6 +129,7 @@ export const supportsConstructableStylesheets = false; const hostRefs: WeakMap = new WeakMap(); export const getHostRef = (ref: d.RuntimeRef) => hostRefs.get(ref); +export const deleteHostRef = (ref: d.RuntimeRef) => hostRefs.delete(ref); export const registerInstance = (lazyInstance: any, hostRef: d.HostRef) => hostRefs.set((hostRef.$lazyInstance$ = lazyInstance), hostRef); diff --git a/src/runtime/bootstrap-custom-element.ts b/src/runtime/bootstrap-custom-element.ts index aeff2f4818e..736f7de6eaf 100644 --- a/src/runtime/bootstrap-custom-element.ts +++ b/src/runtime/bootstrap-custom-element.ts @@ -1,5 +1,14 @@ import { BUILD } from '@app-data'; -import { addHostEventListeners, forceUpdate, getHostRef, registerHost, styles, supportsShadow } from '@platform'; +import { + addHostEventListeners, + deleteHostRef, + forceUpdate, + getHostRef, + plt, + registerHost, + styles, + supportsShadow, +} from '@platform'; import { CMP_FLAGS } from '@utils'; import type * as d from '../declarations'; @@ -89,6 +98,20 @@ export const proxyCustomElement = (Cstr: any, compactMeta: d.ComponentRuntimeMet if (BUILD.disconnectedCallback && originalDisconnectedCallback) { originalDisconnectedCallback.call(this); } + + /** + * Clean up Node references lingering around in `hostRef` objects + * to ensure GC can clean up the memory. + */ + plt.raf(() => { + const hostRef = getHostRef(this); + if (hostRef?.$vnode$?.$elm$ instanceof Node && !hostRef.$vnode$.$elm$.isConnected) { + delete hostRef.$vnode$; + } + if (this instanceof Node && !this.isConnected) { + deleteHostRef(this); + } + }); }, __attachShadow() { if (supportsShadow) { diff --git a/src/runtime/bootstrap-lazy.ts b/src/runtime/bootstrap-lazy.ts index 7bc14684981..2706d8519ed 100644 --- a/src/runtime/bootstrap-lazy.ts +++ b/src/runtime/bootstrap-lazy.ts @@ -159,6 +159,19 @@ export const bootstrapLazy = (lazyBundles: d.LazyBundlesRuntimeData, options: d. disconnectedCallback() { plt.jmp(() => disconnectedCallback(this)); + + /** + * Clear up references within the `$vnode$` object to the DOM + * node that was removed. This is necessary to ensure that these + * references used as keys in the `hostRef` object can be properly + * garbage collected. + */ + plt.raf(() => { + const hostRef = getHostRef(this); + if (hostRef?.$vnode$?.$elm$ instanceof Node && !hostRef.$vnode$.$elm$.isConnected) { + delete hostRef.$vnode$.$elm$; + } + }); } componentOnReady() { diff --git a/src/runtime/disconnected-callback.ts b/src/runtime/disconnected-callback.ts index 137142cddee..634da2a0d4d 100644 --- a/src/runtime/disconnected-callback.ts +++ b/src/runtime/disconnected-callback.ts @@ -3,6 +3,7 @@ import { getHostRef, plt } from '@platform'; import type * as d from '../declarations'; import { PLATFORM_FLAGS } from './runtime-constants'; +import { rootAppliedStyles } from './styles'; import { safeCall } from './update-component'; const disconnectInstance = (instance: any) => { @@ -33,4 +34,18 @@ export const disconnectedCallback = async (elm: d.HostElement) => { hostRef.$onReadyPromise$.then(() => disconnectInstance(hostRef.$lazyInstance$)); } } + + /** + * Remove the element from the `rootAppliedStyles` WeakMap + */ + if (rootAppliedStyles.has(elm)) { + rootAppliedStyles.delete(elm); + } + + /** + * Remove the shadow root from the `rootAppliedStyles` WeakMap + */ + if (elm.shadowRoot && rootAppliedStyles.has(elm.shadowRoot as unknown as Element)) { + rootAppliedStyles.delete(elm.shadowRoot as unknown as Element); + } }; diff --git a/src/runtime/styles.ts b/src/runtime/styles.ts index 3b497c9d00f..579f427b0e1 100644 --- a/src/runtime/styles.ts +++ b/src/runtime/styles.ts @@ -6,7 +6,7 @@ import type * as d from '../declarations'; import { createTime } from './profile'; import { HYDRATED_STYLE_ID, NODE_TYPE, SLOT_FB_CSS } from './runtime-constants'; -const rootAppliedStyles: d.RootAppliedStyleMap = /*@__PURE__*/ new WeakMap(); +export const rootAppliedStyles: d.RootAppliedStyleMap = /*@__PURE__*/ new WeakMap(); /** * Register the styles for a component by creating a stylesheet and then diff --git a/src/runtime/vdom/update-element.ts b/src/runtime/vdom/update-element.ts index 7487a62c02f..5cea42cfe8d 100644 --- a/src/runtime/vdom/update-element.ts +++ b/src/runtime/vdom/update-element.ts @@ -1,5 +1,4 @@ import { BUILD } from '@app-data'; -import { EMPTY_OBJ } from '@utils'; import type * as d from '../../declarations'; import { NODE_TYPE } from '../runtime-constants'; @@ -24,8 +23,8 @@ export const updateElement = (oldVnode: d.VNode | null, newVnode: d.VNode, isSvg newVnode.$elm$.nodeType === NODE_TYPE.DocumentFragment && newVnode.$elm$.host ? newVnode.$elm$.host : (newVnode.$elm$ as any); - const oldVnodeAttrs = (oldVnode && oldVnode.$attrs$) || EMPTY_OBJ; - const newVnodeAttrs = newVnode.$attrs$ || EMPTY_OBJ; + const oldVnodeAttrs = (oldVnode && oldVnode.$attrs$) || {}; + const newVnodeAttrs = newVnode.$attrs$ || {}; if (BUILD.updatable) { // remove attributes no longer present on the vnode by setting them to undefined diff --git a/src/testing/platform/index.ts b/src/testing/platform/index.ts index 5d8e4074c41..4233ba07c99 100644 --- a/src/testing/platform/index.ts +++ b/src/testing/platform/index.ts @@ -1,6 +1,6 @@ export { Build } from './testing-build'; export { modeResolutionChain, styles } from './testing-constants'; -export { getHostRef, registerHost, registerInstance } from './testing-host-ref'; +export { deleteHostRef, getHostRef, registerHost, registerInstance } from './testing-host-ref'; export { consoleDevError, consoleDevInfo, consoleDevWarn, consoleError, setErrorHandler } from './testing-log'; export { isMemberInElement, diff --git a/src/testing/platform/testing-host-ref.ts b/src/testing/platform/testing-host-ref.ts index 6adcc8d9a3c..c7179b1e727 100644 --- a/src/testing/platform/testing-host-ref.ts +++ b/src/testing/platform/testing-host-ref.ts @@ -11,6 +11,15 @@ export const getHostRef = (elm: d.RuntimeRef | undefined): d.HostRef | undefined return hostRefs.get(elm); }; +/** + * Given a {@link d.RuntimeRef} remove the corresponding {@link d.HostRef} from + * the {@link hostRefs} WeakMap. + * + * @param ref the runtime ref of interest + * @returns — true if the element was successfully removed, or false if it was not present. + */ +export const deleteHostRef = (ref: d.RuntimeRef) => hostRefs.delete(ref); + /** * Add the provided `hostRef` instance to the global {@link hostRefs} map, using the provided `lazyInstance` as a key. * @param lazyInstance a Stencil component instance diff --git a/src/utils/constants.ts b/src/utils/constants.ts index d8b9c7df291..b2bf0febaf9 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -120,12 +120,6 @@ export const enum CMP_FLAGS { */ export const DEFAULT_STYLE_MODE = '$'; -/** - * Reusable empty obj/array - * Don't add values to these!! - */ -export const EMPTY_OBJ: Record = {}; - /** * Namespaces */