bug: Conditional slot rendering removes always the last slot element, using key is a workaround

[michaelworm]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

We were using 4.8.2 but it also happens with 4.10.0

Current Behavior

It occurs that always the last slot element will get removed in shadow dom when conditionally rendering the elements and removing the source slot element in the light dom.

experimentalSlotFixes makes no difference but using the "key" prop with a unique name fixes the issue.

Consider the following basic example below.

Expected Behavior

The Header Slot shall be removed in the example but the Footer Slot should be kept.

System Info

npx stencil info

      System: node 20.10.0
    Platform: darwin (23.2.0)
   CPU Model: Apple M1 Pro (10 cpus)
    Compiler: /Users/michaelworm/projects/porsche-design-system/node_modules/@stencil/core/compiler/stencil.js
       Build: 1702322155
     Stencil: 4.8.2 🐳
  TypeScript: 5.2.2
      Rollup: 2.42.3
      Parse5: 7.1.2
      Sizzle: 2.42.3
      Terser: 5.26.0

Steps to Reproduce

When clicking on "Remove Header" the Footer Slot will get removed and the Header Slot will be empty.

<p-example>
  <div slot="header">
    <h1>Header Slot</h1>
  </div>

  <p>Default Slot for Content</p>
  <button onclick="document.querySelector('[slot=header]').remove()">Remove Header</button>

  <div slot="footer">
    <p>Footer Slot</p>
  </div>
</p-example>

import { Component, Element, forceUpdate, h, type JSX } from '@stencil/core';

@Component({
  tag: 'p-example',
  shadow: true,
})
export class Example {
  @Element() public host!: HTMLElement;

  public render(): JSX.Element {
    const hasHeader = !!this.host.querySelector('[slot="header"]');
    const hasFooter = !!this.host.querySelector('[slot="footer"]');

    return (
      <div>
        {hasHeader && <slot name="header" onSlotchange={() => forceUpdate(this.host)} />}
        <slot />
        {hasFooter && <slot name="footer" onSlotchange={() => forceUpdate(this.host)} />}
      </div>
    );
  }
}

It seems that when adding elements between the slots, the bug does not occur.

...
<div>
  {hasHeader && <slot name="header" onSlotchange={() => forceUpdate(this.host)} />}
  <div></div>
  <slot />
  <div></div>
  {hasFooter && <slot name="footer" onSlotchange={() => forceUpdate(this.host)} />}
</div>
...

Code Reproduction URL

https://stackblitz.com/edit/node-dhtsl3?file=package.json,src%2Fcomponents%2Fmy-comp.tsx

Additional Information

Remove the "key" props in the StackBlitz Demo to see the issue.

[tanner-reits]

Hey @michaelworm 👋

I confirmed the issue you were seeing, but think we may already have a solution in the works! I installed a recent dev build of Stencil and it appears to solve the issue you're seeing. If you want, you can try it out by installing @stencil/core@4.9.1-dev.1704816410.051d38e!

In the meantime, I'll get this issue labeled and tied back to the relevant PR. Thanks for letting us know!

[rwaskiewicz]

The fix for this issue has gone out in today's v4.12.0 release

[michaelworm]

@rwaskiewicz Awesome, thank you!

[michaelworm]

@rwaskiewicz I can still reproduce the bug on 4.12.0 release when removing the manually added keys (footer slot is also gone when deleting header slot) => https://stackblitz.com/edit/node-wacxnr?file=src%2Fcomponents%2Fmy-comp.tsx

Is there something I am missing here?

[alicewriteswrongs]

Hey @michaelworm, thanks for checking back in to see about this issue!

I just took at look at that stackblitz repro and I'm seeing something a bit puzzling - I'm only able to reproduce the issue immediately after doing a full-page refresh of the stackblitz environment, and if I refresh the stackblitz 'browser-within-a-browser' I can than no longer reproduce the issue. I'm using this link: https://stackblitz.com/edit/node-wacxnr?file=src%2Fcomponents%2Fmy-comp.tsx

Here's what I see:

Screen.Recording.2024-02-01.at.9.49.35.AM.mov

to sort of narrate that, I'm loading up the link and once the stackblitz env boots and starts running the project I am initially able to reproduce the issue. However, if I then click the refresh button for the little stackblitz browser it won't reproduce any more.

This made me think there might be some caching behavior on stackblitz' side which is contributing to this, so I copied your component code and index.html into a local project I created with npm init stencil, where I wasn't able to reproduce the issue locally using Stencil 4.12.0. It's possible that it still can show up under some circumstances though - I may be missing something here.

So all that said, could you possibly take a look at putting together a local reproduction case? You can create a Stencil project quickly with npm init stencil component $PROJECTNAME and, after creating a reproduction case, push it up to github. If we have a reliable reproduction case we can then re-open the issue and further diagnose what's going on here.

[michaelworm]

@alicewriteswrongs Yes sure, I will try and provide a local example asap.

In the meantime, do you have any idea why the issue never occurs when manually adding keys to the slots (as it is in the first demo on Stackblitz)?

Like this:

return (
      <div>
        {hasHeader && <slot key="header" name="header" onSlotchange={() => forceUpdate(this.host)} />}
        <slot />
        {hasFooter && <slot key="footer" name="footer" onSlotchange={() => forceUpdate(this.host)} />}
      </div>
    );

[alicewriteswrongs]

Yeah, I would expect that in versions of Stencil prior to 4.12.0 adding keys there should fix the issue because the keys will allow the Stencil runtime to do a more accurate job of matching up children across component re-renders, so that the footer won't get dropped when another child disappears. In 4.12.0 we added automatic insertion of randomly-generated keys into JSX nodes where it is safe to do so - in this case, that would be into the un-named <slot /> element, so that should fix the issue as well.

[michaelworm]

@alicewriteswrongs Actually I could not reproduce it locally, you are right. So it must be something with Stackblitz. Thank you for the support!

[alicewriteswrongs]

Oh good, glad we were able to get to the bottom of it! I suspect it's something in the caching that stackblitz does to get their environment to boot so rapidly in the browser, but who knows 🤷‍♀️

good luck with your project!