-
Notifications
You must be signed in to change notification settings - Fork 470
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
Draft: feat(query-deep): Using a query selector that supports queries into the shadow dom of elements #1069
Conversation
…he shadow dom of elements
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 03e1477:
|
👀 |
Is this still in progress? |
Hi!
Sorry about that
We change the test approach cause access the shadow dom was impossible :(
I’m really sad with this decision, but, the product can’t wait.
very thanks. ❤️
…On Tue, 18 Jan 2022 at 06:12 Waldemar Penner ***@***.***> wrote:
Is this still in progress?
—
Reply to this email directly, view it on GitHub
<#1069 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE3QC43RADNQKLEPHBH4CELUWUVIDANCNFSM5GVQJMXQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
@alineDiesel, |
Yeap.
We are testing the form flow state with unit test and functional testing in
the critics flows.
At the first we thought of doing only integration tests…
You have another recommendation?
…On Wed, 9 Feb 2022 at 00:48 Michael Brown ***@***.***> wrote:
Hi! Sorry about that We change the test approach cause access the shadow
dom was impossible :( I’m really sad with this decision, but, the product
can’t wait. very thanks. ❤️
Can I ask what your new "test approach" is?
—
Reply to this email directly, view it on GitHub
<#1069 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE3QC4ZCOVWAGVEPXEUY3PTU2HPYZANCNFSM5GVQJMXQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
Not at this stage, sadly. |
What:
This is the second RFC-PR for solving the shadow-dom problems. It is created as a follow up on #1054, this time showing a less generic approach - by building the shadow dom query functionality right into the framework, rather than opening it to consumers. After discussions, the goal would be to get a review for either of the two to merge it.
This commit will automatically resolve queries that have to reach into the shadow dom of any of the children to find requested elements. You can see an example in the test here: MatthiasKainer@03e1477#diff-03031dc49abaa5227e8e4e13fd7459557e494345a85e4a29e7a78c10d7046cfeR87
This test would fail with the old version and works with this change. It will fix issues #742 & #413 and every other that's related to shadow dom.
Why:
In our project, we are using a design system built with web components (via stenciljs), and different projects in different frameworks (lit, react, vue) want to use this library. However, due to the issues already highlighted in #742 #413 and the fact that the current workaround is not really a long-term solution as it wraps an old version of this library, this change will solve this issue once and for all.
How:
As said, unlike in #1054, this change replaces the query mechanism of the core library. It uses the library Georgegriff/query-selector-shadow-dom - which, to my knowledge, is also the goto solution for other testing libraries that struggle with shadow-dom with 200k downloads a week.
The change gravitates around the following lines of code: MatthiasKainer@03e1477#diff-75da2f8ae17b208e9a92d6eee57458b0a5e95f89ced8e4c400bfeb5e0d66ee85R18
It's a wrapper for the query method, that is to be used by the application rather than using the
querySelector
andquerySelectorAll
methods directly.Given it changes the query mechanism this might be considered a potentially breaking change. I lack the knowledge of all the use cases of this library, but one example can already be found in the adjustments in the node tests: MatthiasKainer@03e1477#diff-03031dc49abaa5227e8e4e13fd7459557e494345a85e4a29e7a78c10d7046cfeR4
Basically, non-browser environments might have issues with this change as the library will need access to the document, and to the Node interface. The reason is performance optimisation (no need to search everything if no shadow element exists) and testing if an element is a node.
Checklist:
docs site
Pinging @eps1lon @alexkrolick to follow up on the discussions from #1054