Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(runtime): clear up rootAppliedStyles #6087

Merged
merged 31 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7f45919
fix(runtime): clear up detached hostRefs and rootAppliedStyles
christian-bromann Dec 31, 2024
a0c07cb
fix build
christian-bromann Dec 31, 2024
22a6ade
prettier
christian-bromann Dec 31, 2024
1d25bf8
fix after rebase
christian-bromann Jan 7, 2025
f7447bc
more reverts
christian-bromann Jan 7, 2025
b78a4ec
more cleanups
christian-bromann Jan 7, 2025
709e57d
remove hostrefs manually instead of cleanups
christian-bromann Jan 7, 2025
5b58ddb
prettier
christian-bromann Jan 7, 2025
155010e
fix build
christian-bromann Jan 7, 2025
15953cf
found the leak
christian-bromann Jan 7, 2025
12e4c8f
found it without errors
christian-bromann Jan 7, 2025
a166959
delete the right element
christian-bromann Jan 7, 2025
04fbc06
clean up after animation frame
christian-bromann Jan 7, 2025
e597320
prettier
christian-bromann Jan 7, 2025
e1053d2
use isConnected to check whether node is in DOM
christian-bromann Jan 7, 2025
224b3ea
prettier
christian-bromann Jan 7, 2025
900ca11
solve leak for dist custom elements
christian-bromann Jan 7, 2025
80a5d1f
prettier
christian-bromann Jan 7, 2025
a892a08
fix: PR comment
christian-bromann Jan 9, 2025
5112b33
more rigurous cleanup
christian-bromann Jan 9, 2025
627d55f
delete the lazy instance after a timeout to ensure that any pending s…
christian-bromann Jan 9, 2025
16456f5
prettier
christian-bromann Jan 9, 2025
1f0c6c4
fix unit test
christian-bromann Jan 9, 2025
7aafaeb
chore(ci): trigger build
christian-bromann Jan 9, 2025
29e4834
disable test
christian-bromann Jan 9, 2025
a559551
logging
christian-bromann Jan 9, 2025
85bb08e
tweak
christian-bromann Jan 9, 2025
2614355
tweak
christian-bromann Jan 10, 2025
2f56e1e
clean up
christian-bromann Jan 10, 2025
8a99e01
minor comment update
christian-bromann Jan 10, 2025
b3a724b
more cleanups
christian-bromann Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/runtime/bootstrap-lazy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import { hmrStart } from './hmr-component';
import { createTime, installDevTools } from './profile';
import { proxyComponent } from './proxy-component';
import { HYDRATED_CSS, PLATFORM_FLAGS, PROXY_FLAGS, SLOT_FB_CSS } from './runtime-constants';
import { HYDRATED_CSS, NODE_TYPE, PLATFORM_FLAGS, PROXY_FLAGS, SLOT_FB_CSS } from './runtime-constants';
import { appDidLoad } from './update-component';
export { setNonce } from '@platform';

Expand Down Expand Up @@ -159,6 +159,23 @@ 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$ &&
hostRef.$vnode$.$elm$.nodeType === NODE_TYPE.ElementNode &&
!isNodeAttached(hostRef.$vnode$.$elm$)
) {
delete hostRef.$vnode$.$elm$;
}
});
}

componentOnReady() {
Expand Down Expand Up @@ -259,3 +276,12 @@ export const bootstrapLazy = (lazyBundles: d.LazyBundlesRuntimeData, options: d.
// Fallback appLoad event
endBootstrap();
};

/**
* Check if a node is attached to the DOM
* @param node an element or document
* @returns true if the node is attached to the DOM
*/
function isNodeAttached(node: Node): boolean {
return node === document || node === document.documentElement || document.contains(node);
christian-bromann marked this conversation as resolved.
Show resolved Hide resolved
}
15 changes: 15 additions & 0 deletions src/runtime/disconnected-callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
}
};
2 changes: 1 addition & 1 deletion src/runtime/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/runtime/vdom/update-element.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand Down
6 changes: 0 additions & 6 deletions src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<never, never> = {};

/**
* Namespaces
*/
Expand Down
Loading