bug: Dynamically rendered slot elements causes NotFoundError on insertBefore call (Angular)

[glafontaine]

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

4.28.2

Current Behavior

Dynamically rendered slot elements causes issue while patched insertBefore function tries to insert the child.

The issue occurs regardless of the Angular version, whether using *ngIf or the new control flow syntax.

The error seems to have been fixed for React #6140 (although i still manage to get the same error in some specific cases.)

Expected Behavior

It should insert elements without an error.

System Info

System: node 22.13.1
    Platform: windows (10.0.26100)
     Stencil: 4.28.2
  TypeScript: 5.5.4
      Rollup: 4.34.9
      Parse5: 7.2.1
      jQuery: 4.0.0-pre
      Terser: 5.37.0

Steps to Reproduce

reproduction url: https://github.com/glafontaine/stencil-angular-repro

Code Reproduction URL

https://github.com/glafontaine/stencil-angular-repro

Additional Information

No response

[christian-bromann]

Thank you for reporting and providing a reproduction case, any contributions that may resolve this bug are much appreciated.

[jsulpis]

Hello @christian-bromann,
I think I found the cause of the issue :

core/src/runtime/bootstrap-custom-element.ts

Lines 47 to 66 in 992a687

	if (BUILD.experimentalSlotFixes) {
	if (BUILD.scoped && cmpMeta.$flags$ & CMP_FLAGS.scopedCssEncapsulation) {
	// This check is intentionally not combined with the surrounding `experimentalSlotFixes` check
	// since, moving forward, we only want to patch the pseudo shadow DOM when the component is scoped
	patchPseudoShadowDom(Cstr.prototype);
	}
	} else {
	if (BUILD.slotChildNodesFix) {
	patchChildSlotNodes(Cstr.prototype);
	}
	if (BUILD.cloneNodeFix) {
	patchCloneNode(Cstr.prototype);
	}
	if (BUILD.appendChildSlotFix) {
	patchSlotAppendChild(Cstr.prototype);
	}
	if (BUILD.scopedSlotTextContentFix && cmpMeta.$flags$ & CMP_FLAGS.scopedCssEncapsulation) {
	patchTextContent(Cstr.prototype);
	}
	}

• the experimentalSlotFixes option only applies to components with scoped styles (I don't see this in the documentation)
• on components without scoped styles, 4 patches are applied based on activated options
• there are 7 other patches, including the one of insertBefore, only applied with experimentalSlotFixes in the case of scoped styles :
  

  core/src/runtime/dom-extras.ts

  Lines 18 to 30 in 992a687

  	export const patchPseudoShadowDom = (hostElementPrototype: HTMLElement) => {
  	patchCloneNode(hostElementPrototype);
  	patchSlotAppendChild(hostElementPrototype);
  	patchSlotAppend(hostElementPrototype);
  	patchSlotPrepend(hostElementPrototype);
  	patchSlotInsertAdjacentElement(hostElementPrototype);
  	patchSlotInsertAdjacentHTML(hostElementPrototype);
  	patchSlotInsertAdjacentText(hostElementPrototype);
  	patchInsertBefore(hostElementPrototype);
  	patchTextContent(hostElementPrototype);
  	patchChildSlotNodes(hostElementPrototype);
  	patchSlotRemoveChild(hostElementPrototype);
  	};

So it seems that components without scoped styles are never fully patched, and trigger errors when used in Angular (or other frameworks I guess). But it also seems that it is an expected behavior ? (it would be quite unconvenient for us to migrate all our components to scoped styles)

core/src/runtime/bootstrap-custom-element.ts

Line 50 in 992a687

// since, moving forward, we only want to patch the pseudo shadow DOM when the component is scoped

If this is a bug, I can work on a fix if you have one in mind.
Thank you for your feedback

[christian-bromann]

• the experimentalSlotFixes option only applies to components with scoped styles (I don't see this in the documentation)

This may be not clear in the docs but all slot fixes are only applied to scoped components as shadow components use the actual web components web features implemented in the browser.

So it seems that components without scoped styles are never fully patched

What do you think needs patching? I haven't been able to investigate the issue but it would make sense to get a clear idea what is the root cause of the problem here.

it would be quite unconvenient for us to migrate all our components to scoped styles)

That is not what we recommend.

If this is a bug, I can work on a fix if you have one in mind.

Any contributions are welcome, thank you!

[jsulpis]

This may be not clear in the docs but all slot fixes are only applied to scoped components as shadow components use the actual web components web features implemented in the browser.

What I meant is that components that have neither shadow dom nor scoped styles (components parameters are { shadow: false, scoped: false }) don't have any patch applied when experimentalSlotFixes is enabled :

 if (experimentalSlotFixes) { 
   if (scopedCssEncapsulation) { 
     // all patches applied
    patchPseudoShadowDom()
   } 
   // HERE no patch applied
 } else {
    // when experimentalSlotFixes is false there are 4 patches available, but not `insertBefore`
    ...
 } 

There is a note in the experimentalScopedSlotChanges section, but not in experimentalSlotFixes so we would expect that it applies on all components, scoped or not.

What do you think needs patching? I haven't been able to investigate the issue but it would make sense to get a clear idea what is the root cause of the problem here.

So the issue is that is it seems currently impossible to have the patch of insertBefore applied on components that have no shadow dom nor scoped style. So an error is triggered when Angular tries to dynamically insert an element in a slot using insertBefore.

I see two options here :

1. either call the function patchPseudoShadowDom also on non scoped components, and update the function itself to add conditions (like apply experimentalScopedSlotChanges only if scopedCssEncapsulation is true). But it goes against this comment in the code that I don't understand : "moving forward, we only want to patch the pseudo shadow DOM when the component is scoped"
2. or add more "extra configs" like a parameter insertBeforeFix that will apply the patch of insertBefore. But there are 7 patches that are currently missing, that might bring a log of additional config.

What do you prefer ?

[glafontaine]

either call the function patchPseudoShadowDom also on non scoped components, and update the function itself to add conditions (like apply experimentalScopedSlotChanges only if scopedCssEncapsulation is true). But it goes against this comment in the code that I don't understand : "moving forward, we only want to patch the pseudo shadow DOM when the component is scoped"

Hello, can we get an answer on this please ?
Would a call of patchPseudoShadowDom on non-scoped components break things ?

[christian-bromann]

What I meant is that components that have neither shadow dom nor scoped styles (components parameters are { shadow: false, scoped: false }) don't have any patch applied when experimentalSlotFixes is enabled

We strongly discourage using components that are non scoped and have no shadow DOM. Our team has found that it would be very hard if not impossible to apply proper slot fixes for these components. In fact we are likely deprecate this type of component in the next major version release.

I see two options here :

Both options require significant amount of development resources and add more complexity to maintain with no fundamental value.

What do you prefer ?

I recommend not to use non-scoped/shadow components and instead use scoped components. Any reasons why this could not be an option for you?

[jsulpis]

Thank you for your feedback.

We have an architecture with the components styles isolated in a dedicated package so it makes sense to use non-scoped styles (we don't need scoping), but if it will become deprecated we will see if we can migrate to scoped styles.

[erwingeiger]

We encounter the same problems. Also we use not Shadow DOM. The reason we don't use Shadow DOM is that it has impact on the performace of the website. Is that true?
Also we have a main CSS file the components are using. Also some components are nested with others and uses the CSS of the parent component

[christian-bromann]

Is that true?

I am not aware of any case where using native web components would slow down an application. I wonder what you've experienced here 🤔

Generally using shadow roots can be challenging when it comes to styling. That is why Stencil continues to offer a scoped mode which polyfills a lot of features that native web components provide without using shadow root.

[erwingeiger]

Is that true?

I am not aware of any case where using native web components would slow down an application. I wonder what you've experienced here 🤔

Generally using shadow roots can be challenging when it comes to styling. That is why Stencil continues to offer a scoped mode which polyfills a lot of features that native web components provide without using shadow root.

About the performance. This was what our main developer said.

We never had any problems without using shadow DOM or scoped. Since a update of Stencil we have the same problems as the author of this topic. But if I understand it correctly. Stencil will not fix this If we not using Shadow DOM or shoped styles?

[christian-bromann]

Stencil will not fix this If we not using Shadow DOM or shoped styles?

Correct. Maintaining the non-scoped and non-shadow mode in Stencil has shown to be too difficult to maintain with no substantial value to users, which is why we consider to deprecate it. If you can provide more information on the performance issue you are facing for scoped components, please raise a new issue.