fix(runtime): prevent additional attempted move of slot content (#4921)

* yo this might actually work

* create single build flag for `experimentalSlotFixes`

* gate new vDOM logic behind build flag

* add e2e test for repro case

* document new node property

* PR feedback

Author: tanner-reits

src/app-data/index.ts

@@ -109,7 +109,7 @@ export const BUILD: BuildConditionals = {
   transformTagName: false,
   attachStyles: true,
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  patchPseudoShadowDom: false,
+  experimentalSlotFixes: false,
 };
 
 export const Env = {};

src/compiler/app-core/app-data.ts

@@ -168,7 +168,7 @@ export const updateBuildConditionals = (config: ValidatedConfig, b: BuildConditi
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
   b.slotChildNodesFix = config.extras.slotChildNodesFix;
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  b.patchPseudoShadowDom = config.extras.experimentalSlotFixes;
+  b.experimentalSlotFixes = config.extras.experimentalSlotFixes;
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
   b.cloneNodeFix = config.extras.cloneNodeFix;
   b.lifecycleDOMEvents = !!(b.isDebug || config._isTesting || config.extras.lifecycleDOMEvents);

src/compiler/output-targets/dist-hydrate-script/hydrate-build-conditionals.ts

@@ -24,7 +24,7 @@ export const getHydrateBuildConditionals = (cmps: d.ComponentCompilerMeta[]) =>
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
   build.slotChildNodesFix = false;
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  build.patchPseudoShadowDom = false;
+  build.experimentalSlotFixes = false;
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
   build.cloneNodeFix = false;
   build.cssAnnotations = true;

src/declarations/stencil-private.ts

@@ -186,7 +186,7 @@ export interface BuildConditionals extends Partial<BuildFeatures> {
   attachStyles?: boolean;
 
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  patchPseudoShadowDom?: boolean;
+  experimentalSlotFixes?: boolean;
 }
 
 export type ModuleFormat =
@@ -1364,6 +1364,19 @@ export interface RenderNode extends HostElement {
    */
   ['s-hn']?: string;
 
+  /**
+   * Slot host tag name:
+   * This is the tag name of the element where this node
+   * has been moved to during slot relocation.
+   *
+   * This allows us to check if the node has been moved and prevent
+   * us from thinking a node _should_ be moved when it may already be in
+   * its final destination.
+   *
+   * This value is set to `undefined` whenever the node is put back into its original location.
+   */
+  ['s-sh']?: string;
+
   /**
    * Original Location Reference:
    * A reference pointing to the comment

src/runtime/bootstrap-custom-element.ts

@@ -45,7 +45,7 @@ export const proxyCustomElement = (Cstr: any, compactMeta: d.ComponentRuntimeMet
 
   // TODO(STENCIL-914): this check and `else` block can go away and be replaced by just `BUILD.scoped` once we
   // default our pseudo-slot behavior
-  if (BUILD.patchPseudoShadowDom && BUILD.scoped) {
+  if (BUILD.experimentalSlotFixes && BUILD.scoped) {
     patchPseudoShadowDom(Cstr.prototype, cmpMeta);
   } else {
     if (BUILD.slotChildNodesFix) {

src/runtime/bootstrap-lazy.ts

@@ -140,7 +140,7 @@ export const bootstrapLazy = (lazyBundles: d.LazyBundlesRuntimeData, options: d.
 
       // TODO(STENCIL-914): this check and `else` block can go away and be replaced by just `BUILD.scoped` once we
       // default our pseudo-slot behavior
-      if (BUILD.patchPseudoShadowDom && BUILD.scoped) {
+      if (BUILD.experimentalSlotFixes && BUILD.scoped) {
         patchPseudoShadowDom(HostElement.prototype, cmpMeta);
       } else {
         if (BUILD.slotChildNodesFix) {

src/runtime/vdom/vdom-render.ts

@@ -181,6 +181,9 @@ const putBackInOriginalLocation = (parentElm: Node, recursive: boolean) => {
       childNode['s-ol'].remove();
       childNode['s-ol'] = undefined;
 
+      // Reset so we can correctly move the node around again.
+      childNode['s-sh'] = undefined;
+
       checkSlotRelocate = true;
     }
 
@@ -723,8 +726,17 @@ const markSlotContentForRelocation = (elm: d.RenderNode) => {
 
         // check that the node is not a content reference node or a node
         // reference and then check that the host name does not match that of
-        // childNode
-        if (!node['s-cn'] && !node['s-nr'] && node['s-hn'] !== childNode['s-hn']) {
+        // childNode.
+        // In addition, check that the slot either has not already been relocated, or
+        // that its current location's host is not childNode's host. This is essentially
+        // a check so that we don't try to relocate (and then hide) a node that is already
+        // where it should be.
+        if (
+          !node['s-cn'] &&
+          !node['s-nr'] &&
+          node['s-hn'] !== childNode['s-hn'] &&
+          (!BUILD.experimentalSlotFixes || !node['s-sh'] || node['s-sh'] !== childNode['s-hn'])
+        ) {
           // if `node` is located in the slot that `childNode` refers to (via the
           // `'s-sn'` property) then we need to relocate it from it's current spot
           // (under the host element parent) to the right slot location
@@ -740,11 +752,13 @@ const markSlotContentForRelocation = (elm: d.RenderNode) => {
             node['s-sn'] = node['s-sn'] || slotName;
 
             if (relocateNodeData) {
+              relocateNodeData.$nodeToRelocate$['s-sh'] = childNode['s-hn'];
               // we marked this node for relocation previously but didn't find
               // out the slot reference node to which it needs to be relocated
               // so write it down now!
               relocateNodeData.$slotRefNode$ = childNode;
             } else {
+              node['s-sh'] = childNode['s-hn'];
               // add to our list of nodes to relocate
               relocateNodes.push({
                 $slotRefNode$: childNode,

src/testing/reset-build-conditionals.ts

@@ -56,5 +56,5 @@ export function resetBuildConditionals(b: d.BuildConditionals) {
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
   b.slotChildNodesFix = false;
   // TODO(STENCIL-914): remove this option when `experimentalSlotFixes` is the default behavior
-  b.patchPseudoShadowDom = false;
+  b.experimentalSlotFixes = false;
 }

test/karma/test-app/components.d.ts

@@ -150,6 +150,12 @@ export namespace Components {
     interface InputBasicRoot {
         "value"?: string;
     }
+    interface IonChild {
+    }
+    interface IonHost {
+    }
+    interface IonParent {
+    }
     interface JsonBasic {
     }
     interface KeyReorder {
@@ -730,6 +736,24 @@ declare global {
         prototype: HTMLInputBasicRootElement;
         new (): HTMLInputBasicRootElement;
     };
+    interface HTMLIonChildElement extends Components.IonChild, HTMLStencilElement {
+    }
+    var HTMLIonChildElement: {
+        prototype: HTMLIonChildElement;
+        new (): HTMLIonChildElement;
+    };
+    interface HTMLIonHostElement extends Components.IonHost, HTMLStencilElement {
+    }
+    var HTMLIonHostElement: {
+        prototype: HTMLIonHostElement;
+        new (): HTMLIonHostElement;
+    };
+    interface HTMLIonParentElement extends Components.IonParent, HTMLStencilElement {
+    }
+    var HTMLIonParentElement: {
+        prototype: HTMLIonParentElement;
+        new (): HTMLIonParentElement;
+    };
     interface HTMLJsonBasicElement extends Components.JsonBasic, HTMLStencilElement {
     }
     var HTMLJsonBasicElement: {
@@ -1364,6 +1388,9 @@ declare global {
         "image-import": HTMLImageImportElement;
         "init-css-root": HTMLInitCssRootElement;
         "input-basic-root": HTMLInputBasicRootElement;
+        "ion-child": HTMLIonChildElement;
+        "ion-host": HTMLIonHostElement;
+        "ion-parent": HTMLIonParentElement;
         "json-basic": HTMLJsonBasicElement;
         "key-reorder": HTMLKeyReorderElement;
         "key-reorder-root": HTMLKeyReorderRootElement;
@@ -1594,6 +1621,12 @@ declare namespace LocalJSX {
     interface InputBasicRoot {
         "value"?: string;
     }
+    interface IonChild {
+    }
+    interface IonHost {
+    }
+    interface IonParent {
+    }
     interface JsonBasic {
     }
     interface KeyReorder {
@@ -1865,6 +1898,9 @@ declare namespace LocalJSX {
         "image-import": ImageImport;
         "init-css-root": InitCssRoot;
         "input-basic-root": InputBasicRoot;
+        "ion-child": IonChild;
+        "ion-host": IonHost;
+        "ion-parent": IonParent;
         "json-basic": JsonBasic;
         "key-reorder": KeyReorder;
         "key-reorder-root": KeyReorderRoot;
@@ -2011,6 +2047,9 @@ declare module "@stencil/core" {
             "image-import": LocalJSX.ImageImport & JSXBase.HTMLAttributes<HTMLImageImportElement>;
             "init-css-root": LocalJSX.InitCssRoot & JSXBase.HTMLAttributes<HTMLInitCssRootElement>;
             "input-basic-root": LocalJSX.InputBasicRoot & JSXBase.HTMLAttributes<HTMLInputBasicRootElement>;
+            "ion-child": LocalJSX.IonChild & JSXBase.HTMLAttributes<HTMLIonChildElement>;
+            "ion-host": LocalJSX.IonHost & JSXBase.HTMLAttributes<HTMLIonHostElement>;
+            "ion-parent": LocalJSX.IonParent & JSXBase.HTMLAttributes<HTMLIonParentElement>;
             "json-basic": LocalJSX.JsonBasic & JSXBase.HTMLAttributes<HTMLJsonBasicElement>;
             "key-reorder": LocalJSX.KeyReorder & JSXBase.HTMLAttributes<HTMLKeyReorderElement>;
             "key-reorder-root": LocalJSX.KeyReorderRoot & JSXBase.HTMLAttributes<HTMLKeyReorderRootElement>;

test/karma/test-app/scoped-slot-in-slot/child.tsx

@@ -0,0 +1,15 @@
+import { Component, h } from '@stencil/core';
+
+@Component({
+  tag: 'ion-child',
+  scoped: true,
+})
+export class Child {
+  render() {
+    return (
+      <div style={{ display: 'flex', gap: '13px' }}>
+        <slot name="suffix" />
+      </div>
+    );
+  }
+}

test/karma/test-app/scoped-slot-in-slot/host.tsx

@@ -0,0 +1,19 @@
+import { Component, h } from '@stencil/core';
+
+@Component({
+  tag: 'ion-host',
+  scoped: true,
+})
+export class Host {
+  render() {
+    return (
+      <div>
+        <ion-parent>
+          <slot name="label" slot="label" />
+          <slot name="suffix" slot="suffix" />
+          <slot name="message" slot="message" />
+        </ion-parent>
+      </div>
+    );
+  }
+}

test/karma/test-app/scoped-slot-in-slot/index.html

@@ -0,0 +1,10 @@
+<!doctype html>
+<meta charset="utf8" />
+<script src="./build/testapp.esm.js" type="module"></script>
+<script src="./build/testapp.js" nomodule></script>
+
+<ion-host>
+  <span slot="label">Label text</span>
+  <span slot="suffix">Suffix text</span>
+  <span slot="message">Message text</span>
+</ion-host>

test/karma/test-app/scoped-slot-in-slot/karma.spec.ts

@@ -0,0 +1,42 @@
+import { setupDomTests } from '../util';
+
+describe('scoped-slot-in-slot', () => {
+  const { setupDom, tearDownDom } = setupDomTests(document);
+
+  let app: HTMLElement | undefined;
+  let host: HTMLElement | undefined;
+
+  beforeEach(async () => {
+    app = await setupDom('/scoped-slot-in-slot/index.html');
+    host = app.querySelector('ion-host');
+  });
+
+  afterEach(tearDownDom);
+
+  it('correctly renders content slotted through multiple levels of nested slots', async () => {
+    expect(host).toBeDefined();
+
+    // Check the parent content
+    const parent = host.querySelector('ion-parent');
+    expect(parent).toBeDefined();
+    expect(parent.firstElementChild.tagName).toBe('LABEL');
+
+    // Ensure the label slot content made it through
+    const span = parent.firstElementChild.firstElementChild;
+    expect(span).toBeDefined();
+    expect(span.tagName).toBe('SPAN');
+    expect(span.textContent).toBe('Label text');
+
+    // Ensure the message slot content made it through
+    expect(parent.lastElementChild.tagName).toBe('SPAN');
+    expect(parent.lastElementChild.textContent).toBe('Message text');
+
+    // Check the child content
+    const child = parent.querySelector('ion-child');
+    expect(child).toBeDefined();
+
+    // Ensure the suffix slot content made it through
+    expect(child.firstElementChild.firstElementChild.tagName).toBe('SPAN');
+    expect(child.firstElementChild.firstElementChild.textContent).toBe('Suffix text');
+  });
+});

test/karma/test-app/scoped-slot-in-slot/parent.tsx

@@ -0,0 +1,21 @@
+import { Component, Fragment, h } from '@stencil/core';
+
+@Component({
+  tag: 'ion-parent',
+  scoped: true,
+})
+export class Parent {
+  render() {
+    return (
+      <Fragment>
+        <label>
+          <slot name="label" />
+        </label>
+        <ion-child>
+          <slot name="suffix" slot="suffix" />
+        </ion-child>
+        <slot name="message" />
+      </Fragment>
+    );
+  }
+}