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.

Sign up for GitHub

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

Jump to bottom

Proposal: fetch elements within shadow root with shadow$ and shadow$$ #6314

Closed
wants to merge 1 commit into from
Closed

Proposal: fetch elements within shadow root with shadow$ and shadow$$ #6314

wants to merge 1 commit into from
+81 −0

Conversation

christian-bromann
Copy link
Contributor

@christian-bromann christian-bromann commented Aug 6, 2020 •
edited

There are various issues on missing support for fetching elements within the shadow DOM of elements. Some related issues are:

Also for Puppeteer embedder projects like WebdriverIO:

This PR proposes shadow$ and shadow$$ to allow query an element through element shadow roots. Given the following DOM structure:

<html>
<body>
  <div id="elem">
    #shadow-root
    <customA></customA>
    <customA>
      #shadow-root
      <subCustomB>
        #shadow-root
        <div class="container"></div>
      </subCustomB>
    </customA>
  </div>
</body>
</html>

Given someone would fetch the root of an shadow element:

const elem = await page.$('#elem')

The following queries would give following results:

  • elem.shadow$(['customA', 'subCustomB', '.container'])
    would return null because we would always pick the first element being found
  • const customAs = await elem.shadow$$(['customA'])
return customAs[1].shadow$(['subCustomB', '.container'])
    would return div.container

One idea could be to always use Element.querySelectorAll and iterate through all nodes. This could be potentially a very expensive operation though and would only make sense for shadow$$.

WDYAT?

@LarsDenBakker
Copy link
Contributor

LarsDenBakker commented Aug 6, 2020 •
edited

👍 I think this is a great step for testing web components. Always picking the first instance found is fine here I think.

One concern I have with this is that it makes tests very brittle, and tightly coupled to private APIs of web components. You have the specify the entire path of all the elements. When you build your whole application out of web components, you have a lot of implementation details in your tests.

In traditional webpages, people are used to being able to select elements by just using some landmarks such as ids, element names, form references or data attributes. With shadow dom this used by possible with the /deep/ selector but this was removed from the spec.

I think something like this is necessary to have, but it doesn't fully address all the use cases. So I wouldn't close all the related issues. There should also be a way to query elements deeply, such as implemented by libraries like https://github.com/Georgegriff/query-selector-shadow-dom

@jackfranklin
Copy link
Collaborator

Pinging @mathiasbynens @johanbay @paullewis on this as they've worked on the query handler parts more than me 👍

@mathiasbynens
Copy link
Member

Thanks for kicking off this discussion, @christian-bromann! As a general note, building on top of the existing custom query handler infrastructure makes a lot of sense 👍

This PR lets you select across a single shadow root by explicitly referencing it. That's indeed better than anything we support out of the box right now.

But, I think where we want to go eventually is a little different: as a user, I want to either select using the default magic (querySelector{All}), or select across all shadow roots (without having to reference any of them in particular). This patch only lets me pierce a single shadow root, by explicitly referencing it.

WDYT?

@LarsDenBakker
Copy link
Contributor

What can we do to move forward with this?

@christian-bromann
Copy link
Contributor Author

@LarsDenBakker I haven't been able to follow up on this but will take some time this week to address @mathiasbynens comment.

@jackfranklin
Copy link
Collaborator

@mathiasbynens is this PR still useful? Or is the work than @johanbay has been doing now going to enable this more easily?

@johanbay
Copy link
Collaborator

With the changes to the query handler infrastructure, we can do something like this #6509 to support querying elements inside shadow roots.
So in the examples above one would do page.$('pierce/.container').
Does that work for you @LarsDenBakker and @christian-bromann?

@LarsDenBakker
Copy link
Contributor

Yep that works for me.

@christian-bromann
Copy link
Contributor Author

Sounds good to me too? Shall we close this PR then?

@johanbay
Copy link
Collaborator

Yes it sounds like we can close this one and merge #6509. Thanks!

@johanbay johanbay closed this Oct 13, 2020
@christian-bromann christian-bromann deleted the cb-shadow-element branch October 13, 2020 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@christian-bromann @LarsDenBakker @jackfranklin @mathiasbynens @johanbay @googlebot