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

Add utility to enable shadow root piercing #90

Closed
wants to merge 4 commits into from

Conversation

devpow112
Copy link

@devpow112 devpow112 commented Feb 7, 2023

This adds a utility that lets you easily pierce shadow roots when running CSS selectors via puppeteer framework function. Simply add shadow/ to the front of any CSS selector to make it auto pierce shadow roots. We use this set-up in a number of projects, hoping to just add it to the framework.

@devpow112 devpow112 force-pushed the depowell/shadow-root-piercing branch from ff43608 to d5a41d6 Compare February 7, 2023 20:03
@github-actions

This comment was marked as resolved.

@devpow112 devpow112 force-pushed the depowell/shadow-root-piercing branch from d5a41d6 to 0c1649c Compare February 7, 2023 20:07
@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

Co-authored-by: github-actions <github-actions@github.com>
@devpow112 devpow112 changed the title Add utility to enable shaow root piercing Add utility to enable shadow root piercing Feb 7, 2023
@devpow112 devpow112 marked this pull request as ready for review February 7, 2023 20:15
@devpow112 devpow112 requested a review from a team as a code owner February 7, 2023 20:15
@devpow112 devpow112 added the enhancement New feature or request label Feb 7, 2023
@@ -0,0 +1,12 @@
import { Puppeteer } from 'puppeteer';
import { QueryHandler } from 'query-selector-shadow-dom/plugins/puppeteer/index.js';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is interesting:

Update: As of 5.4.0 Puppeteer now has a built in shadow Dom selector, this module might not be required for Puppeteer anymore. They don't have any documentation: puppeteer/puppeteer#6509

This is something I wasn't aware of - I'm interested to see how this can be done, and maybe we don't need a special plugin. We are currently on Puppeteer 19.

Note: we are considering moving our driver to Playwright, and it too has support for this, so apparently no special plugin needed there either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per this issue (puppeteer/puppeteer#6217), maybe try this:

const shadowButton = page.$('pierce/.my-btn-style')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good if we can do this without the extra plugin at all 👍.

Also looks like in Playwright it pierces by default, no special selector required:

https://www.programsbuzz.com/article/playwright-selecting-elements-shadow-dom
microsoft/playwright#1784

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had tried using pierce/ before and it didn't work. Yeah I'm looking forward to the move to playwright just trying to limit a bunch of code copying now. Maybe I had something else wrong so this didn't work as expected. I'll give it a try tomorrow. If it works then no need for this.

Copy link
Author

@devpow112 devpow112 Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still trying to get this to work. Currently it just doesn't seem to work with pierce/. Not sure if there is something I need to do to enable it or if it's enabled by default Supposed to be enabled by default.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so this doesn't seem to work in all uses. It basically only works for stuff like page.$ but just fails for things that like page.waitForSelector. The solution proposed in this PR work for all page utilities. Not sure what we want to do in this case. I can just make my own helper repo I guess if we don't want to do this here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks for investigating, that makes sense. In that case I'm not against merging this, it does seem useful - @dbatiste any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd kinda like to dig in and figure out what does and doesn't work with the native, so we can determine whether it's worth adding another flavour of deep selection. 🤷‍♂️

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd kinda like to dig in and figure out what does and doesn't work with the native, so we can determine whether it's worth adding another flavour of deep selection. 🤷‍♂️

This was my thoughts so I closed this for now so I can look into it at some-point. If I manage to figure out something more concise I'll post it here but no need in keeping this open.

@devpow112 devpow112 closed this Feb 8, 2023
@devpow112 devpow112 deleted the depowell/shadow-root-piercing branch February 8, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants