Skip to content

Commit

Permalink
fix(runtime): relocate slot content from non-shadow to shadow compone…
Browse files Browse the repository at this point in the history
…nts w/ slot name change (#4940)

* yo this might actually work

* create single build flag for `experimentalSlotFixes`

* gate new vDOM logic behind build flag

* add e2e test for repro case

* document new node property

* fix(runtime): keep track of a slot's slot attribute for shadow contexts

* ignore non-element nodes

* add e2e test for repro case

* fix SNC

* comment prop assignment

* PR feedback
  • Loading branch information
tanner-reits committed Oct 23, 2023
1 parent adb3ccf commit 0fe78c7
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 3 deletions.
18 changes: 18 additions & 0 deletions src/declarations/stencil-private.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,24 @@ export interface RenderNode extends HostElement {
*/
['s-sh']?: string;

/**
* Slot forward slot:
* This is the slot that the original `slot` tag element was going to be
* forwarded to in another element. For instance:
*
* ```html
* <my-cmp>
* <slot name="my-slot" slot="another-slot"></slot>
* </my-cmp>
* ```
*
* In this case, the value would be `another-slot`.
*
* This allows us to correctly set the `slot` attribute on an element when it is moved
* from a non-shadow to shadow element.
*/
['s-fs']?: string;

/**
* Original Location Reference:
* A reference pointing to the comment
Expand Down
33 changes: 30 additions & 3 deletions src/runtime/vdom/vdom-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ const createElm = (oldParentVNode: d.VNode, newParentVNode: d.VNode, childIndex:
// remember the content reference comment
elm['s-sr'] = true;

// Persist the name of the slot that this slot was going to be projected into.
elm['s-fs'] = newVNode.$attrs$?.slot;

// remember the content reference comment
elm['s-cr'] = contentRef;

Expand Down Expand Up @@ -972,12 +975,13 @@ render() {
for (i = 0; i < relocateNodes.length; i++) {
relocateData = relocateNodes[i];
nodeToRelocate = relocateData.$nodeToRelocate$;
const slotRefNode = relocateData.$slotRefNode$;

if (relocateData.$slotRefNode$) {
if (slotRefNode) {
// by default we're just going to insert it directly
// after the slot reference node
parentNodeRef = relocateData.$slotRefNode$.parentNode;
insertBeforeNode = relocateData.$slotRefNode$.nextSibling;
parentNodeRef = slotRefNode.parentNode;
insertBeforeNode = slotRefNode.nextSibling;
orgLocationNode = nodeToRelocate['s-ol'] as any;

while ((orgLocationNode = orgLocationNode.previousSibling as any)) {
Expand All @@ -1004,6 +1008,29 @@ render() {
// probably a component in the index.html that doesn't have its hostname set
nodeToRelocate['s-hn'] = nodeToRelocate['s-ol'].parentNode.nodeName;
}

// Handle a niche use-case where we relocate a slot from a non-shadow
// to shadow component (and potentially into further nested components) where
// the slot name changes along the way (for instance, a default to a named slot).
// In this case, we need to update the relocated node's slot attribute to match
// the slot name it is being relocated into.
//
// There is a very niche use case where we may be relocating a text node. For now,
// we ignore anything that is not an element node since non-element nodes cannot have
// attributes to specify the slot. We'll deal with this if it becomes a problem... but super edge-case-y
if (
BUILD.experimentalSlotFixes &&
nodeToRelocate.nodeType === NODE_TYPE.ElementNode &&
slotRefNode.parentElement?.shadowRoot != null &&
slotRefNode['s-fs'] !== nodeToRelocate.getAttribute('slot')
) {
if (!slotRefNode['s-fs']) {
nodeToRelocate.removeAttribute('slot');
} else {
nodeToRelocate.setAttribute('slot', slotRefNode['s-fs']);
}
}

// add it back to the dom but in its new home
parentNodeRef.insertBefore(nodeToRelocate, insertBeforeNode);
}
Expand Down
39 changes: 39 additions & 0 deletions test/karma/test-app/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ export namespace Components {
}
interface DomReattachCloneHost {
}
interface DropDown {
}
interface DropDownContent {
}
interface DynamicCssVariable {
}
interface DynamicImport {
Expand Down Expand Up @@ -211,6 +215,8 @@ export namespace Components {
}
interface NodeResolution {
}
interface NonShadowHost {
}
interface ParentReflectNanAttribute {
}
interface ParentWithReflectChild {
Expand Down Expand Up @@ -613,6 +619,18 @@ declare global {
prototype: HTMLDomReattachCloneHostElement;
new (): HTMLDomReattachCloneHostElement;
};
interface HTMLDropDownElement extends Components.DropDown, HTMLStencilElement {
}
var HTMLDropDownElement: {
prototype: HTMLDropDownElement;
new (): HTMLDropDownElement;
};
interface HTMLDropDownContentElement extends Components.DropDownContent, HTMLStencilElement {
}
var HTMLDropDownContentElement: {
prototype: HTMLDropDownContentElement;
new (): HTMLDropDownContentElement;
};
interface HTMLDynamicCssVariableElement extends Components.DynamicCssVariable, HTMLStencilElement {
}
var HTMLDynamicCssVariableElement: {
Expand Down Expand Up @@ -946,6 +964,12 @@ declare global {
prototype: HTMLNodeResolutionElement;
new (): HTMLNodeResolutionElement;
};
interface HTMLNonShadowHostElement extends Components.NonShadowHost, HTMLStencilElement {
}
var HTMLNonShadowHostElement: {
prototype: HTMLNonShadowHostElement;
new (): HTMLNonShadowHostElement;
};
interface HTMLParentReflectNanAttributeElement extends Components.ParentReflectNanAttribute, HTMLStencilElement {
}
var HTMLParentReflectNanAttributeElement: {
Expand Down Expand Up @@ -1373,6 +1397,8 @@ declare global {
"dom-reattach-clone": HTMLDomReattachCloneElement;
"dom-reattach-clone-deep-slot": HTMLDomReattachCloneDeepSlotElement;
"dom-reattach-clone-host": HTMLDomReattachCloneHostElement;
"drop-down": HTMLDropDownElement;
"drop-down-content": HTMLDropDownContentElement;
"dynamic-css-variable": HTMLDynamicCssVariableElement;
"dynamic-import": HTMLDynamicImportElement;
"es5-addclass-svg": HTMLEs5AddclassSvgElement;
Expand Down Expand Up @@ -1415,6 +1441,7 @@ declare global {
"listen-window": HTMLListenWindowElement;
"no-delegates-focus": HTMLNoDelegatesFocusElement;
"node-resolution": HTMLNodeResolutionElement;
"non-shadow-host": HTMLNonShadowHostElement;
"parent-reflect-nan-attribute": HTMLParentReflectNanAttributeElement;
"parent-with-reflect-child": HTMLParentWithReflectChildElement;
"reflect-nan-attribute": HTMLReflectNanAttributeElement;
Expand Down Expand Up @@ -1586,6 +1613,10 @@ declare namespace LocalJSX {
}
interface DomReattachCloneHost {
}
interface DropDown {
}
interface DropDownContent {
}
interface DynamicCssVariable {
}
interface DynamicImport {
Expand Down Expand Up @@ -1690,6 +1721,8 @@ declare namespace LocalJSX {
}
interface NodeResolution {
}
interface NonShadowHost {
}
interface ParentReflectNanAttribute {
}
interface ParentWithReflectChild {
Expand Down Expand Up @@ -1883,6 +1916,8 @@ declare namespace LocalJSX {
"dom-reattach-clone": DomReattachClone;
"dom-reattach-clone-deep-slot": DomReattachCloneDeepSlot;
"dom-reattach-clone-host": DomReattachCloneHost;
"drop-down": DropDown;
"drop-down-content": DropDownContent;
"dynamic-css-variable": DynamicCssVariable;
"dynamic-import": DynamicImport;
"es5-addclass-svg": Es5AddclassSvg;
Expand Down Expand Up @@ -1925,6 +1960,7 @@ declare namespace LocalJSX {
"listen-window": ListenWindow;
"no-delegates-focus": NoDelegatesFocus;
"node-resolution": NodeResolution;
"non-shadow-host": NonShadowHost;
"parent-reflect-nan-attribute": ParentReflectNanAttribute;
"parent-with-reflect-child": ParentWithReflectChild;
"reflect-nan-attribute": ReflectNanAttribute;
Expand Down Expand Up @@ -2032,6 +2068,8 @@ declare module "@stencil/core" {
"dom-reattach-clone": LocalJSX.DomReattachClone & JSXBase.HTMLAttributes<HTMLDomReattachCloneElement>;
"dom-reattach-clone-deep-slot": LocalJSX.DomReattachCloneDeepSlot & JSXBase.HTMLAttributes<HTMLDomReattachCloneDeepSlotElement>;
"dom-reattach-clone-host": LocalJSX.DomReattachCloneHost & JSXBase.HTMLAttributes<HTMLDomReattachCloneHostElement>;
"drop-down": LocalJSX.DropDown & JSXBase.HTMLAttributes<HTMLDropDownElement>;
"drop-down-content": LocalJSX.DropDownContent & JSXBase.HTMLAttributes<HTMLDropDownContentElement>;
"dynamic-css-variable": LocalJSX.DynamicCssVariable & JSXBase.HTMLAttributes<HTMLDynamicCssVariableElement>;
"dynamic-import": LocalJSX.DynamicImport & JSXBase.HTMLAttributes<HTMLDynamicImportElement>;
"es5-addclass-svg": LocalJSX.Es5AddclassSvg & JSXBase.HTMLAttributes<HTMLEs5AddclassSvgElement>;
Expand Down Expand Up @@ -2074,6 +2112,7 @@ declare module "@stencil/core" {
"listen-window": LocalJSX.ListenWindow & JSXBase.HTMLAttributes<HTMLListenWindowElement>;
"no-delegates-focus": LocalJSX.NoDelegatesFocus & JSXBase.HTMLAttributes<HTMLNoDelegatesFocusElement>;
"node-resolution": LocalJSX.NodeResolution & JSXBase.HTMLAttributes<HTMLNodeResolutionElement>;
"non-shadow-host": LocalJSX.NonShadowHost & JSXBase.HTMLAttributes<HTMLNonShadowHostElement>;
"parent-reflect-nan-attribute": LocalJSX.ParentReflectNanAttribute & JSXBase.HTMLAttributes<HTMLParentReflectNanAttributeElement>;
"parent-with-reflect-child": LocalJSX.ParentWithReflectChild & JSXBase.HTMLAttributes<HTMLParentWithReflectChildElement>;
"reflect-nan-attribute": LocalJSX.ReflectNanAttribute & JSXBase.HTMLAttributes<HTMLReflectNanAttributeElement>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Component, h } from '@stencil/core';

@Component({
tag: 'drop-down-content',
shadow: true,
})
export class DropdownContent {
render() {
return (
<div>
<p>content before</p>
<slot />
<p>content after</p>
</div>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Component, h } from '@stencil/core';

@Component({
tag: 'drop-down',
shadow: true,
})
export class Dropdown {
render() {
return (
<div>
<p>dropdown before</p>
<slot name="main-content" />
<slot name="dropdown-content-element" />
<p>dropdown after</p>
</div>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!doctype html>
<meta charset="utf8" />
<script src="./build/testapp.esm.js" type="module"></script>
<script src="./build/testapp.js" nomodule></script>

<non-shadow-host>
<b slot="main-content">main slotted content</b>
<b slot="dropdown-content">dropdown slotted content</b>
</non-shadow-host>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { setupDomTests } from '../util';

describe('non-shadow-to-shadow-slot-relocation', () => {
const { setupDom, tearDownDom } = setupDomTests(document);
let app: HTMLElement | undefined;
let host: HTMLElement | undefined;

beforeEach(async () => {
app = await setupDom('non-shadow-to-shadow-slot-relocation/index.html');
host = app.querySelector('non-shadow-host');
});

afterEach(tearDownDom);

it('should correctly render slotted content', () => {
expect(host).toBeDefined();

const mainDiv = host.firstElementChild.firstElementChild;
expect(mainDiv).toBeDefined();
expect(mainDiv.children.length).toBe(2);

// Main content
expect(mainDiv.children[0].tagName).toBe('B');
expect(mainDiv.children[0].getAttribute('slot')).toBe('main-content');

// Dropdown content
const slottedDiv = mainDiv.children[1];
expect(slottedDiv.tagName).toBe('DIV');
expect(slottedDiv.getAttribute('slot')).toBe('dropdown-content-element');
expect(slottedDiv.children.length).toBe(1);

const slottedDropdownContent = slottedDiv.children[0];
expect(slottedDropdownContent.tagName).toBe('DROP-DOWN-CONTENT');
expect(slottedDropdownContent.children.length).toBe(1);
expect(slottedDropdownContent.children[0].tagName).toBe('B');
expect(slottedDropdownContent.children[0].getAttribute('slot')).toBe(null);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { Component, h } from '@stencil/core';

@Component({
tag: 'non-shadow-host',
shadow: false,
})
export class NonShadowHost {
render() {
return (
<div>
<drop-down>
<slot name="main-content" slot="main-content" />
<div slot="dropdown-content-element">
<drop-down-content>
<slot name="dropdown-content" />
</drop-down-content>
</div>
</drop-down>
</div>
);
}
}

0 comments on commit 0fe78c7

Please sign in to comment.