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.

Sign up for GitHub

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

Jump to bottom

fix: children and childNodes not returning top-level slotted children #4098

Merged
merged 4 commits into from Mar 27, 2024
Merged

fix: children and childNodes not returning top-level slotted children #4098

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9843b5a
fix: children and childNodes not returning top-level slotted children
abdulsattar Mar 25, 2024
6166bbe
fix: revert getAllMatches and do no filtering
abdulsattar Mar 27, 2024
316f47a
Update packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts
abdulsattar Mar 27, 2024
f902276
fix: comment
abdulsattar Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/@lwc/integration-karma/test/light-dom/root/index.spec.js
View file
Open in desktop
Expand Up @@ -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);
});
});
6 changes: 5 additions & 1 deletion packages/@lwc/synthetic-shadow/src/faux-shadow/element.ts
View file
Open in desktop
Expand Up @@ -92,7 +92,11 @@ function shadowRootGetterPatched(this: Element): ShadowRoot | null {

function childrenGetterPatched(this: Element): HTMLCollectionOf<Element> {
const owner = getNodeOwner(this);
const childNodes = isNull(owner) ? [] : getAllMatches(owner, getFilteredChildNodes(this));
const filteredChildNodes = getFilteredChildNodes(this);
// no need to filter nodes by owner in case of light DOM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also applies to root shadow DOM components:

Suggested change
// no need to filter nodes by owner in case of light DOM
// No need to filter by owner for non-shadowed elements

const childNodes = isNull(owner)
? filteredChildNodes
: getAllMatches(owner, filteredChildNodes);
return createStaticHTMLCollection(
ArrayFilter.call(childNodes, (node: Node | Element) => node instanceof Element)
);
Expand Down
6 changes: 5 additions & 1 deletion packages/@lwc/synthetic-shadow/src/faux-shadow/node.ts
View file
Open in desktop
Expand Up @@ -173,7 +173,11 @@ function cloneNodePatched(this: Node, deep?: boolean): Node {
function childNodesGetterPatched(this: Node): NodeListOf<Node> {
if (isSyntheticShadowHost(this)) {
const owner = getNodeOwner(this);
const childNodes = isNull(owner) ? [] : getAllMatches(owner, getFilteredChildNodes(this));
const filteredChildNodes = getFilteredChildNodes(this);
// no need to filter nodes by owner in case of light DOM
abdulsattar marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
2 changes: 2 additions & 0 deletions packages/@lwc/synthetic-shadow/src/shared/node-ownership.ts
View file
Open in desktop
Expand Up @@ -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
Comment on lines +61 to +62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand this comment. Are we short-circuiting here so that we don't need to traverse to the root?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <slot> element will belong to some other shadow root and check if the node belongs to the owner of the <slot> element. This check will fail and it won't return any elements.

if (!isNull(host) && isSyntheticSlotElement(host)) {
return undefined;
}
Expand Down