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: puppeteer: piercing on level 1 breaks #5717

Open
3 tasks done
danyball opened this issue Apr 29, 2024 · 8 comments
Open
3 tasks done

bug: puppeteer: piercing on level 1 breaks #5717

danyball opened this issue Apr 29, 2024 · 8 comments
Labels
Resolution: Refine This PR is marked for Jira refinement. We're not working on it - we're talking it through.

Comments

@danyball
Copy link

Prerequisites

Stencil Version

4.14.0

Current Behavior

We have a teaser component with this shadowRoot:
Pasted Graphic

Our test wants to select the "foo" element. This works till 4.14.0:
const label = await page.find("sdx-teaser >>> .label")

Now the "bar" element in sdx-tag's shadowRoot gets selected, because it also has the .label class.

Expected Behavior

Not really sure if this is a bug or it works as designed by puppeteer. But my expectation is that a "level 1 piercing" should only select elements in its own shadowRoot.

System Info

System: node 18.19.0
    Platform: darwin (23.4.0)
   CPU Model: Apple M1 Max (10 cpus)
    Compiler: /node_modules/@stencil/core/compiler/stencil.js
       Build: 1713902307
     Stencil: 4.17.1 🚒
  TypeScript: 5.4.5
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.30.3

Steps to Reproduce

  1. checkout https://github.com/danyball/stencil-piercing
  2. npm run test
  3. (optional) if you downgrade stencil version, the test will run

Code Reproduction URL

https://github.com/danyball/stencil-piercing

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Apr 29, 2024
@christian-bromann christian-bromann self-assigned this Apr 29, 2024
@jcfranco
Copy link
Contributor

I think this is working as expected based on the Stencil doc.

By default Stencil will look into all shadow roots to find the element.

FWIW, I also thought this was a bug when the feature was introduced, as I would expect the opposite (explicit >>> per shadow root to look into).

@christian-bromann
Copy link
Member

Thanks for clarifying @jcfranco

The Stencil docs state that:

By default Stencil will look into all shadow roots to find the element.

@danyball do you have an idea how we can make the docs more clear? It does not seem like that there is a way to query only the shadow root of a specific component.

@danyball
Copy link
Author

danyball commented May 2, 2024

So I can not run the test in my reproducer repo without adding classes or something like that? What if I dont just have this simple component in component setup? Could be difficult to get the correct element if there are multiple levels with multiple ".label" classes in there.

Maybe a solution could be to change the order of "look into all shadow roots": first of all check the level0 shadow, and if nothing found start searching in all other shadow roots.

A little hint: the test in my reproducer repo breaks since 4.14.0, so this version is a breaking change release. Maybe mark it in some way.

The docs could show an example, like:
It is not possible to just search for an element in a specific shadow root. If you have this structure...

<foo-component>
#shadow-root (open)
      <bar-component>
      #shadow-root (open)
            <div class="label" />
      </bar-component>

      <div class="label">
        Hello, World! I'm {this.getText()}
      </div>
</foo-component>;

... you need to use a helper class to select the lower .label element. foo-component >>> .label will return the one in bar-component.

@christian-bromann
Copy link
Member

Maybe a solution could be to change the order of "look into all shadow roots": first of all check the level0 shadow, and if nothing found start searching in all other shadow roots.

I can see why this would be an interesting solution given how the current selector works. However all query commands usually work in a way that they find whatever element comes first in the DOM. Now, in a "Shadow-dom-less" world we could just do foo-component > .label but this won't work.

A little hint: the test in my reproducer repo breaks since 4.14.0, so this version is a breaking change release. Maybe mark it in some way.

Well, that behavior was broken (even if it was working for you).

One workaround could be:

await page.setContent('<my-component first="foo"></my-component2>');
const element = await page.find('my-component')
expect(element.shadowRoot.querySelector('.label').textContent)
  .toEqualText("Hello, World! I'm foo");

@danyball
Copy link
Author

danyball commented May 3, 2024

Thanks for the workaround! But would be cool to have an E2EElement to do something like expect(await label.isVisible()).toBe(true).

A last idea regarding "looking in all shadow doms" logic:
A webcomponent is a delimited element. All tests should relate exclusively to this element and not include elements of other shadow doms. Because these elements in turn are tested in the test of this component. If I am explicit interested in one of these nested shadows, I could use the cool new deep piercing selector.

@christian-bromann
Copy link
Member

@danyball can you please elaborate on this last idea?

@danyball
Copy link
Author

danyball commented May 7, 2024

Sure: We have the sdx-teaser component. It has sdx-tag as a child in its shadow. Everything which has to do with sdx-tag is tested in tag.e2e. This component should be a "black box" for sdx-teaser. There is no need for a teaser test to go into the tag's shadow. I can not see a use case for that. And if I really want to, I could do it with >>>.
If you see each web component as a independent system the .find() function should not look into each shadow to find an element.

@christian-bromann
Copy link
Member

I agree, this should be at least configurable, similar how the newSpecPage allows to define the components that are suppose to be loaded. WebdriverIO has similar options when it comes to Stencil component tests.

For now, all I can do is to ingest this into our backlog and find a resolution on this with the team. Thanks for your input.

@christian-bromann christian-bromann added Resolution: Refine This PR is marked for Jira refinement. We're not working on it - we're talking it through. and removed triage labels May 7, 2024
@christian-bromann christian-bromann removed their assignment May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Refine This PR is marked for Jira refinement. We're not working on it - we're talking it through.
Projects
None yet
Development

No branches or pull requests

3 participants