ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.5k stars 783 forks source link

Nested slots: Nodes are removed/added from DOM instead of updated #2232

Open simonvizzini opened 4 years ago

simonvizzini commented 4 years ago

Stencil version:

 @stencil/core@1.8.11

I'm submitting a:

[x] bug report [ ] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior: We have a few "design components" which provide nothing more but a default slot and some styling. Nesting more than one such component causes issues when the consuming component has to update due to State/Prop changes. The child nodes inside the wrapper component will get removed from the DOM and then re-added, instead of getting updated. This causes issues especially when trying to use CSS transitions: instead of updating the classList of an element, the whole element gets removed and added, and thus the transition will not get triggered.

This issue does not happen when native shadow DOM is enabled for the components, but only when using the slot polyfill. Unfortunately we still have to support browser that do not support native shadow DOM

Expected behavior: DOM nodes inside nested slots should get updated instead of getting removed completely from the DOM and re-added.

Steps to reproduce:

A full example that reproduces the issues can be found here: https://github.com/simonvizzini/stenciljs-slot-bug-example

It uses a DOM MutationObserver to visualize the issue. The first example is using only a single wrapper component and it works as expected, no DOM mutations are logged and the background transition works fine. The second example uses two nested wrapper components and will log DOM mutations when increasing the counter. Inspecting the logged mutations will reveal that stencil is removing and then adding DOM nodes, thus breaking the CSS transition.

Related code:

// First wrapper component
@Component({
  tag: "my-wrapper-one",
  styleUrl: "wrapper-one.css"
})
export class WrapperOne {
  render() {
    return (
      <Host>
        <slot></slot>
      </Host>
    );
  }
}

// Second wrapper component, which uses 'my-wrapper-one' internally
@Component({
  tag: "my-wrapper-two",
  styleUrl: "wrapper-two.css"
})
export class WrapperTwo {
  render() {
    return (
      <Host>
        <my-wrapper-one>
          <slot></slot>
        </my-wrapper-one>
      </Host>
    );
  }
}

// Using 'my-wrapper-one' is fine, but with 'my-wrapper-two' the transition doesn't work anymore
@Component({
  tag: "my-component",
  styleUrl: "my-component.css"
})
export class MyComponent {
  @State() count: number = 0;
  private increase = () => {
    this.count += 1;
  };

  render() {
    const yellowBg = this.count % 2 === 0;

    return (
      <div>
        <button type="button" onClick={this.increase}>
          Increase
        </button>
        <my-wrapper-two>
          <div class={{ content: true, alternate: yellowBg }}>
            Count: (<span>{this.count}</span>)
          </div>
        </my-wrapper-two>
      </div>
    );
  }
}

Other information:

Paul-Hebert commented 3 years ago

I believe I'm running into this as well. In my case the nested slot is an iframe. Removing and adding the iframe causes the entire iframe to reload, which leads to issues when the iframe contains dynamic content like a video embed.

ionitron-bot[bot] commented 9 months ago

Thanks for the issue! This issue is being closed due to inactivity. If this is still an issue with the latest version of Stencil, please create a new issue and ensure the template is fully filled out.

Thank you for using Stencil!

rwaskiewicz commented 9 months ago

Reopening - this shouldn't have been closed. Sorry about that!