stencil icon indicating copy to clipboard operation
stencil copied to clipboard

feature: allow to exclude components from rendering in E2E tests

Open danyball opened this issue 1 year ago • 9 comments

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

danyball avatar Apr 29 '24 14:04 danyball

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).

jcfranco avatar Apr 29 '24 20:04 jcfranco

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.

christian-bromann avatar Apr 30 '24 18:04 christian-bromann

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.

danyball avatar May 02 '24 07:05 danyball

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");

christian-bromann avatar May 02 '24 17:05 christian-bromann

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.

danyball avatar May 03 '24 08:05 danyball

@danyball can you please elaborate on this last idea?

christian-bromann avatar May 03 '24 15:05 christian-bromann

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.

danyball avatar May 07 '24 06:05 danyball

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 avatar May 07 '24 17:05 christian-bromann

I moving this into a feature request. To improve Stencils testing capabilities it would be nice if the newE2EPage would provide primitives that would allow users to ignore certain components that are not relevant to the test case to be ignored from rendering. Any contributions on this would be appreciated.

christian-bromann avatar Jul 17 '24 18:07 christian-bromann