Skip to content

Commit

Permalink
fix(runtime): dynamic slot name change (#5304)
Browse files Browse the repository at this point in the history
* fix for shadow components

Fixes an issue where a slot's `name` attribute would not update correctly if it was bound to a `@Prop()` (for Shadow DOM components)

STENCIL-295

* fix for scoped components

Fixes an issue where a slot's `name` attribute would not update correctly if it was bound to a `@Prop()` (for non-Shadow DOM components)

STENCIL-295

* add e2e tests

* little comment on change

* put scoped changes behind slot fix flag
  • Loading branch information
tanner-reits committed Jan 29, 2024
1 parent 97138a4 commit 9d9fe41
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 5 deletions.
20 changes: 15 additions & 5 deletions src/runtime/vdom/vdom-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,16 @@ const relocateToHostRoot = (parentElm: Element) => {

const host = parentElm.closest(hostTagName.toLowerCase());
if (host != null) {
for (const childNode of Array.from(parentElm.childNodes) as d.RenderNode[]) {
const contentRefNode = (Array.from(host.childNodes) as d.RenderNode[]).find((ref) => ref['s-cr']);
const childNodeArray = Array.from(parentElm.childNodes) as d.RenderNode[];

// If we have a content ref, we need to invert the order of the nodes we're relocating
// to preserve the correct order of elements in the DOM on future relocations
for (const childNode of contentRefNode ? childNodeArray.reverse() : childNodeArray) {
// Only relocate nodes that were slotted in
if (childNode['s-sh'] != null) {
host.insertBefore(childNode, null);
host.insertBefore(childNode, contentRefNode ?? null);

// Reset so we can correctly move the node around again.
childNode['s-sh'] = undefined;

Expand Down Expand Up @@ -638,8 +644,11 @@ export const patch = (oldVNode: d.VNode, newVNode: d.VNode, isInitialRender = fa
}

if (BUILD.vdomAttribute || BUILD.reflect) {
if (BUILD.slot && tag === 'slot') {
// minifier will clean this up
if (BUILD.slot && tag === 'slot' && !useNativeShadowDom) {
if (BUILD.experimentalSlotFixes && oldVNode.$name$ !== newVNode.$name$) {
newVNode.$elm$['s-sn'] = newVNode.$name$ || '';
relocateToHostRoot(newVNode.$elm$.parentElement);
}
} else {
// either this is the first render of an element OR it's an update
// AND we already know it's possible it could have changed
Expand Down Expand Up @@ -978,9 +987,10 @@ render() {
if (BUILD.scoped || BUILD.shadowDom) {
scopeId = hostElm['s-sc'];
}

useNativeShadowDom = supportsShadow && (cmpMeta.$flags$ & CMP_FLAGS.shadowDomEncapsulation) !== 0;
if (BUILD.slotRelocation) {
contentRef = hostElm['s-cr'];
useNativeShadowDom = supportsShadow && (cmpMeta.$flags$ & CMP_FLAGS.shadowDomEncapsulation) !== 0;

// always reset
checkSlotFallbackVisibility = false;
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 @@ -301,6 +301,12 @@ export namespace Components {
}
interface SlotChildrenRoot {
}
interface SlotDynamicNameChangeScoped {
"slotName": string;
}
interface SlotDynamicNameChangeShadow {
"slotName": string;
}
interface SlotDynamicScopedList {
"items": Array<string>;
}
Expand Down Expand Up @@ -1205,6 +1211,18 @@ declare global {
prototype: HTMLSlotChildrenRootElement;
new (): HTMLSlotChildrenRootElement;
};
interface HTMLSlotDynamicNameChangeScopedElement extends Components.SlotDynamicNameChangeScoped, HTMLStencilElement {
}
var HTMLSlotDynamicNameChangeScopedElement: {
prototype: HTMLSlotDynamicNameChangeScopedElement;
new (): HTMLSlotDynamicNameChangeScopedElement;
};
interface HTMLSlotDynamicNameChangeShadowElement extends Components.SlotDynamicNameChangeShadow, HTMLStencilElement {
}
var HTMLSlotDynamicNameChangeShadowElement: {
prototype: HTMLSlotDynamicNameChangeShadowElement;
new (): HTMLSlotDynamicNameChangeShadowElement;
};
interface HTMLSlotDynamicScopedListElement extends Components.SlotDynamicScopedList, HTMLStencilElement {
}
var HTMLSlotDynamicScopedListElement: {
Expand Down Expand Up @@ -1577,6 +1595,8 @@ declare global {
"slot-basic-order-root": HTMLSlotBasicOrderRootElement;
"slot-basic-root": HTMLSlotBasicRootElement;
"slot-children-root": HTMLSlotChildrenRootElement;
"slot-dynamic-name-change-scoped": HTMLSlotDynamicNameChangeScopedElement;
"slot-dynamic-name-change-shadow": HTMLSlotDynamicNameChangeShadowElement;
"slot-dynamic-scoped-list": HTMLSlotDynamicScopedListElement;
"slot-dynamic-shadow-list": HTMLSlotDynamicShadowListElement;
"slot-dynamic-wrapper": HTMLSlotDynamicWrapperElement;
Expand Down Expand Up @@ -1920,6 +1940,12 @@ declare namespace LocalJSX {
}
interface SlotChildrenRoot {
}
interface SlotDynamicNameChangeScoped {
"slotName"?: string;
}
interface SlotDynamicNameChangeShadow {
"slotName"?: string;
}
interface SlotDynamicScopedList {
"items"?: Array<string>;
}
Expand Down Expand Up @@ -2134,6 +2160,8 @@ declare namespace LocalJSX {
"slot-basic-order-root": SlotBasicOrderRoot;
"slot-basic-root": SlotBasicRoot;
"slot-children-root": SlotChildrenRoot;
"slot-dynamic-name-change-scoped": SlotDynamicNameChangeScoped;
"slot-dynamic-name-change-shadow": SlotDynamicNameChangeShadow;
"slot-dynamic-scoped-list": SlotDynamicScopedList;
"slot-dynamic-shadow-list": SlotDynamicShadowList;
"slot-dynamic-wrapper": SlotDynamicWrapper;
Expand Down Expand Up @@ -2296,6 +2324,8 @@ declare module "@stencil/core" {
"slot-basic-order-root": LocalJSX.SlotBasicOrderRoot & JSXBase.HTMLAttributes<HTMLSlotBasicOrderRootElement>;
"slot-basic-root": LocalJSX.SlotBasicRoot & JSXBase.HTMLAttributes<HTMLSlotBasicRootElement>;
"slot-children-root": LocalJSX.SlotChildrenRoot & JSXBase.HTMLAttributes<HTMLSlotChildrenRootElement>;
"slot-dynamic-name-change-scoped": LocalJSX.SlotDynamicNameChangeScoped & JSXBase.HTMLAttributes<HTMLSlotDynamicNameChangeScopedElement>;
"slot-dynamic-name-change-shadow": LocalJSX.SlotDynamicNameChangeShadow & JSXBase.HTMLAttributes<HTMLSlotDynamicNameChangeShadowElement>;
"slot-dynamic-scoped-list": LocalJSX.SlotDynamicScopedList & JSXBase.HTMLAttributes<HTMLSlotDynamicScopedListElement>;
"slot-dynamic-shadow-list": LocalJSX.SlotDynamicShadowList & JSXBase.HTMLAttributes<HTMLSlotDynamicShadowListElement>;
"slot-dynamic-wrapper": LocalJSX.SlotDynamicWrapper & JSXBase.HTMLAttributes<HTMLSlotDynamicWrapperElement>;
Expand Down
17 changes: 17 additions & 0 deletions test/karma/test-app/slot-dynamic-name-change/cmp-scoped.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Component, h, Prop } from '@stencil/core';

@Component({
tag: 'slot-dynamic-name-change-scoped',
scoped: true,
})
export class SlotDynamicNameChangeScoped {
@Prop() slotName = 'greeting';

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

@Component({
tag: 'slot-dynamic-name-change-shadow',
shadow: true,
})
export class SlotDynamicNameChangeShadow {
@Prop() slotName = 'greeting';

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

<slot-dynamic-name-change-shadow>
<p slot="greeting">Hello</p>
<p slot="farewell">Goodbye</p>
</slot-dynamic-name-change-shadow>
<slot-dynamic-name-change-scoped>
<p slot="greeting">Hello</p>
<p slot="farewell">Goodbye</p>
</slot-dynamic-name-change-scoped>

<button>Toggle slot name</button>

<script>
document.querySelector('button').addEventListener('click', () => {
document.querySelector('slot-dynamic-name-change-shadow').setAttribute('slot-name', 'farewell');
document.querySelector('slot-dynamic-name-change-scoped').setAttribute('slot-name', 'farewell');
});
</script>
40 changes: 40 additions & 0 deletions test/karma/test-app/slot-dynamic-name-change/karma.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { setupDomTests, waitForChanges } from '../util';

/**
* Tests the case where a `slot` element in a component has its
* `name` attribute changed dynamically via a property.
*/
describe('slot dynamic name change', () => {
const { setupDom, tearDownDom } = setupDomTests(document);
let app: HTMLElement;

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

afterEach(tearDownDom);

it('should change the slot name for a shadow component', async () => {
const cmp = app.querySelector('slot-dynamic-name-change-shadow');
expect(cmp.innerText).toBe('Hello');
expect(cmp.shadowRoot.querySelector('slot').getAttribute('name')).toBe('greeting');

app.querySelector('button').click();
await waitForChanges();

expect(cmp.innerText).toBe('Goodbye');
expect(cmp.shadowRoot.querySelector('slot').getAttribute('name')).toBe('farewell');
});

it('should change the slot name for a scoped component', async () => {
const cmp = app.querySelector('slot-dynamic-name-change-scoped');
expect(cmp.innerText).toBe('Hello');
expect(cmp.querySelector('p:not([hidden])').getAttribute('slot')).toBe('greeting');

app.querySelector('button').click();
await waitForChanges();

expect(cmp.innerText).toBe('Goodbye');
expect(cmp.querySelector('p:not([hidden])').getAttribute('slot')).toBe('farewell');
});
});

0 comments on commit 9d9fe41

Please sign in to comment.