From 956c19651772ce1770598e605b6c50e20b39cefa Mon Sep 17 00:00:00 2001 From: Tanner Reits <47483144+tanner-reits@users.noreply.github.com> Date: Tue, 12 Dec 2023 11:35:13 -0500 Subject: [PATCH] fix(runtime): patch `removeChild` for `scoped` components (#5148) * 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 * add e2e tests * PR feedback Co-authored-by: Ryan Waskiewicz --------- Co-authored-by: johnjenkins Co-authored-by: Ryan Waskiewicz --- src/runtime/dom-extras.ts | 46 ++++++++++++- src/runtime/vdom/vdom-render.ts | 2 +- test/karma/test-app/components.d.ts | 13 ++++ .../karma/test-app/remove-child-patch/cmp.tsx | 18 ++++++ .../test-app/remove-child-patch/index.html | 27 ++++++++ .../test-app/remove-child-patch/karma.spec.ts | 64 +++++++++++++++++++ 6 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 test/karma/test-app/remove-child-patch/cmp.tsx create mode 100644 test/karma/test-app/remove-child-patch/index.html create mode 100644 test/karma/test-app/remove-child-patch/karma.spec.ts diff --git a/src/runtime/dom-extras.ts b/src/runtime/dom-extras.ts index 84f35f1daf6..c3a0e1bd8ef 100644 --- a/src/runtime/dom-extras.ts +++ b/src/runtime/dom-extras.ts @@ -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, @@ -19,6 +20,7 @@ export const patchPseudoShadowDom = ( patchSlotInsertAdjacentText(hostElementPrototype); patchTextContent(hostElementPrototype); patchChildSlotNodes(hostElementPrototype, descriptorPrototype); + patchSlotRemoveChild(hostElementPrototype); }; export const patchCloneNode = (HostElementPrototype: HTMLElement) => { @@ -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) { @@ -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. * diff --git a/src/runtime/vdom/vdom-render.ts b/src/runtime/vdom/vdom-render.ts index 0b08c42089f..9643007de0f 100644 --- a/src/runtime/vdom/vdom-render.ts +++ b/src/runtime/vdom/vdom-render.ts @@ -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) { diff --git a/test/karma/test-app/components.d.ts b/test/karma/test-app/components.d.ts index 22dc7b39c68..198f5c0bf36 100644 --- a/test/karma/test-app/components.d.ts +++ b/test/karma/test-app/components.d.ts @@ -242,6 +242,8 @@ export namespace Components { "str": string; "undef"?: string; } + interface RemoveChildPatch { + } interface ReparentStyleNoVars { } interface ReparentStyleWithVars { @@ -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: { @@ -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; @@ -1862,6 +1871,8 @@ declare namespace LocalJSX { "str"?: string; "undef"?: string; } + interface RemoveChildPatch { + } interface ReparentStyleNoVars { } interface ReparentStyleWithVars { @@ -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; @@ -2276,6 +2288,7 @@ declare module "@stencil/core" { "reflect-nan-attribute": LocalJSX.ReflectNanAttribute & JSXBase.HTMLAttributes; "reflect-nan-attribute-hyphen": LocalJSX.ReflectNanAttributeHyphen & JSXBase.HTMLAttributes; "reflect-to-attr": LocalJSX.ReflectToAttr & JSXBase.HTMLAttributes; + "remove-child-patch": LocalJSX.RemoveChildPatch & JSXBase.HTMLAttributes; "reparent-style-no-vars": LocalJSX.ReparentStyleNoVars & JSXBase.HTMLAttributes; "reparent-style-with-vars": LocalJSX.ReparentStyleWithVars & JSXBase.HTMLAttributes; "sass-cmp": LocalJSX.SassCmp & JSXBase.HTMLAttributes; diff --git a/test/karma/test-app/remove-child-patch/cmp.tsx b/test/karma/test-app/remove-child-patch/cmp.tsx new file mode 100644 index 00000000000..46ae41b2dc3 --- /dev/null +++ b/test/karma/test-app/remove-child-patch/cmp.tsx @@ -0,0 +1,18 @@ +import { Component, h } from '@stencil/core'; + +@Component({ + tag: 'remove-child-patch', + scoped: true, +}) +export class RemoveChildPatch { + render() { + return ( +
+

I'm not in a slot

+
+ Slot fallback content +
+
+ ); + } +} diff --git a/test/karma/test-app/remove-child-patch/index.html b/test/karma/test-app/remove-child-patch/index.html new file mode 100644 index 00000000000..767face3d28 --- /dev/null +++ b/test/karma/test-app/remove-child-patch/index.html @@ -0,0 +1,27 @@ + + + + + + + Slotted 1 + Slotted 2 + + + + + + diff --git a/test/karma/test-app/remove-child-patch/karma.spec.ts b/test/karma/test-app/remove-child-patch/karma.spec.ts new file mode 100644 index 00000000000..7aac1200e5f --- /dev/null +++ b/test/karma/test-app/remove-child-patch/karma.spec.ts @@ -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('#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('#remove-child-div-button'); + button.click(); + await waitForChanges(); + + expect(host.querySelector('div')).toBeNull(); + }); +});