Skip to content

fix(runtime): ensure parentNode and parentElement consistency for slotted content #6284

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 26 additions & 3 deletions src/runtime/dom-extras.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ const patchInsertBefore = (HostElementPrototype: HTMLElement) => {
* Fixes an issue where slotted elements are dynamically relocated in React, such as after data fetch.
*
* When a slotted element is passed to another scoped component (e.g., <A><C slot="header"/></A>),
* the childs __parentNode (original parent node property) does not match this.
* the child's __parentNode (original parent node property) does not match this.
*
* To prevent errors, this checks if the current child's parent node differs from this.
* If so, appendChild(newChild) is called to ensure the child is correctly inserted,
Expand Down Expand Up @@ -433,6 +433,7 @@ export const patchSlottedNode = (node: Node) => {
if (node.nodeType === Node.ELEMENT_NODE) {
patchNextElementSibling(node as Element);
patchPreviousElementSibling(node as Element);
patchParentElement(node as Element);
}
};

Expand Down Expand Up @@ -533,7 +534,8 @@ export const patchParentNode = (node: Node) => {
patchHostOriginalAccessor('parentNode', node);
Object.defineProperty(node, 'parentNode', {
get: function () {
return this['s-ol']?.parentNode || this.__parentNode;
// Return the actual DOM parent (wrapper element) instead of the slot original location parent
return this.__parentNode;
},
set: function (value) {
// mock-doc sets parentNode?
Expand All @@ -542,9 +544,30 @@ export const patchParentNode = (node: Node) => {
});
};

/**
* Patches the `parentElement` accessor of a non-shadow slotted node
*
* @param element the slotted element node to be patched
*/
const patchParentElement = (element: Element) => {
if (!element || (element as any).__parentElement) return;

patchHostOriginalAccessor('parentElement', element);
Object.defineProperty(element, 'parentElement', {
get: function () {
// Return the actual DOM parent (wrapper element) to match parentNode behavior
return this.__parentElement;
},
set: function (value) {
// mock-doc sets parentElement?
this.__parentElement = value;
},
});
};

/// UTILS ///

const validElementPatches = ['children', 'nextElementSibling', 'previousElementSibling'] as const;
const validElementPatches = ['children', 'nextElementSibling', 'previousElementSibling', 'parentElement'] as const;
const validNodesPatches = [
'childNodes',
'firstChild',
Expand Down
27 changes: 24 additions & 3 deletions test/wdio/scoped-slot-slotted-parentnode/cmp.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { h } from '@stencil/core';
import { render } from '@wdio/browser-runner/stencil';
import { $, expect } from '@wdio/globals';

describe('checks slotted node parentNode', () => {
beforeEach(async () => {
render({
components: [],
template: () => (
<cmp-slotted-parentnode>
A text node <div>An element</div>
Expand All @@ -13,17 +15,36 @@ describe('checks slotted node parentNode', () => {
await $('cmp-slotted-parentnode label').waitForExist();
});

it('slotted nodes and elements `parentNode` do not return component internals', async () => {
it('slotted nodes and elements `parentNode` return the actual DOM parent', async () => {
// After the fix, parentNode should return the actual DOM parent (wrapper element)
expect((document.querySelector('cmp-slotted-parentnode').children[0].parentNode as Element).tagName).toBe(
'CMP-SLOTTED-PARENTNODE',
'LABEL',
);
expect((document.querySelector('cmp-slotted-parentnode').childNodes[0].parentNode as Element).tagName).toBe(
'CMP-SLOTTED-PARENTNODE',
'LABEL',
);
});

it('slotted nodes and elements `__parentNode` return component internals', async () => {
expect((document.querySelector('cmp-slotted-parentnode').children[0] as any).__parentNode.tagName).toBe('LABEL');
expect((document.querySelector('cmp-slotted-parentnode').childNodes[0] as any).__parentNode.tagName).toBe('LABEL');
});

it('should have consistent parentNode and parentElement behavior', async () => {
const slottedElement = document.querySelector('cmp-slotted-parentnode').children[0];
const wrapperLabel = document.querySelector('cmp-slotted-parentnode label');

// Both should reference the same element - the actual DOM parent (wrapper element)
const parentNodeType = (slottedElement.parentNode as Element)?.tagName;
const parentElementType = slottedElement.parentElement?.tagName;

// According to the user's expected behavior, both should return the wrapper element
expect(slottedElement.parentNode).toBe(wrapperLabel);
expect(slottedElement.parentElement).toBe(wrapperLabel);

// Both should be consistent
expect(slottedElement.parentNode).toBe(slottedElement.parentElement);
expect(parentNodeType).toBe('LABEL');
expect(parentElementType).toBe('LABEL');
});
});
Loading