-
Notifications
You must be signed in to change notification settings - Fork 811
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
base: main
Are you sure you want to change the base?
Conversation
…tted content When experimentalSlotFixes is enabled, parentNode and parentElement for slotted content in scoped components were returning different values, causing inconsistency with standard DOM behavior. - parentNode incorrectly returned the component host element - parentElement correctly returned the actual DOM parent (wrapper element) This commit fixes the issue by: - Updating patchParentNode to return this.__parentNode (actual DOM parent) instead of this['s-ol']?.parentNode (component host) - Adding patchParentElement function for consistency - Adding 'parentElement' to validElementPatches array Both properties now consistently return the wrapper element that contains the slot, matching user expectations and DOM specifications. BREAKING CHANGE: parentNode now returns the actual DOM parent instead of the component host for slotted content. This aligns with correct DOM behavior but may affect code that relied on the previous incorrect implementation. Fixes issue where frameworks manipulating slotted content experienced insertBefore errors due to parentNode/parentElement inconsistency.
Hey @christian-bromann ! |
I believe the correct behaviour should be for parentElement to also return the component host |
@johnjenkins thank you for bringing this up. You are pointing out an interesting tension in the design philosophy of Stencil. You're absolutely correct that:
I tested this against real shadow DOM behavior, and you're right—in true shadow DOM, slotted content always maintains the component host as its parent, regardless of internal wrapper elements. However, this change addresses a real-world framework integration issue. The inconsistency between // Before the fix:
element.parentNode.tagName // "CMP-HOST" (component)
element.parentElement.tagName // "LABEL" (wrapper)
// This breaks DOM manipulation:
element.parentNode.insertBefore(newEl, element) // Error: element not in parentNode This violates the DOM specification where these properties should be consistent and is breaking frameworks like React/Vue that rely on standard DOM manipulation patterns. We're facing competing goals:
There are some options we can go here: Option 1: Enhanced Documentation + Migration Support
Option 2: Feature Flag with Deprecation Path
Option 3: Framework-Specific Workarounds
This issue highlights a broader architectural question: Should Stencil prioritize theoretical shadow DOM compatibility or practical web platform integration? Throughts? |
I’d think the fix needs to be on the insertBefore monkey patch we have on host element? |
@christian-bromann do you have a link to the 'real-world' example for me to look at? ![]() If you have something for me to look I'll fix it up 👍 |
@johnjenkins thanks for looking into this, have you tried this reproduction case https://github.com/AndreBarr/stenciljs-scoped-parentNode-issue/tree/main by @AndreBarr? |
I believe the repro just describes the expected / desired behaviour unless I'm missing something? For more context around the intention of this behaviour, here's the original PR and here's the accompanying bug it sought to fix Regarding why we don't patch If later, we find some framework using |
What is the current behavior?
When
experimentalSlotFixes
is enabled, slotted content in scoped components with wrapper elements around slots exhibits inconsistent behavior betweenparentNode
andparentElement
properties:parentNode
returns the component host element (e.g.,<my-component>
)parentElement
returns the actual DOM parent/wrapper element (e.g.,<div class="slot-wrapper">
)This inconsistency violates DOM specifications where both properties should reference the same element when the parent is an Element node. It causes issues for frameworks and libraries that manipulate slotted content, leading to
insertBefore
errors and other DOM manipulation failures.Example:
GitHub Issue Number: #6243
What is the new behavior?
Both
parentNode
andparentElement
now consistently return the actual DOM parent (wrapper element) for slotted content:parentNode
now returns the wrapper element instead of the component hostparentElement
continues to return the wrapper element (unchanged)Changes made:
patchParentNode
to returnthis.__parentNode
(actual DOM parent) instead ofthis['s-ol']?.parentNode
(component host)patchParentElement
function to ensure consistent behavior'parentElement'
to thevalidElementPatches
array for proper patchingDocumentation
No documentation changes required - this is an internal DOM behavior fix that makes Stencil conform to standard DOM specifications.
Does this introduce a breaking change?
Breaking Change Details:
parentNode
now returns the actual DOM parent (wrapper element) instead of the component host for slotted content whenexperimentalSlotFixes
is enabled.Impact:
parentNode
returning the component host will need to be updatedMigration Path:
__parentNode
property or traverse up the DOM tree manuallyTesting
Test Coverage:
scoped-slot-slotted-parentnode/cmp.test.tsx
to verifyparentNode
returns the wrapper element instead of component hostparentNode
andparentElement
return the same value**/scoped-slot*/**/*.test.tsx
) to ensure no functionality is brokenTest Results:
Manual Verification:
insertBefore
error scenarioOther information
Before the fix:
After the fix:
This fix specifically addresses issues reported by users working with Angular and other frameworks that manipulate slotted content dynamically, where the
parentNode
/parentElement
inconsistency was causingNotFoundError: Failed to execute 'insertBefore'
errors.The fix only affects components with
experimentalSlotFixes: true
in the Stencil config, so it will not impact applications not using this experimental feature.