Skip to content

Commit

Permalink
fix(runtime): nested multiple default slot relocation (#5403)
Browse files Browse the repository at this point in the history
* fix(renderer): add hostname to host slot node finder method

expected hostname is added to the getHostSlotNode method to retrieve the correct host's slot node since it was getting the first found slot node. However, same slot name or even another custom component's default slot can be placed before the searching host element's slot node

fixes one of the issues raised in #5335

* fix(renderer): add check for already relocated nodes to avoid reordering

another check is added to avoid changing already relocated nodes order by updating the anchor node which is used by insertBefore

fixes one of the issues raised in #5335

* test(slot): add e2e test to check order of dynamically generated nested slot elements

* test(slot): fix failing test case by adding optional chaining

* test(wdio): migrate conditional rendering & nested dynamic & slot ref tests to wdio

---------

Co-authored-by: Tanner Reits <47483144+tanner-reits@users.noreply.github.com>
  • Loading branch information
yigityuce and tanner-reits committed Apr 1, 2024
1 parent 2fd001d commit 363c07b
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 10 deletions.
17 changes: 9 additions & 8 deletions src/runtime/dom-extras.ts
Expand Up @@ -81,7 +81,7 @@ export const patchSlotAppendChild = (HostElementPrototype: any) => {
HostElementPrototype.__appendChild = HostElementPrototype.appendChild;
HostElementPrototype.appendChild = function (this: d.RenderNode, newChild: d.RenderNode) {
const slotName = (newChild['s-sn'] = getSlotName(newChild));
const slotNode = getHostSlotNode(this.childNodes, slotName);
const slotNode = getHostSlotNode(this.childNodes, slotName, this.tagName);
if (slotNode) {
const slotChildNodes = getHostSlotChildNodes(slotNode, slotName);
const appendAfter = slotChildNodes[slotChildNodes.length - 1];
Expand All @@ -107,7 +107,7 @@ const patchSlotRemoveChild = (ElementPrototype: any) => {
ElementPrototype.__removeChild = ElementPrototype.removeChild;
ElementPrototype.removeChild = function (this: d.RenderNode, toRemove: d.RenderNode) {
if (toRemove && typeof toRemove['s-sn'] !== 'undefined') {
const slotNode = getHostSlotNode(this.childNodes, toRemove['s-sn']);
const slotNode = getHostSlotNode(this.childNodes, toRemove['s-sn'], this.tagName);
if (slotNode) {
// Get all slot content
const slotChildNodes = getHostSlotChildNodes(slotNode, toRemove['s-sn']);
Expand Down Expand Up @@ -141,7 +141,7 @@ export const patchSlotPrepend = (HostElementPrototype: HTMLElement) => {
newChild = this.ownerDocument.createTextNode(newChild) as unknown as d.RenderNode;
}
const slotName = (newChild['s-sn'] = getSlotName(newChild));
const slotNode = getHostSlotNode(this.childNodes, slotName);
const slotNode = getHostSlotNode(this.childNodes, slotName, this.tagName);
if (slotNode) {
const slotPlaceholder: d.RenderNode = document.createTextNode('') as any;
slotPlaceholder['s-nr'] = newChild;
Expand Down Expand Up @@ -322,7 +322,7 @@ export const patchTextContent = (hostElementPrototype: HTMLElement): void => {
get(): string | null {
// get the 'default slot', which would be the first slot in a shadow tree (if we were using one), whose name is
// the empty string
const slotNode = getHostSlotNode(this.childNodes, '');
const slotNode = getHostSlotNode(this.childNodes, '', this.tagName);
// when a slot node is found, the textContent _may_ be found in the next sibling (text) node, depending on how
// nodes were reordered during the vdom render. first try to get the text content from the sibling.
if (slotNode?.nextSibling?.nodeType === NODE_TYPES.TEXT_NODE) {
Expand All @@ -338,7 +338,7 @@ export const patchTextContent = (hostElementPrototype: HTMLElement): void => {
set(value: string | null) {
// get the 'default slot', which would be the first slot in a shadow tree (if we were using one), whose name is
// the empty string
const slotNode = getHostSlotNode(this.childNodes, '');
const slotNode = getHostSlotNode(this.childNodes, '', this.tagName);
// when a slot node is found, the textContent _may_ need to be placed in the next sibling (text) node,
// depending on how nodes were reordered during the vdom render. first try to set the text content on the
// sibling.
Expand Down Expand Up @@ -431,18 +431,19 @@ const getSlotName = (node: d.RenderNode) =>
* Recursively searches a series of child nodes for a slot with the provided name.
* @param childNodes the nodes to search for a slot with a specific name.
* @param slotName the name of the slot to match on.
* @param hostName the host name of the slot to match on.
* @returns a reference to the slot node that matches the provided name, `null` otherwise
*/
const getHostSlotNode = (childNodes: NodeListOf<ChildNode>, slotName: string) => {
const getHostSlotNode = (childNodes: NodeListOf<ChildNode>, slotName: string, hostName: string) => {
let i = 0;
let childNode: d.RenderNode;

for (; i < childNodes.length; i++) {
childNode = childNodes[i] as any;
if (childNode['s-sr'] && childNode['s-sn'] === slotName) {
if (childNode['s-sr'] && childNode['s-sn'] === slotName && childNode['s-hn'] === hostName) {
return childNode;
}
childNode = getHostSlotNode(childNode.childNodes, slotName);
childNode = getHostSlotNode(childNode.childNodes, slotName, hostName);
if (childNode) {
return childNode;
}
Expand Down
10 changes: 8 additions & 2 deletions src/runtime/vdom/vdom-render.ts
Expand Up @@ -1061,12 +1061,18 @@ render() {
(insertBeforeNode && insertBeforeNode.nodeType === NODE_TYPE.ElementNode)
) {
let orgLocationNode = nodeToRelocate['s-ol']?.previousSibling as d.RenderNode | null;

while (orgLocationNode) {
let refNode = orgLocationNode['s-nr'] ?? null;

if (refNode && refNode['s-sn'] === nodeToRelocate['s-sn'] && parentNodeRef === refNode.parentNode) {
refNode = refNode.nextSibling as any;
refNode = refNode.nextSibling as d.RenderNode | null;

// If the refNode is the same node to be relocated or another element's slot reference, keep searching to find the
// correct relocation target
while (refNode === nodeToRelocate || refNode?.['s-sr']) {
refNode = refNode?.nextSibling as d.RenderNode | null;
}

if (!refNode || !refNode['s-nr']) {
insertBeforeNode = refNode;
break;
Expand Down
22 changes: 22 additions & 0 deletions test/wdio/slot-nested-dynamic/cmp-child.tsx
@@ -0,0 +1,22 @@
import { Component, h, Host } from '@stencil/core';

@Component({
tag: 'slot-nested-dynamic-child',
shadow: false,
scoped: true,
})
export class SlotNestedDynamicChild {
render() {
return (
<Host>
<slot-nested-dynamic-wrapper id="banner">Banner notification</slot-nested-dynamic-wrapper>
<slot name="header" />
<slot-nested-dynamic-wrapper id="level-1">
<slot-nested-dynamic-wrapper id="level-2">
<slot />
</slot-nested-dynamic-wrapper>
</slot-nested-dynamic-wrapper>
</Host>
);
}
}
19 changes: 19 additions & 0 deletions test/wdio/slot-nested-dynamic/cmp-parent.tsx
@@ -0,0 +1,19 @@
import { Component, h, Host } from '@stencil/core';

@Component({
tag: 'slot-nested-dynamic-parent',
shadow: false,
scoped: true,
})
export class SlotNestedDynamicParent {
render() {
return (
<Host>
<slot-nested-dynamic-child>
<span slot="header">Header Text</span>
<slot />
</slot-nested-dynamic-child>
</Host>
);
}
}
16 changes: 16 additions & 0 deletions test/wdio/slot-nested-dynamic/cmp-wrapper.tsx
@@ -0,0 +1,16 @@
import { Component, h, Host } from '@stencil/core';

@Component({
tag: 'slot-nested-dynamic-wrapper',
shadow: false,
scoped: true,
})
export class SlotNestedDynamicWrapper {
render() {
return (
<Host>
<slot />
</Host>
);
}
}
65 changes: 65 additions & 0 deletions test/wdio/slot-nested-dynamic/cmp.test.tsx
@@ -0,0 +1,65 @@
import { Fragment, h } from '@stencil/core';
import { render } from '@wdio/browser-runner/stencil';

const ITEM_CLASSNAME = 'default-slot-item';

describe('slot-nested-dynamic', function () {
const getRandomizeButton = () => $('#randomizeButton');
const getItems = () => $$(`.${ITEM_CLASSNAME}`);

beforeEach(() => {
render({
flushQueue: true,
template: () => {
function randomize() {
const parent = document.querySelector('slot-nested-dynamic-parent');

for (const item of Array.from(parent?.querySelectorAll('.default-slot-item') || [])) {
item.remove();
}

const min = 3;
const max = 20;
const count = Math.floor(Math.random() * (max - min + 1)) + min;
const items = Array.from(new Array(count)).map((_, i) => {
const element = document.createElement('span');
element.className = 'default-slot-item';
element.textContent = 'item-' + i;
return element;
});

for (const item of items) {
parent?.appendChild(item);
}
}
return (
<>
<slot-nested-dynamic-parent></slot-nested-dynamic-parent>

<button id="randomizeButton" onClick={() => randomize()}>
Randomize Item Count
</button>
</>
);
},
});
});

it('correct order after dynamic list generation', async () => {
const button = getRandomizeButton();

/* RUN DYNAMIC GENERATION */
await button.click();
const items = getItems();
await expect(await items[0].getText()).toBe('item-0');
await expect(await items[1].getText()).toBe('item-1');
await expect(await items[2].getText()).toBe('item-2');

/* RE-RUN DYNAMIC GENERATION */
await button.click();
const itemsRoundTwo = getItems();
await expect(await itemsRoundTwo[0].getText()).toBe('item-0');
await expect(await itemsRoundTwo[1].getText()).toBe('item-1');
await expect(await itemsRoundTwo[2].getText()).toBe('item-2');
});
});

0 comments on commit 363c07b

Please sign in to comment.