Skip to content

Commit

Permalink
fix(runtime): patch removeChild for scoped components (#5148)
Browse files Browse the repository at this point in the history
* fix(runtime): add patch for `removeChild` on `scoped` components

This commit adds a patch for an HtmlElement's `removeChild` method

Fixes: #3278 #2259

STENCIL-937

Co-authored-by: johnjenkins <johnljenkins@hotmail.com>

* add e2e tests

* PR feedback

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

---------

Co-authored-by: johnjenkins <johnljenkins@hotmail.com>
Co-authored-by: Ryan Waskiewicz <ryanwaskiewicz@gmail.com>
  • Loading branch information
3 people committed Dec 12, 2023
1 parent c6b723f commit 956c196
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 2 deletions.
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);
patchChildSlotNodes(hostElementPrototype, descriptorPrototype);
patchSlotRemoveChild(hostElementPrototype);
};

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 slot exists. Otherwise, it falls-back to the original `removeChild` method.
*
* @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) => {
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 @@ -1042,6 +1044,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 @@ -1551,6 +1559,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 @@ -1862,6 +1871,8 @@ declare namespace LocalJSX {
"str"?: string;
"undef"?: string;
}
interface RemoveChildPatch {
}
interface ReparentStyleNoVars {
}
interface ReparentStyleWithVars {
Expand Down Expand Up @@ -2112,6 +2123,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 @@ -2276,6 +2288,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();
});
});

0 comments on commit 956c196

Please sign in to comment.