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

Avoid recursively setting KEY__SHADOW_RESOLVER #4089

Open
nolanlawson opened this issue Mar 20, 2024 · 0 comments
Open

Avoid recursively setting KEY__SHADOW_RESOLVER #4089

nolanlawson opened this issue Mar 20, 2024 · 0 comments

Comments

@nolanlawson
Copy link
Contributor

nolanlawson commented Mar 20, 2024

As described in #4088, we have a function called recursivelySetShadowResolver:

function recursivelySetShadowResolver(node: Node, fn: any) {
(node as any)[KEY__SHADOW_RESOLVER] = fn;
const childNodes = childNodesGetter.call(node);
for (let i = 0, n = childNodes.length; i < n; i++) {
recursivelySetShadowResolver(childNodes[i], fn);
}
}

This is called at the top level for all static fragments. For non-static fragments, we set it on all nodes in linkNodeToShadow:

function linkNodeToShadow(elm: Node, owner: VM, renderer: RendererAPI) {
const { renderRoot, renderMode, shadowMode } = owner;
const { isSyntheticShadowDefined } = renderer;
// TODO [#1164]: this should eventually be done by the polyfill directly
if (isSyntheticShadowDefined) {
if (shadowMode === ShadowMode.Synthetic || renderMode === RenderMode.Light) {
(elm as any)[KEY__SHADOW_RESOLVER] = renderRoot[KEY__SHADOW_RESOLVER];
}
}
}

This serves two purposes:

  1. isNodeShadowed/isNodeFromTemplate – legacy API used by Locker
  2. "portals" aka MutationObserver – the patched MutationObserver has to know which nodes come from synthetic shadow roots

This could be much more efficient, though, if we computed it on-demand. Rather than traversing through the whole DOM tree to set this value on every node (not just elements – nodes!), we could compute it on-demand based on the parent/host of the node.

This would 1) restrict the cost to only when isNodeShadowed/MutationObserver is invoked, and 2) be more efficient in general, since walking up the tree is faster than traversing down the tree.

OTOH we would probably have to do some caching to avoid recomputing this for expensive MutationObserver calls.

@nolanlawson nolanlawson changed the title Remove recursivelySetShadowResolver Avoid recursively setting KEY__SHADOW_RESOLVER Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant