Skip to content

Commit

Permalink
fix(runtime): slot name forwarding & attribute reset (#4993)
Browse files Browse the repository at this point in the history
* always update slot attributes when relocating

* add test case

* darn apostrophe

Co-authored-by: Ryan Waskiewicz <ryanwaskiewicz@gmail.com>

* remove slot ref check

---------

Co-authored-by: Ryan Waskiewicz <ryanwaskiewicz@gmail.com>
  • Loading branch information
tanner-reits and rwaskiewicz committed Oct 27, 2023
1 parent 4959295 commit ee60f3b
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 24 deletions.
51 changes: 31 additions & 20 deletions src/runtime/vdom/vdom-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,13 @@ const putBackInOriginalLocation = (parentElm: Node, recursive: boolean) => {
// 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.setAttribute('slot', childNode['s-sn'] ?? '');
}

checkSlotRelocate = true;
}

Expand Down Expand Up @@ -666,23 +673,29 @@ const updateFallbackSlotVisibility = (elm: d.RenderNode) => {
// we need to check all of its sibling nodes in order to see if
// `childNode` should be hidden
for (const siblingNode of childNodes) {
if (siblingNode['s-hn'] !== childNode['s-hn'] || slotName !== '') {
// this sibling node is from a different component OR is a named
// fallback slot node
if (siblingNode.nodeType === NODE_TYPE.ElementNode && slotName === siblingNode.getAttribute('slot')) {
childNode.hidden = true;
break;
}
} else {
// this is a default fallback slot node
// any element or text node (with content)
// should hide the default fallback slot node
if (
siblingNode.nodeType === NODE_TYPE.ElementNode ||
(siblingNode.nodeType === NODE_TYPE.TextNode && siblingNode.textContent.trim() !== '')
) {
childNode.hidden = true;
break;
// Don't check the node against itself
if (siblingNode !== childNode) {
if (siblingNode['s-hn'] !== childNode['s-hn'] || slotName !== '') {
// this sibling node is from a different component OR is a named
// fallback slot node
if (
siblingNode.nodeType === NODE_TYPE.ElementNode &&
(slotName === siblingNode.getAttribute('slot') || slotName === siblingNode['s-sn'])
) {
childNode.hidden = true;
break;
}
} else {
// this is a default fallback slot node
// any element or text node (with content)
// should hide the default fallback slot node
if (
siblingNode.nodeType === NODE_TYPE.ElementNode ||
(siblingNode.nodeType === NODE_TYPE.TextNode && siblingNode.textContent.trim() !== '')
) {
childNode.hidden = true;
break;
}
}
}
}
Expand Down Expand Up @@ -1009,8 +1022,7 @@ render() {
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
// Handle a use-case where we relocate a slot 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.
Expand All @@ -1021,7 +1033,6 @@ render() {
if (
BUILD.experimentalSlotFixes &&
nodeToRelocate.nodeType === NODE_TYPE.ElementNode &&
slotRefNode.parentElement?.shadowRoot != null &&
slotRefNode['s-fs'] !== nodeToRelocate.getAttribute('slot')
) {
if (!slotRefNode['s-fs']) {
Expand Down
28 changes: 28 additions & 0 deletions test/karma/test-app/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ export namespace Components {
}
interface SlotMapOrderRoot {
}
interface SlotNestedNameChange {
}
interface SlotNestedNameChangeChild {
"state": boolean;
}
interface SlotNestedOrderChild {
}
interface SlotNestedOrderParent {
Expand Down Expand Up @@ -1240,6 +1245,18 @@ declare global {
prototype: HTMLSlotMapOrderRootElement;
new (): HTMLSlotMapOrderRootElement;
};
interface HTMLSlotNestedNameChangeElement extends Components.SlotNestedNameChange, HTMLStencilElement {
}
var HTMLSlotNestedNameChangeElement: {
prototype: HTMLSlotNestedNameChangeElement;
new (): HTMLSlotNestedNameChangeElement;
};
interface HTMLSlotNestedNameChangeChildElement extends Components.SlotNestedNameChangeChild, HTMLStencilElement {
}
var HTMLSlotNestedNameChangeChildElement: {
prototype: HTMLSlotNestedNameChangeChildElement;
new (): HTMLSlotNestedNameChangeChildElement;
};
interface HTMLSlotNestedOrderChildElement extends Components.SlotNestedOrderChild, HTMLStencilElement {
}
var HTMLSlotNestedOrderChildElement: {
Expand Down Expand Up @@ -1487,6 +1504,8 @@ declare global {
"slot-list-light-scoped-root": HTMLSlotListLightScopedRootElement;
"slot-map-order": HTMLSlotMapOrderElement;
"slot-map-order-root": HTMLSlotMapOrderRootElement;
"slot-nested-name-change": HTMLSlotNestedNameChangeElement;
"slot-nested-name-change-child": HTMLSlotNestedNameChangeChildElement;
"slot-nested-order-child": HTMLSlotNestedOrderChildElement;
"slot-nested-order-parent": HTMLSlotNestedOrderParentElement;
"slot-ng-if": HTMLSlotNgIfElement;
Expand Down Expand Up @@ -1837,6 +1856,11 @@ declare namespace LocalJSX {
}
interface SlotMapOrderRoot {
}
interface SlotNestedNameChange {
}
interface SlotNestedNameChangeChild {
"state"?: boolean;
}
interface SlotNestedOrderChild {
}
interface SlotNestedOrderParent {
Expand Down Expand Up @@ -2006,6 +2030,8 @@ declare namespace LocalJSX {
"slot-list-light-scoped-root": SlotListLightScopedRoot;
"slot-map-order": SlotMapOrder;
"slot-map-order-root": SlotMapOrderRoot;
"slot-nested-name-change": SlotNestedNameChange;
"slot-nested-name-change-child": SlotNestedNameChangeChild;
"slot-nested-order-child": SlotNestedOrderChild;
"slot-nested-order-parent": SlotNestedOrderParent;
"slot-ng-if": SlotNgIf;
Expand Down Expand Up @@ -2158,6 +2184,8 @@ declare module "@stencil/core" {
"slot-list-light-scoped-root": LocalJSX.SlotListLightScopedRoot & JSXBase.HTMLAttributes<HTMLSlotListLightScopedRootElement>;
"slot-map-order": LocalJSX.SlotMapOrder & JSXBase.HTMLAttributes<HTMLSlotMapOrderElement>;
"slot-map-order-root": LocalJSX.SlotMapOrderRoot & JSXBase.HTMLAttributes<HTMLSlotMapOrderRootElement>;
"slot-nested-name-change": LocalJSX.SlotNestedNameChange & JSXBase.HTMLAttributes<HTMLSlotNestedNameChangeElement>;
"slot-nested-name-change-child": LocalJSX.SlotNestedNameChangeChild & JSXBase.HTMLAttributes<HTMLSlotNestedNameChangeChildElement>;
"slot-nested-order-child": LocalJSX.SlotNestedOrderChild & JSXBase.HTMLAttributes<HTMLSlotNestedOrderChildElement>;
"slot-nested-order-parent": LocalJSX.SlotNestedOrderParent & JSXBase.HTMLAttributes<HTMLSlotNestedOrderParentElement>;
"slot-ng-if": LocalJSX.SlotNgIf & JSXBase.HTMLAttributes<HTMLSlotNgIfElement>;
Expand Down
8 changes: 4 additions & 4 deletions test/karma/test-app/slot-html/karma.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ describe('slot-html', () => {
expect(results[1].textContent.trim()).toBe('default slot element 2');
expect(results[2].textContent.trim()).toBe('default slot element 3');

results = app.querySelectorAll('.results4 div article span content-start[slot="start"]');
results = app.querySelectorAll('.results4 div article span content-start');
expect(results[0].textContent.trim()).toBe('start slot 1');
expect(results[1].textContent.trim()).toBe('start slot 2');

result = app.querySelector('.results4 div content-default');
expect(result.textContent.trim()).toBe('default slot');

results = app.querySelectorAll('.results5 div article span content-start[slot="start"]');
results = app.querySelectorAll('.results5 div article span content-start');
expect(results[0].textContent.trim()).toBe('start slot 1');
expect(results[1].textContent.trim()).toBe('start slot 2');

Expand All @@ -54,11 +54,11 @@ describe('slot-html', () => {
expect(results[0].textContent.trim()).toBe('default slot 1');
expect(results[1].textContent.trim()).toBe('default slot 2');

results = app.querySelectorAll('.results8 div section content-end[slot="end"]');
results = app.querySelectorAll('.results8 div section content-end');
expect(results[0].textContent.trim()).toBe('end slot 1');
expect(results[1].textContent.trim()).toBe('end slot 2');

results = app.querySelectorAll('.results9 div section content-end[slot="end"]');
results = app.querySelectorAll('.results9 div section content-end');
expect(results[0].textContent.trim()).toBe('end slot 1');
expect(results[1].textContent.trim()).toBe('end slot 2');

Expand Down
17 changes: 17 additions & 0 deletions test/karma/test-app/slot-nested-name-change/cmp-child.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Component, h, Host, Prop } from '@stencil/core';

@Component({
tag: 'slot-nested-name-change-child',
})
export class SlotNestedNameChangeChild {
@Prop() state: boolean;

render() {
return (
<Host>
<div>State: {this.state.toString()}</div>
<slot name="default" />
</Host>
);
}
}
18 changes: 18 additions & 0 deletions test/karma/test-app/slot-nested-name-change/cmp-parent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Component, h, Host } from '@stencil/core';

@Component({
tag: 'slot-nested-name-change',
})
export class SlotNestedNameChange {
render() {
return (
<Host>
<div>
<slot-nested-name-change-child state={true}>
<slot slot="default" />
</slot-nested-name-change-child>
</div>
</Host>
);
}
}
8 changes: 8 additions & 0 deletions test/karma/test-app/slot-nested-name-change/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!doctype html>
<meta charset="utf8" />
<script src="./build/testapp.esm.js" type="module"></script>
<script src="./build/testapp.js" nomodule></script>

<slot-nested-name-change>
<p>Hello</p>
</slot-nested-name-change>
32 changes: 32 additions & 0 deletions test/karma/test-app/slot-nested-name-change/karma.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { setupDomTests } from '../util';

describe('slot-nested-name-change', function () {
const { setupDom, tearDownDom } = setupDomTests(document);
let app: HTMLElement | undefined;
let host: HTMLElement | undefined;

beforeEach(async () => {
app = await setupDom('/slot-nested-name-change/index.html');
host = app.querySelector('slot-nested-name-change');
});

afterEach(tearDownDom);

it('should render', () => {
expect(host).toBeDefined();
});

it('should correctly render the slot content', () => {
const childCmp = host.querySelector('slot-nested-name-change-child');

expect(childCmp.children.length).toBe(2);

const firstChild = childCmp.children[0];
expect(firstChild.tagName).toBe('DIV');
expect(firstChild.textContent.trim()).toBe('State: true');

const secondChild = childCmp.children[1];
expect(secondChild.tagName).toBe('P');
expect(secondChild.textContent.trim()).toBe('Hello');
});
});

0 comments on commit ee60f3b

Please sign in to comment.