From 4add75c51610d6b5bcba53f265ff8c255fbbd7e4 Mon Sep 17 00:00:00 2001 From: Daniel Leroux Date: Thu, 13 Feb 2025 23:28:52 +0100 Subject: [PATCH] refactor(runtime): make WeakMap obsolete (#6156) * poc: use hostrefs inside nodes * Add cleanup logic of vnode * fix typings * remove vdome cleanup to void stable refs * remove dev log * fix return types of delete * use get function to resolve hostref * remove not needed function * change type of childNode * remove WeapMap implementation from hydration and testing --- src/client/client-host-ref.ts | 43 +++++++---------------- src/declarations/stencil-private.ts | 4 ++- src/hydrate/platform/index.ts | 19 ++++++---- src/runtime/bootstrap-custom-element.ts | 25 +------------ src/runtime/vdom/vdom-annotations.ts | 2 +- src/testing/platform/index.ts | 2 +- src/testing/platform/testing-constants.ts | 5 --- src/testing/platform/testing-host-ref.ts | 27 +++++--------- src/testing/platform/testing-platform.ts | 3 +- 9 files changed, 40 insertions(+), 90 deletions(-) diff --git a/src/client/client-host-ref.ts b/src/client/client-host-ref.ts index a055194b2c6..00d0c3f633a 100644 --- a/src/client/client-host-ref.ts +++ b/src/client/client-host-ref.ts @@ -3,41 +3,19 @@ import { reWireGetterSetter } from '@utils/es2022-rewire-class-members'; import type * as d from '../declarations'; -/** - * A WeakMap mapping runtime component references to their corresponding host reference - * instances. - * - * **Note**: If we're in an HMR context we need to store a reference to this - * value on `window` in order to maintain the mapping of {@link d.RuntimeRef} - * to {@link d.HostRef} across HMR updates. - * - * This is necessary because when HMR updates for a component are processed by - * the browser-side dev server client the JS bundle for that component is - * re-fetched. Since the module containing {@link hostRefs} is included in - * that bundle, if we do not store a reference to it the new iteration of the - * component will not have access to the previous hostRef map, leading to a - * bug where the new version of the component cannot properly initialize. - */ -const hostRefs: WeakMap = /*@__PURE__*/ BUILD.hotModuleReplacement - ? ((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} * * @param ref the runtime ref of interest * @returns the Host reference (if found) or undefined */ -export const getHostRef = (ref: d.RuntimeRef): d.HostRef | undefined => hostRefs.get(ref); +export const getHostRef = (ref: d.RuntimeRef): d.HostRef | undefined => { + if (ref.__stencil__getHostRef) { + return ref.__stencil__getHostRef(); + } + + return undefined; +}; /** * Register a lazy instance with the {@link hostRefs} object so it's @@ -47,7 +25,9 @@ export const getHostRef = (ref: d.RuntimeRef): d.HostRef | undefined => hostRefs * @param hostRef that instances `HostRef` object */ export const registerInstance = (lazyInstance: any, hostRef: d.HostRef) => { - hostRefs.set((hostRef.$lazyInstance$ = lazyInstance), hostRef); + lazyInstance.__stencil__getHostRef = () => hostRef; + hostRef.$lazyInstance$ = lazyInstance; + if (BUILD.modernPropertyDecls && (BUILD.state || BUILD.prop)) { reWireGetterSetter(lazyInstance, hostRef); } @@ -81,7 +61,8 @@ export const registerHost = (hostElement: d.HostElement, cmpMeta: d.ComponentRun hostElement['s-rc'] = []; } - const ref = hostRefs.set(hostElement, hostRef); + const ref = hostRef; + hostElement.__stencil__getHostRef = () => ref; if (!BUILD.lazyLoad && BUILD.modernPropertyDecls && (BUILD.state || BUILD.prop)) { reWireGetterSetter(hostElement, hostRef); diff --git a/src/declarations/stencil-private.ts b/src/declarations/stencil-private.ts index d1a785ac1d5..0dfb7b9995d 100644 --- a/src/declarations/stencil-private.ts +++ b/src/declarations/stencil-private.ts @@ -1088,6 +1088,8 @@ export interface HostElement extends HTMLElement { host?: Element; forceUpdate?: () => void; + __stencil__getHostRef?: () => HostRef; + // "s-" prefixed properties should not be property renamed // and should be common between all versions of stencil @@ -1724,7 +1726,7 @@ export type ComponentRuntimeReflectingAttr = [string, string | undefined]; * keys in a `WeakMap` which maps {@link HostElement} instances to their * associated {@link HostRef} instance. */ -export type RuntimeRef = HostElement | {}; +export type RuntimeRef = HostElement | { __stencil__getHostRef?: () => HostRef }; /** * Interface used to track an Element, it's virtual Node (`VNode`), and other data diff --git a/src/hydrate/platform/index.ts b/src/hydrate/platform/index.ts index 5f55ba7df08..fb78f977016 100644 --- a/src/hydrate/platform/index.ts +++ b/src/hydrate/platform/index.ts @@ -127,17 +127,22 @@ export const supportsListenerOptions = false; export const supportsConstructableStylesheets = false; -const hostRefs: WeakMap = new WeakMap(); +export const getHostRef = (ref: d.RuntimeRef) => { + if (ref.__stencil__getHostRef) { + return ref.__stencil__getHostRef(); + } -export const getHostRef = (ref: d.RuntimeRef) => hostRefs.get(ref); -export const deleteHostRef = (ref: d.RuntimeRef) => hostRefs.delete(ref); + return undefined; +}; export const registerInstance = (lazyInstance: any, hostRef: d.HostRef) => { - const ref = hostRefs.set((hostRef.$lazyInstance$ = lazyInstance), hostRef); + lazyInstance.__stencil__getHostRef = () => hostRef; + hostRef.$lazyInstance$ = lazyInstance; + if (BUILD.modernPropertyDecls && (BUILD.state || BUILD.prop)) { reWireGetterSetter(lazyInstance, hostRef); } - return ref; + return hostRef; }; export const registerHost = (elm: d.HostElement, cmpMeta: d.ComponentRuntimeMeta) => { @@ -152,7 +157,9 @@ export const registerHost = (elm: d.HostElement, cmpMeta: d.ComponentRuntimeMeta hostRef.$onReadyPromise$ = new Promise((r) => (hostRef.$onReadyResolve$ = r)); elm['s-p'] = []; elm['s-rc'] = []; - return hostRefs.set(elm, hostRef); + elm.__stencil__getHostRef = () => hostRef; + + return hostRef; }; export const Build: d.UserBuildConditionals = { diff --git a/src/runtime/bootstrap-custom-element.ts b/src/runtime/bootstrap-custom-element.ts index 8bd005657b5..6ddfe135d92 100644 --- a/src/runtime/bootstrap-custom-element.ts +++ b/src/runtime/bootstrap-custom-element.ts @@ -1,14 +1,5 @@ import { BUILD } from '@app-data'; -import { - addHostEventListeners, - deleteHostRef, - forceUpdate, - getHostRef, - plt, - 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'; @@ -102,20 +93,6 @@ 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/vdom/vdom-annotations.ts b/src/runtime/vdom/vdom-annotations.ts index b432f7c3737..3c8155bb8f1 100644 --- a/src/runtime/vdom/vdom-annotations.ts +++ b/src/runtime/vdom/vdom-annotations.ts @@ -128,7 +128,7 @@ const parseVNodeAnnotations = ( */ const childNodes = [...Array.from(node.childNodes), ...Array.from(node.shadowRoot?.childNodes || [])]; childNodes.forEach((childNode) => { - const hostRef = getHostRef(childNode); + const hostRef = getHostRef(childNode as d.RuntimeRef); if (hostRef != null && !docData.staticComponents.has(childNode.nodeName.toLowerCase())) { const cmpData: CmpData = { nodeIds: 0, diff --git a/src/testing/platform/index.ts b/src/testing/platform/index.ts index 4233ba07c99..5d8e4074c41 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 { deleteHostRef, getHostRef, registerHost, registerInstance } from './testing-host-ref'; +export { 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-constants.ts b/src/testing/platform/testing-constants.ts index 85e979fe884..7f974ca86d9 100644 --- a/src/testing/platform/testing-constants.ts +++ b/src/testing/platform/testing-constants.ts @@ -21,8 +21,3 @@ export const queuedLoadModules: QueuedLoadModule[] = []; * A collection of errors that were detected to surface during the rendering process */ export const caughtErrors: Error[] = []; -/** - * A mapping of runtime references to HTML elements to the data structure Stencil uses to track the element alongside - * additional metadata - */ -export const hostRefs = new Map(); diff --git a/src/testing/platform/testing-host-ref.ts b/src/testing/platform/testing-host-ref.ts index c7179b1e727..03729dd471a 100644 --- a/src/testing/platform/testing-host-ref.ts +++ b/src/testing/platform/testing-host-ref.ts @@ -1,37 +1,26 @@ import type * as d from '@stencil/core/internal'; -import { hostRefs } from './testing-constants'; - /** * Retrieve the data structure tracking the component by its runtime reference * @param elm the reference to the element * @returns the corresponding Stencil reference data structure, or undefined if one cannot be found */ export const getHostRef = (elm: d.RuntimeRef | undefined): d.HostRef | undefined => { - return hostRefs.get(elm); -}; + if (elm.__stencil__getHostRef) { + return elm.__stencil__getHostRef(); + } -/** - * 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); + return undefined; +}; /** * Add the provided `hostRef` instance to the global {@link hostRefs} map, using the provided `lazyInstance` as a key. * @param lazyInstance a Stencil component instance * @param hostRef an optional reference to Stencil's tracking data for the component. If none is provided, one will be created. - * @returns the updated `hostRefs` data structure * @throws if the provided `lazyInstance` coerces to `null`, or if the `lazyInstance` does not have a `constructor` * property */ -export const registerInstance = ( - lazyInstance: any, - hostRef: d.HostRef | null | undefined, -): Map => { +export const registerInstance = (lazyInstance: any, hostRef: d.HostRef | null | undefined) => { if (lazyInstance == null || lazyInstance.constructor == null) { throw new Error(`Invalid component constructor`); } @@ -44,8 +33,8 @@ export const registerInstance = ( hostRef = getHostRef(elm); } + lazyInstance.__stencil__getHostRef = () => hostRef; hostRef.$lazyInstance$ = lazyInstance; - return hostRefs.set(lazyInstance, hostRef); }; /** @@ -65,5 +54,5 @@ export const registerHost = (elm: d.HostElement, cmpMeta: d.ComponentRuntimeMeta hostRef.$onReadyPromise$ = new Promise((r) => (hostRef.$onReadyResolve$ = r)); elm['s-p'] = []; elm['s-rc'] = []; - hostRefs.set(elm, hostRef); + elm.__stencil__getHostRef = () => hostRef; }; diff --git a/src/testing/platform/testing-platform.ts b/src/testing/platform/testing-platform.ts index 772f9209128..5f2d81f3ead 100644 --- a/src/testing/platform/testing-platform.ts +++ b/src/testing/platform/testing-platform.ts @@ -1,6 +1,6 @@ import type * as d from '@stencil/core/internal'; -import { cstrs, hostRefs, moduleLoaded, styles } from './testing-constants'; +import { cstrs, moduleLoaded, styles } from './testing-constants'; import { flushAll, resetTaskQueue } from './testing-task-queue'; import { win } from './testing-window'; @@ -54,7 +54,6 @@ export function resetPlatform(defaults: Partial = {}) { win.close(); } - hostRefs.clear(); styles.clear(); plt.$flags$ = 0; Object.assign(plt, defaults);