Skip to content

Commit

Permalink
fix(runtime): relocate slotted content when slot parent element tag c…
Browse files Browse the repository at this point in the history
…hanges (#5120)

* fix(runtime): re-relocate slot if parent element's tagname changes

If a slot is located in an element and that element's tag is dynamically changed (e.g. from `p` to `span`), we need to re-relocate the slot on re-render

STENCIL-672: slot element loses its parent reference and disappears when its parent is rendered conditionally

Fixes: #4284, #3913

* add e2e tests

* code documentation

* put changes behind slot fix flag

* resolve new SNC

* Apply suggestions from code review

Co-authored-by: Ryan Waskiewicz <[email protected]>

---------

Co-authored-by: Ryan Waskiewicz <[email protected]>
  • Loading branch information
tanner-reits and rwaskiewicz authored Dec 6, 2023
1 parent e78a9a6 commit 4303d6a
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 8 deletions.
55 changes: 47 additions & 8 deletions src/runtime/vdom/vdom-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,27 +154,66 @@ const createElm = (oldParentVNode: d.VNode, newParentVNode: d.VNode, childIndex:
// check if we've got an old vnode for this slot
oldVNode = oldParentVNode && oldParentVNode.$children$ && oldParentVNode.$children$[childIndex];
if (oldVNode && oldVNode.$tag$ === newVNode.$tag$ && oldParentVNode.$elm$) {
// we've got an old slot vnode and the wrapper is being replaced
// so let's move the old slot content back to it's original location
putBackInOriginalLocation(oldParentVNode.$elm$, false);
if (BUILD.experimentalSlotFixes) {
// we've got an old slot vnode and the wrapper is being replaced
// so let's move the old slot content to the root of the element currently being rendered
relocateToHostRoot(oldParentVNode.$elm$);
} else {
// we've got an old slot vnode and the wrapper is being replaced
// so let's move the old slot content back to its original location
putBackInOriginalLocation(oldParentVNode.$elm$, false);
}
}
}
}

return elm;
};

/**
* Relocates all child nodes of an element that were a part of a previous slot relocation
* to the root of the Stencil component currently being rendered. This happens when a parent
* element of a slot reference node dynamically changes and triggers a re-render. We cannot use
* `putBackInOriginalLocation()` because that may relocate nodes to elements that will not be re-rendered
* and so they will not be relocated again.
*
* @param parentElm The element potentially containing relocated nodes.
*/
const relocateToHostRoot = (parentElm: Element) => {
plt.$flags$ |= PLATFORM_FLAGS.isTmpDisconnected;

const host = parentElm.closest(hostTagName.toLowerCase());
if (host != null) {
for (const childNode of Array.from(parentElm.childNodes) as d.RenderNode[]) {
// Only relocate nodes that were slotted in
if (childNode['s-sh'] != null) {
host.insertBefore(childNode, null);
// Reset so we can correctly move the node around again.
childNode['s-sh'] = undefined;

// When putting an element node back in its original location,
// we need to reset the `slot` attribute back to the value it originally had
// so we can correctly relocate it again in the future
if (childNode.nodeType === NODE_TYPE.ElementNode && !!childNode['s-sn']) {
childNode.setAttribute('slot', childNode['s-sn']);
}

// Need to tell the render pipeline to check to relocate slot content again
checkSlotRelocate = true;
}
}
}

plt.$flags$ &= ~PLATFORM_FLAGS.isTmpDisconnected;
};

const putBackInOriginalLocation = (parentElm: Node, recursive: boolean) => {
plt.$flags$ |= PLATFORM_FLAGS.isTmpDisconnected;

const oldSlotChildNodes = parentElm.childNodes;
for (let i = oldSlotChildNodes.length - 1; i >= 0; i--) {
const childNode = oldSlotChildNodes[i] as any;
if (childNode['s-hn'] !== hostTagName && childNode['s-ol']) {
// // this child node in the old element is from another component
// // remove this node from the old slot's parent
// childNode.remove();

// and relocate it back to it's original location
parentReferenceNode(childNode).insertBefore(childNode, referenceNode(childNode));

Expand Down Expand Up @@ -1027,7 +1066,7 @@ render() {
// has a different next sibling or parent relocated

if (nodeToRelocate !== insertBeforeNode) {
if (!nodeToRelocate['s-hn'] && nodeToRelocate['s-ol']) {
if (!BUILD.experimentalSlotFixes && !nodeToRelocate['s-hn'] && nodeToRelocate['s-ol']) {
// probably a component in the index.html that doesn't have its hostname set
nodeToRelocate['s-hn'] = nodeToRelocate['s-ol'].parentNode.nodeName;
}
Expand Down
30 changes: 30 additions & 0 deletions test/karma/test-app/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,12 @@ export namespace Components {
}
interface SlotNoDefault {
}
interface SlotParentTagChange {
"element": string;
}
interface SlotParentTagChangeRoot {
"element": string;
}
interface SlotReorder {
"reordered": boolean;
}
Expand Down Expand Up @@ -1306,6 +1312,18 @@ declare global {
prototype: HTMLSlotNoDefaultElement;
new (): HTMLSlotNoDefaultElement;
};
interface HTMLSlotParentTagChangeElement extends Components.SlotParentTagChange, HTMLStencilElement {
}
var HTMLSlotParentTagChangeElement: {
prototype: HTMLSlotParentTagChangeElement;
new (): HTMLSlotParentTagChangeElement;
};
interface HTMLSlotParentTagChangeRootElement extends Components.SlotParentTagChangeRoot, HTMLStencilElement {
}
var HTMLSlotParentTagChangeRootElement: {
prototype: HTMLSlotParentTagChangeRootElement;
new (): HTMLSlotParentTagChangeRootElement;
};
interface HTMLSlotReorderElement extends Components.SlotReorder, HTMLStencilElement {
}
var HTMLSlotReorderElement: {
Expand Down Expand Up @@ -1538,6 +1556,8 @@ declare global {
"slot-nested-order-parent": HTMLSlotNestedOrderParentElement;
"slot-ng-if": HTMLSlotNgIfElement;
"slot-no-default": HTMLSlotNoDefaultElement;
"slot-parent-tag-change": HTMLSlotParentTagChangeElement;
"slot-parent-tag-change-root": HTMLSlotParentTagChangeRootElement;
"slot-reorder": HTMLSlotReorderElement;
"slot-reorder-root": HTMLSlotReorderRootElement;
"slot-replace-wrapper": HTMLSlotReplaceWrapperElement;
Expand Down Expand Up @@ -1904,6 +1924,12 @@ declare namespace LocalJSX {
}
interface SlotNoDefault {
}
interface SlotParentTagChange {
"element"?: string;
}
interface SlotParentTagChangeRoot {
"element"?: string;
}
interface SlotReorder {
"reordered"?: boolean;
}
Expand Down Expand Up @@ -2074,6 +2100,8 @@ declare namespace LocalJSX {
"slot-nested-order-parent": SlotNestedOrderParent;
"slot-ng-if": SlotNgIf;
"slot-no-default": SlotNoDefault;
"slot-parent-tag-change": SlotParentTagChange;
"slot-parent-tag-change-root": SlotParentTagChangeRoot;
"slot-reorder": SlotReorder;
"slot-reorder-root": SlotReorderRoot;
"slot-replace-wrapper": SlotReplaceWrapper;
Expand Down Expand Up @@ -2231,6 +2259,8 @@ declare module "@stencil/core" {
"slot-nested-order-parent": LocalJSX.SlotNestedOrderParent & JSXBase.HTMLAttributes<HTMLSlotNestedOrderParentElement>;
"slot-ng-if": LocalJSX.SlotNgIf & JSXBase.HTMLAttributes<HTMLSlotNgIfElement>;
"slot-no-default": LocalJSX.SlotNoDefault & JSXBase.HTMLAttributes<HTMLSlotNoDefaultElement>;
"slot-parent-tag-change": LocalJSX.SlotParentTagChange & JSXBase.HTMLAttributes<HTMLSlotParentTagChangeElement>;
"slot-parent-tag-change-root": LocalJSX.SlotParentTagChangeRoot & JSXBase.HTMLAttributes<HTMLSlotParentTagChangeRootElement>;
"slot-reorder": LocalJSX.SlotReorder & JSXBase.HTMLAttributes<HTMLSlotReorderElement>;
"slot-reorder-root": LocalJSX.SlotReorderRoot & JSXBase.HTMLAttributes<HTMLSlotReorderRootElement>;
"slot-replace-wrapper": LocalJSX.SlotReplaceWrapper & JSXBase.HTMLAttributes<HTMLSlotReplaceWrapperElement>;
Expand Down
16 changes: 16 additions & 0 deletions test/karma/test-app/slot-parent-tag-change/cmp-root.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Component, h, Prop } from '@stencil/core';

@Component({
tag: 'slot-parent-tag-change-root',
})
export class SlotParentTagChangeRoot {
@Prop() element = 'p';

render() {
return (
<slot-parent-tag-change element={this.element}>
<slot></slot>
</slot-parent-tag-change>
);
}
}
16 changes: 16 additions & 0 deletions test/karma/test-app/slot-parent-tag-change/cmp.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Component, h, Prop } from '@stencil/core';

@Component({
tag: 'slot-parent-tag-change',
})
export class SlotParentTagChange {
@Prop() element = 'p';

render() {
return (
<this.element>
<slot></slot>
</this.element>
);
}
}
21 changes: 21 additions & 0 deletions test/karma/test-app/slot-parent-tag-change/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html>
<meta charset="utf8" />
<script src="./build/testapp.esm.js" type="module"></script>
<script src="./build/testapp.js" nomodule></script>

<!-- Test "top-level" slot -->
<slot-parent-tag-change id="top-level"> Hello </slot-parent-tag-change>
<!-- Test nested slot -->
<slot-parent-tag-change-root> World </slot-parent-tag-change-root>

<button type="button" id="top-level-button">Change Top-Level Slot</button>
<button type="button" id="nested-button">Change Nested Slot</button>

<script>
document.querySelector('#top-level-button').addEventListener('click', () => {
document.querySelector('#top-level').setAttribute('element', 'span');
});
document.querySelector('#nested-button').addEventListener('click', () => {
document.querySelector('slot-parent-tag-change-root').setAttribute('element', 'span');
});
</script>
52 changes: 52 additions & 0 deletions test/karma/test-app/slot-parent-tag-change/karma.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { setupDomTests, waitForChanges } from '../util';

/**
* Tests the cases where a node is slotted in from the root `index.html` file
* and the slot's parent element dynamically changes (e.g. from `p` to `span`).
*/
describe('slot-parent-tag-change', () => {
const { setupDom, tearDownDom } = setupDomTests(document);
let app: HTMLElement;

beforeEach(async () => {
app = await setupDom('/slot-parent-tag-change/index.html');
});

afterEach(tearDownDom);

describe('direct slot', () => {
it('should relocate the text node to the slot after the parent tag changes', async () => {
const root = app.querySelector('#top-level');

expect(root).not.toBeNull();
expect(root.children.length).toBe(1);
expect(root.children[0].tagName).toBe('P');
expect(root.children[0].textContent.trim()).toBe('Hello');

app.querySelector<HTMLButtonElement>('#top-level-button').click();
await waitForChanges();

expect(root.children.length).toBe(1);
expect(root.children[0].tagName).toBe('SPAN');
expect(root.children[0].textContent.trim()).toBe('Hello');
});
});

describe('nested slot', () => {
it('should relocate the text node to the slot after the parent tag changes', async () => {
const root = app.querySelector('slot-parent-tag-change-root slot-parent-tag-change');

expect(root).not.toBeNull();
expect(root.children.length).toBe(1);
expect(root.children[0].tagName).toBe('P');
expect(root.children[0].textContent.trim()).toBe('World');

app.querySelector<HTMLButtonElement>('#nested-button').click();
await waitForChanges();

expect(root.children.length).toBe(1);
expect(root.children[0].tagName).toBe('SPAN');
expect(root.children[0].textContent.trim()).toBe('World');
});
});
});

0 comments on commit 4303d6a

Please sign in to comment.