Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Component markup disappears when overwriting default slot using text #3977

Closed
3 tasks done
oscargm opened this issue Jan 12, 2023 · 9 comments · Fixed by #5146
Closed
3 tasks done

bug: Component markup disappears when overwriting default slot using text #3977

oscargm opened this issue Jan 12, 2023 · 9 comments · Fixed by #5146
Assignees
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@oscargm
Copy link

oscargm commented Jan 12, 2023

Prerequisites

Stencil Version

3.0.0-beta.1

Current Behavior

When the following component is defined:

import { h, Component, ComponentInterface, Element, Host } from '@stencil/core';

@Component({
  tag: 'custom-element-button',
  styleUrl: 'button.scss',
  shadow: false,
  scoped: false,
})
export class Button implements ComponentInterface {
  @Element() host;

  render(): HTMLElement {
    return (
      <Host class="test-button">
        <button>
          <div class="content-wrapper">
            <span class="prefix">
              <slot name="prefix" />
            </span>
            <slot />
            <slot name="suffix" />
          </div>
        </button>
      </Host>
    );
  }
}

When modifying default slot content using javascript:

<custom-element-button>Button</custom-element-button>
document.querySelector('custom-element-button').innerText = "newButtonText"

All the markup inside disappears and only the text remains.

Expected Behavior

Same as with shadow DOM

System Info

N/A

not relevant, happening in multiple OS's, browsers, and node versions

Steps to Reproduce

Download the linked repository, install, start the project, and type into the input element

Code Reproduction URL

https://github.com/oscargm/stencil-slot-problem

Additional Information

In the reporduction repository you'll find a demo using components with:

  • shadowDOM (works fine)
  • scopedCSS without shadowDOM (issue reproducible)
  • neither scopedCSS or shadowDOM (issue reproducible)

If this is the intended behaviour I'd like to know how to avoid this issue.

I found out that adding a span like htis:

<scoped-css-button class="button with-children-element">
  <span>Button</span>
</scoped-css-button>

and changing the text of the span everything works fine, or even with the prefix/suffix named slots changing the textContent or innerText it works fine.

@ionitron-bot ionitron-bot bot added the triage label Jan 12, 2023
@oscargm
Copy link
Author

oscargm commented Jan 12, 2023

Also this is not related with beta version, also happening in 2.13

@yigityuce
Copy link
Contributor

We are having the same issue in our project and it's a huge blocker. Could you pls prioritize slot-related issues? @rwaskiewicz

@johnjenkins
Copy link
Contributor

johnjenkins commented Jan 15, 2023

just chiming in to say you can achieve what you need now by enabling the extras option scopedSlotTextContentFix, then using textContent (instead of innerText).

@oscargm
Copy link
Author

oscargm commented Jan 16, 2023

Hi @johnjenkins ,

Many thanks for pointing the extras config, I wasn't aware of it.
Adding the parameter to the config fixes the problem for scopedCSS, but not for elements which don't have shadowDOM neither scopedCSS.

I see that you linked the issue to a PR, did you test components without shadowDOM and without scopedCSS in your branch?

I was assuming stencil would work in the same mode and output the same code & behaviour for the following cases:

  • components with shadowDOM
  • components with scopedCSS
  • components without scopedCSS and without shadowDOM

I see that more or less the first 2 cases are working similar with the scopedSlotTextContentFix option set to true.
But third case still not working

@johnjenkins
Copy link
Contributor

johnjenkins commented Jan 16, 2023

hm that's odd.
AFAIK the 'decision' to polyfill slot behaviour has nothing to do with scoped css :/

@tanner-reits tanner-reits self-assigned this Jan 17, 2023
@tanner-reits
Copy link
Contributor

Hey @oscargm 👋

Just took a look at your repro and fiddled with the recommendations @johnjenkins posed. Does look like enabling that extras option fixes some use cases, but definitely still something funky happening when not using scoped: true or shadow: true (as you found). I'll get this labeled appropriately and capture this info for the team!

Thanks for reporting the issue!

@tanner-reits tanner-reits added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels Jan 17, 2023
@oscargm
Copy link
Author

oscargm commented Jan 17, 2023

Hey, many thanks for dedicating some time.on validating the bug.

@yigityuce found out that patchTextContent function execution in dom-extras was only executed with scopedCSS and the flag enabled.
He implemented a custom polyfill in our project wich allowed us to move forward without shadowDOM and without scopedCSS. So far the polyfill is working for our needs and it is something similar to patchTextContent but a bit different.

@yigityuce
Copy link
Contributor

Hey @tanner-reits !

As @oscargm mentioned we implemented custom polyfill for the components, I am planning to create a PR and if it is okay for you I would like to assign it to you for your reviews.

tanner-reits pushed a commit that referenced this issue Dec 8, 2023
This commit updates our patch for `textContent` to mimic the Shadow Root implementation

Fixes: #3977

STENCIL-687
github-merge-queue bot pushed a commit that referenced this issue Dec 11, 2023
* only patch scoped components

* patch `textContent` for `scoped` components

This commit updates our patch for `textContent` to mimic the Shadow Root implementation

Fixes: #3977

STENCIL-687

* fix e2e tests for new implementation

* ignore comment nodes in getter

* fix issue with removing multiple nodes in slot for setter

* add e2e tests for `textContent` patch

* re-add build flag checks
@christian-bromann
Copy link
Member

Hey there 👋 we have released a Stencil version v4.8.2 with a fix for this. Make sure you enable the experimentalSlotFixes flag to apply it. If you have any questions, please let us know. Thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
5 participants