Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(runtime): patch removeChild for scoped components #5148

Merged
merged 3 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion src/runtime/dom-extras.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { CMP_FLAGS, HOST_FLAGS } from '@utils';

import type * as d from '../declarations';
import { PLATFORM_FLAGS } from './runtime-constants';
import { updateFallbackSlotVisibility } from './vdom/vdom-render';

export const patchPseudoShadowDom = (
hostElementPrototype: HTMLElement,
Expand All @@ -19,6 +20,7 @@ export const patchPseudoShadowDom = (
patchSlotInsertAdjacentText(hostElementPrototype);
patchTextContent(hostElementPrototype, descriptorPrototype);
patchChildSlotNodes(hostElementPrototype, descriptorPrototype);
patchSlotRemoveChild(hostElementPrototype);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanna call out that this will only be patched for scoped components. The associated issues had some examples using no encapsulation, but we're moving away from patching this behavior for that case.

};

export const patchCloneNode = (HostElementPrototype: HTMLElement) => {
Expand Down Expand Up @@ -66,6 +68,14 @@ export const patchCloneNode = (HostElementPrototype: HTMLElement) => {
};
};

/**
* Patches the `appendChild` method on a `scoped` Stencil component.
* The patch will attempt to find a slot with the same name as the node being appended
* and insert it into the slot reference if found. Otherwise, it falls-back to the original
* `appendChild` method.
*
* @param HostElementPrototype The Stencil component to be patched
*/
export const patchSlotAppendChild = (HostElementPrototype: any) => {
HostElementPrototype.__appendChild = HostElementPrototype.appendChild;
HostElementPrototype.appendChild = function (this: d.RenderNode, newChild: d.RenderNode) {
Expand All @@ -74,12 +84,46 @@ export const patchSlotAppendChild = (HostElementPrototype: any) => {
if (slotNode) {
const slotChildNodes = getHostSlotChildNodes(slotNode, slotName);
const appendAfter = slotChildNodes[slotChildNodes.length - 1];
return appendAfter.parentNode.insertBefore(newChild, appendAfter.nextSibling);
appendAfter.parentNode.insertBefore(newChild, appendAfter.nextSibling);
// Check if there is fallback content that should be hidden
updateFallbackSlotVisibility(this);
return;
}
return (this as any).__appendChild(newChild);
};
};

/**
* Patches the `removeChild` method on a `scoped` Stencil component.
* This patch attempts to remove the specified node from a slot reference
* if the the slot exists. Otherwise, it falls-back to the original `removeChild` method.
tanner-reits marked this conversation as resolved.
Show resolved Hide resolved
*
* @param ElementPrototype The Stencil component to be patched
*/
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']);
if (slotNode) {
// Get all slot content
const slotChildNodes = getHostSlotChildNodes(slotNode, toRemove['s-sn']);
// See if any of the slotted content matches the node to remove
const existingNode = slotChildNodes.find((n) => n === toRemove);

if (existingNode) {
existingNode.remove();
// Check if there is fallback content that should be displayed if that
// was the last node in the slot
updateFallbackSlotVisibility(this);
return;
}
}
}
return (this as any).__removeChild(toRemove);
};
};

/**
* Patches the `prepend` method for a slotted node inside a scoped component.
*
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/vdom/vdom-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ export const patch = (oldVNode: d.VNode, newVNode: d.VNode) => {
*
* @param elm the element of interest
*/
const updateFallbackSlotVisibility = (elm: d.RenderNode) => {
export const updateFallbackSlotVisibility = (elm: d.RenderNode) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to export this to use in the patches in dom-extras. Can move this somewhere else if we want

const childNodes: d.RenderNode[] = elm.childNodes as any;

for (const childNode of childNodes) {
Expand Down
13 changes: 13 additions & 0 deletions test/karma/test-app/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,8 @@ export namespace Components {
"str": string;
"undef"?: string;
}
interface RemoveChildPatch {
}
interface ReparentStyleNoVars {
}
interface ReparentStyleWithVars {
Expand Down Expand Up @@ -1038,6 +1040,12 @@ declare global {
prototype: HTMLReflectToAttrElement;
new (): HTMLReflectToAttrElement;
};
interface HTMLRemoveChildPatchElement extends Components.RemoveChildPatch, HTMLStencilElement {
}
var HTMLRemoveChildPatchElement: {
prototype: HTMLRemoveChildPatchElement;
new (): HTMLRemoveChildPatchElement;
};
interface HTMLReparentStyleNoVarsElement extends Components.ReparentStyleNoVars, HTMLStencilElement {
}
var HTMLReparentStyleNoVarsElement: {
Expand Down Expand Up @@ -1535,6 +1543,7 @@ declare global {
"reflect-nan-attribute": HTMLReflectNanAttributeElement;
"reflect-nan-attribute-hyphen": HTMLReflectNanAttributeHyphenElement;
"reflect-to-attr": HTMLReflectToAttrElement;
"remove-child-patch": HTMLRemoveChildPatchElement;
"reparent-style-no-vars": HTMLReparentStyleNoVarsElement;
"reparent-style-with-vars": HTMLReparentStyleWithVarsElement;
"sass-cmp": HTMLSassCmpElement;
Expand Down Expand Up @@ -1844,6 +1853,8 @@ declare namespace LocalJSX {
"str"?: string;
"undef"?: string;
}
interface RemoveChildPatch {
}
interface ReparentStyleNoVars {
}
interface ReparentStyleWithVars {
Expand Down Expand Up @@ -2090,6 +2101,7 @@ declare namespace LocalJSX {
"reflect-nan-attribute": ReflectNanAttribute;
"reflect-nan-attribute-hyphen": ReflectNanAttributeHyphen;
"reflect-to-attr": ReflectToAttr;
"remove-child-patch": RemoveChildPatch;
"reparent-style-no-vars": ReparentStyleNoVars;
"reparent-style-with-vars": ReparentStyleWithVars;
"sass-cmp": SassCmp;
Expand Down Expand Up @@ -2252,6 +2264,7 @@ declare module "@stencil/core" {
"reflect-nan-attribute": LocalJSX.ReflectNanAttribute & JSXBase.HTMLAttributes<HTMLReflectNanAttributeElement>;
"reflect-nan-attribute-hyphen": LocalJSX.ReflectNanAttributeHyphen & JSXBase.HTMLAttributes<HTMLReflectNanAttributeHyphenElement>;
"reflect-to-attr": LocalJSX.ReflectToAttr & JSXBase.HTMLAttributes<HTMLReflectToAttrElement>;
"remove-child-patch": LocalJSX.RemoveChildPatch & JSXBase.HTMLAttributes<HTMLRemoveChildPatchElement>;
"reparent-style-no-vars": LocalJSX.ReparentStyleNoVars & JSXBase.HTMLAttributes<HTMLReparentStyleNoVarsElement>;
"reparent-style-with-vars": LocalJSX.ReparentStyleWithVars & JSXBase.HTMLAttributes<HTMLReparentStyleWithVarsElement>;
"sass-cmp": LocalJSX.SassCmp & JSXBase.HTMLAttributes<HTMLSassCmpElement>;
Expand Down
18 changes: 18 additions & 0 deletions test/karma/test-app/remove-child-patch/cmp.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Component, h } from '@stencil/core';

@Component({
tag: 'remove-child-patch',
scoped: true,
})
export class RemoveChildPatch {
render() {
return (
<div>
<p>I'm not in a slot</p>
<div class="slot-container">
<slot>Slot fallback content</slot>
</div>
</div>
);
}
}
27 changes: 27 additions & 0 deletions test/karma/test-app/remove-child-patch/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!doctype html>
<meta charset="utf8" />
<script src="./build/testapp.esm.js" type="module"></script>
<script src="./build/testapp.js" nomodule></script>

<remove-child-patch>
<span>Slotted 1</span>
<span>Slotted 2</span>
</remove-child-patch>

<button id="remove-child-button" type="button">Remove Last Child</button>
<button id="remove-child-div-button" type="button">Remove Child Div</button>

<script>
document.querySelector('#remove-child-button').addEventListener('click', () => {
const el = document.querySelector('remove-child-patch');
const slotContainer = el.querySelector('.slot-container');
const elementToRemove = slotContainer.children[slotContainer.children.length - 1];
el.removeChild(elementToRemove);
});

document.querySelector('#remove-child-div-button').addEventListener('click', () => {
const el = document.querySelector('remove-child-patch');
const elementToRemove = el.querySelector('div');
el.removeChild(elementToRemove);
});
</script>
64 changes: 64 additions & 0 deletions test/karma/test-app/remove-child-patch/karma.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { setupDomTests, waitForChanges } from '../util';

/**
* Tests for the patched `removeChild()` method on `scoped` components.
*/
describe('remove-child-patch', () => {
const { setupDom, tearDownDom } = setupDomTests(document);

let app: HTMLElement | undefined;
let host: HTMLElement | undefined;

beforeEach(async () => {
app = await setupDom('/remove-child-patch/index.html');
host = app.querySelector('remove-child-patch');
});

afterEach(tearDownDom);

it('should remove the last slotted node', async () => {
expect(host).toBeDefined();

const slotContainer = host.querySelector('.slot-container');
expect(slotContainer).toBeDefined();
const slottedSpans = slotContainer.querySelectorAll('span');
expect(slottedSpans.length).toBe(2);

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

const slottedSpansAfter = slotContainer.querySelectorAll('span');
expect(slottedSpansAfter.length).toBe(1);
});

it('should show slot-fb if the last slotted node is removed', async () => {
expect(host).toBeDefined();

const slotContainer = host.querySelector('.slot-container');
expect(slotContainer).toBeDefined();
const slottedSpans = slotContainer.querySelectorAll('span');
expect(slottedSpans.length).toBe(2);

const button = document.querySelector<HTMLButtonElement>('#remove-child-button');
button.click();
await waitForChanges();
button.click();
await waitForChanges();

const slottedSpansAfter = slotContainer.querySelectorAll('span');
expect(slottedSpansAfter.length).toBe(0);
expect(slotContainer.textContent.trim()).toBe('Slot fallback content');
});

it('should still be able to remove nodes not slotted', async () => {
expect(host).toBeDefined();

expect(host.querySelector('div')).toBeDefined();

const button = document.querySelector<HTMLButtonElement>('#remove-child-div-button');
button.click();
await waitForChanges();

expect(host.querySelector('div')).toBeNull();
});
});