webcomponents / polyfills

Web Components Polyfills
BSD 3-Clause "New" or "Revised" License
1.13k stars 166 forks source link

ShadyDOM breaks querySelector polyfills #480

Closed romainmenke closed 1 year ago

romainmenke commented 2 years ago

Description

querySelector and friends require polyfills for features like :scope or :has() in older browsers.

These polyfills break when also including @webcomponents/shadydom/shadydom.min.js. Not sure why as I am not familiar with the code base here. Could be because an unpolyfilled querySelector is memoized or because it is overwritten without wrapping and extending. the original

Example

include :

run someElement.querySelector(':scope')

This will not return the expected result.

Expected behavior

Expected ShadyDOM to leave querySelector as is or correctly wrap it.

Actual behavior

ShadyDOM breaks querySelector polyfills.

Version

"@webcomponents/shadydom": "^1.9.0"

Browsers affected

mgol commented 2 years ago

This is not just breaking qSA polyfills; ShadyDom actually breaks support for :scope in qSA in browsers that natively support it. This causes problems for jQuery users: https://github.com/jquery/jquery/issues/5032. It will make the upcoming jQuery 4.0.0 completely broken when this polyfill is loaded as it assumes :scope support in all non-IE browsers.

romainmenke commented 2 years ago

@bicknellr What is the support and maintenance status of these polyfills?

bicknellr commented 2 years ago

@sorvell would have a better idea of the status of Shady DOM specifically but he's out of office for a few days; I'll ping him when he's back. Shadow DOM and custom elements are supported in all modern browsers so we've generally been handling issues for those polyfills on an on-demand basis, depending on severity.

A couple things that would be helpful to know: Are you seeing this issue in situations where the polyfill is expected to be disabled? (I.e. in a browser that supports shadow DOM and the polyfill is not being explicitly enabled.) Also, in what order does your repro install Shady DOM and the other polyfill?

mgol commented 2 years ago

I see that the report that jQuery got was with window.ShadyDOM = {force: true}; here's a modified version to load just ShadyDOM instead of the whole Web Components bundle and to show the issue without the use of jQuery: https://codepen.io/mgol/pen/JjLGLZy?editors=1010

@blackjackyau what's the reason you're forcing ShadyDOM to apply even in browsers with native Shadow DOM support?

bicknellr commented 2 years ago

Thanks for the repro! It looks like this issue is happening because Shady DOM's wrappers for these methods walk the nodes in the relevant shadow root and call matches on them with the provided selector to build the list of matched nodes, which doesn't make sense for a selector containing :scope (and a few other things, for that matter). I'm not the right person to ask about whether or not this is known / considered in scope for Shady DOM. Also, the comment in Shady DOM there points to https://github.com/webcomponents/shadydom/pull/210 and https://github.com/webcomponents/shadydom/pull/216, which seem relevant but I've only skimmed them so far.

blackjackyau commented 2 years ago

@mgol yes, shady is needed in our case as the library is built with the assumption of shady polyfill. And yes, it looks like shadydom is no longer needed ( deprecated ) as most of the modern browsers are now supported with native shadow dom support.

bicknellr commented 2 years ago

@sorvell, @justinfagnani, and I met to discuss this issue and talked through some options. This is going to be a tradeoff between correctness and performance, so we want to get feedback from this thread to make sure that our proposed option would actually work for your use cases before diving in on the implementation.

For context, Shady DOM emulates Shadow DOM by making the structure of the real tree match (to the extent that it can) what would be the flattened tree from the perspective of a user of the DOM APIs it wraps, which is effectively what is rendered by browsers with native Shadow DOM support. More specifically, this means that in the real tree (a) children of shadow root hosts are replaced by the children of their shadow roots, (b) slots in those shadow roots are replaced by the children of those shadow roots' hosts that would be assigned to them, and (c) elements which would not be assigned to a slot are removed from the tree. As you would expect, this has implications for selectors since the browser matches selectors against the tree as it is structured in its internal model and not against Shady DOM's user-facing model somehow.


Open this for an example of how Shady DOM builds the real tree from the user-facing tree and how this affects selectors, if you're interested. Consider this tree: ```html the text ``` In a browser with native Shadow DOM support, "the text" is red because the `` matches `outer-element > inner-element`. However, when Shady DOM is being used[^dsd], it flattens out the shadow root so that the real tree looks like this: ```html
the text
``` If we assume Shady CSS[^shadycss] isn't being used to rewrite selectors in that `