From 9843b5a87e8fd94c04b893c18b8796310d0b3e64 Mon Sep 17 00:00:00 2001 From: Mohammed Abdul Sattar Date: Mon, 25 Mar 2024 12:43:16 -0400 Subject: [PATCH 1/4] fix: children and childNodes not returning top-level slotted children --- .../test/light-dom/root/index.spec.js | 2 ++ .../src/faux-shadow/element.ts | 2 +- .../synthetic-shadow/src/faux-shadow/node.ts | 2 +- .../src/faux-shadow/traverse.ts | 23 +++++++++++++------ .../src/shared/node-ownership.ts | 2 ++ 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/@lwc/integration-karma/test/light-dom/root/index.spec.js b/packages/@lwc/integration-karma/test/light-dom/root/index.spec.js index 7055e5f3a9..ad5e11ce10 100644 --- a/packages/@lwc/integration-karma/test/light-dom/root/index.spec.js +++ b/packages/@lwc/integration-karma/test/light-dom/root/index.spec.js @@ -78,5 +78,7 @@ describe('root light element', () => { expect(nodes['x-list'].querySelector('button')).toEqual(nodes.button); expect(nodes['x-list'].getElementsByTagName('button')[0]).toEqual(nodes.button); expect(nodes['x-list'].getElementsByClassName('button')[0]).toEqual(nodes.button); + expect(nodes['x-list'].childNodes[0]).toEqual(nodes.button); + expect(nodes['x-list'].children[0]).toEqual(nodes.button); }); }); diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts index 528d32f838..96a3e4eb45 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts @@ -92,7 +92,7 @@ function shadowRootGetterPatched(this: Element): ShadowRoot | null { function childrenGetterPatched(this: Element): HTMLCollectionOf { const owner = getNodeOwner(this); - const childNodes = isNull(owner) ? [] : getAllMatches(owner, getFilteredChildNodes(this)); + const childNodes = getAllMatches(owner, getFilteredChildNodes(this)); return createStaticHTMLCollection( ArrayFilter.call(childNodes, (node: Node | Element) => node instanceof Element) ); diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts index 493ee5b644..bc452b74cc 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts @@ -173,7 +173,7 @@ function cloneNodePatched(this: Node, deep?: boolean): Node { function childNodesGetterPatched(this: Node): NodeListOf { if (isSyntheticShadowHost(this)) { const owner = getNodeOwner(this); - const childNodes = isNull(owner) ? [] : getAllMatches(owner, getFilteredChildNodes(this)); + const childNodes = getAllMatches(owner, getFilteredChildNodes(this)); return createStaticNodeList(childNodes); } // nothing to do here since this does not have a synthetic shadow attached to it diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts index e9ac1f005f..555a36a3d9 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts @@ -134,16 +134,26 @@ export function isSlotElement(node: Node): node is HTMLSlotElement { return node instanceof HTMLSlotElement; } -export function isNodeOwnedBy(owner: Element, node: Node): boolean { +export function isNodeOwnedBy(owner: Element | null, node: Node): boolean { if (process.env.NODE_ENV !== 'production') { - if (!(owner instanceof HTMLElement)) { - // eslint-disable-next-line no-console - console.error(`isNodeOwnedBy() should be called with an element as the first argument`); - } if (!(node instanceof Node)) { // eslint-disable-next-line no-console console.error(`isNodeOwnedBy() should be called with a node as the second argument`); } + } + + // owner can be null if all the parents are in the light DOM + // ownerKey is undefined for such nodes + const ownerKey = getNodeNearestOwnerKey(node); + if (isNull(owner)) { + return isUndefined(ownerKey); + } + + if (process.env.NODE_ENV !== 'production') { + if (!(owner instanceof HTMLElement)) { + // eslint-disable-next-line no-console + console.error(`isNodeOwnedBy() should be called with an element as the first argument`); + } if (!(compareDocumentPosition.call(node, owner) & DOCUMENT_POSITION_CONTAINS)) { // eslint-disable-next-line no-console console.error( @@ -151,7 +161,6 @@ export function isNodeOwnedBy(owner: Element, node: Node): boolean { ); } } - const ownerKey = getNodeNearestOwnerKey(node); if (isUndefined(ownerKey)) { // in case of root level light DOM element slotting into a synthetic shadow @@ -196,7 +205,7 @@ export function getFirstSlottedMatch(host: Element, nodeList: Element[]): Elemen return null; } -export function getAllMatches(owner: Element, nodeList: Node[]): T[] { +export function getAllMatches(owner: Element | null, nodeList: Node[]): T[] { const filteredAndPatched: T[] = []; for (let i = 0, len = nodeList.length; i < len; i += 1) { const node = nodeList[i]; diff --git a/packages/@lwc/synthetic-shadow/src/shared/node-ownership.ts b/packages/@lwc/synthetic-shadow/src/shared/node-ownership.ts index 63ddd73b56..eac9d58502 100644 --- a/packages/@lwc/synthetic-shadow/src/shared/node-ownership.ts +++ b/packages/@lwc/synthetic-shadow/src/shared/node-ownership.ts @@ -58,6 +58,8 @@ export function getNodeNearestOwnerKey(node: Node): number | undefined { } host = parentNodeGetter.call(host) as ShadowedNode | null; + // Elements slotted from top level light DOM into synthetic shadow + // reach the slot tag from the shadow element first if (!isNull(host) && isSyntheticSlotElement(host)) { return undefined; } From 6166bbeb1f74df9be861669589d6a87335533934 Mon Sep 17 00:00:00 2001 From: Mohammed Abdul Sattar Date: Wed, 27 Mar 2024 14:59:49 -0400 Subject: [PATCH 2/4] fix: revert getAllMatches and do no filtering --- .../src/faux-shadow/element.ts | 6 ++++- .../synthetic-shadow/src/faux-shadow/node.ts | 6 ++++- .../src/faux-shadow/traverse.ts | 23 ++++++------------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts index 96a3e4eb45..bd1f697c22 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts @@ -92,7 +92,11 @@ function shadowRootGetterPatched(this: Element): ShadowRoot | null { function childrenGetterPatched(this: Element): HTMLCollectionOf { const owner = getNodeOwner(this); - const childNodes = getAllMatches(owner, getFilteredChildNodes(this)); + const filteredChildNodes = getFilteredChildNodes(this); + // no need to filter nodes by owner in case of light DOM + const childNodes = isNull(owner) + ? filteredChildNodes + : getAllMatches(owner, filteredChildNodes); return createStaticHTMLCollection( ArrayFilter.call(childNodes, (node: Node | Element) => node instanceof Element) ); diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts index bc452b74cc..d0b6f9685a 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts @@ -173,7 +173,11 @@ function cloneNodePatched(this: Node, deep?: boolean): Node { function childNodesGetterPatched(this: Node): NodeListOf { if (isSyntheticShadowHost(this)) { const owner = getNodeOwner(this); - const childNodes = getAllMatches(owner, getFilteredChildNodes(this)); + const filteredChildNodes = getFilteredChildNodes(this); + // no need to filter nodes by owner in case of light DOM + const childNodes = isNull(owner) + ? filteredChildNodes + : getAllMatches(owner, filteredChildNodes); return createStaticNodeList(childNodes); } // nothing to do here since this does not have a synthetic shadow attached to it diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts index 555a36a3d9..e9ac1f005f 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/traverse.ts @@ -134,26 +134,16 @@ export function isSlotElement(node: Node): node is HTMLSlotElement { return node instanceof HTMLSlotElement; } -export function isNodeOwnedBy(owner: Element | null, node: Node): boolean { - if (process.env.NODE_ENV !== 'production') { - if (!(node instanceof Node)) { - // eslint-disable-next-line no-console - console.error(`isNodeOwnedBy() should be called with a node as the second argument`); - } - } - - // owner can be null if all the parents are in the light DOM - // ownerKey is undefined for such nodes - const ownerKey = getNodeNearestOwnerKey(node); - if (isNull(owner)) { - return isUndefined(ownerKey); - } - +export function isNodeOwnedBy(owner: Element, node: Node): boolean { if (process.env.NODE_ENV !== 'production') { if (!(owner instanceof HTMLElement)) { // eslint-disable-next-line no-console console.error(`isNodeOwnedBy() should be called with an element as the first argument`); } + if (!(node instanceof Node)) { + // eslint-disable-next-line no-console + console.error(`isNodeOwnedBy() should be called with a node as the second argument`); + } if (!(compareDocumentPosition.call(node, owner) & DOCUMENT_POSITION_CONTAINS)) { // eslint-disable-next-line no-console console.error( @@ -161,6 +151,7 @@ export function isNodeOwnedBy(owner: Element | null, node: Node): boolean { ); } } + const ownerKey = getNodeNearestOwnerKey(node); if (isUndefined(ownerKey)) { // in case of root level light DOM element slotting into a synthetic shadow @@ -205,7 +196,7 @@ export function getFirstSlottedMatch(host: Element, nodeList: Element[]): Elemen return null; } -export function getAllMatches(owner: Element | null, nodeList: Node[]): T[] { +export function getAllMatches(owner: Element, nodeList: Node[]): T[] { const filteredAndPatched: T[] = []; for (let i = 0, len = nodeList.length; i < len; i += 1) { const node = nodeList[i]; From 316f47a493d937090e5d40c48d62fcabe4ff80de Mon Sep 17 00:00:00 2001 From: Abdulsattar Mohammed Date: Wed, 27 Mar 2024 17:15:21 -0400 Subject: [PATCH 3/4] Update packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts Co-authored-by: Eugene Kashida --- packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts index d0b6f9685a..e1a7a89595 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts @@ -174,7 +174,7 @@ function childNodesGetterPatched(this: Node): NodeListOf { if (isSyntheticShadowHost(this)) { const owner = getNodeOwner(this); const filteredChildNodes = getFilteredChildNodes(this); - // no need to filter nodes by owner in case of light DOM + // No need to filter by owner for non-shadowed nodes const childNodes = isNull(owner) ? filteredChildNodes : getAllMatches(owner, filteredChildNodes); From f9022763e1ca2721718379582737377cc65ba8e2 Mon Sep 17 00:00:00 2001 From: Mohammed Abdul Sattar Date: Wed, 27 Mar 2024 17:16:01 -0400 Subject: [PATCH 4/4] fix: comment --- packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts b/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts index bd1f697c22..97c9b9d8a2 100644 --- a/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts +++ b/packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts @@ -93,7 +93,7 @@ function shadowRootGetterPatched(this: Element): ShadowRoot | null { function childrenGetterPatched(this: Element): HTMLCollectionOf { const owner = getNodeOwner(this); const filteredChildNodes = getFilteredChildNodes(this); - // no need to filter nodes by owner in case of light DOM + // No need to filter by owner for non-shadowed nodes const childNodes = isNull(owner) ? filteredChildNodes : getAllMatches(owner, filteredChildNodes);