ShadyDOM breaks querySelector polyfills

[romainmenke]

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 :

• https://github.com/jonathantneal/element-qsa-scope
• @webcomponents/shadydom/shadydom.min.js

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

• Chrome
• Firefox
• Edge
• Safari
• IE 11

[mgol]

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: jquery/jquery#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]

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

[bicknellr]

@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]

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]

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 webcomponents/shadydom#210 and webcomponents/shadydom#216, which seem relevant but I've only skimmed them so far.

[blackjackyau]

@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]

@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:

<style>
outer-element inner-element {
  color: blue;
}
outer-element > inner-element {
  color: red;
}
</style>
<outer-element>
  <template shadowroot="open">
    <div>
      <slot></slot>
    </div>
  </template>
  <inner-element>the text</inner-element>
</outer-element>

In a browser with native Shadow DOM support, "the text" is red because the <inner-element> matches outer-element > inner-element. However, when Shady DOM is being used¹, it flattens out the shadow root so that the real tree looks like this:

<style>
outer-element inner-element {
  color: blue;
}
outer-element > inner-element {
  color: red;
}
</style>
<outer-element>
  <div>
    <inner-element>the text</inner-element>
  </div>
</outer-element>

If we assume Shady CSS² isn't being used to rewrite selectors in that <style>³, then the outer-element > inner-element no longer matches the <inner-element> (so "the text" is blue). Point being, there's a number of selectors that Shady DOM inherently breaks given that the browser still has to do selector matching for the purpose of rendering styles and not all selectors are able to be rewritten by Shady CSS.

---

Here's a summary of the options that we talked about:

1. Rearrange the relevant part of the real tree to match the user-facing tree of the root containing the context element for the duration of the wrapped querySelector and querySelectorAll calls so that the wrappers can call the native functions and get the correct elements directly.

   This would be the most correct option but it seems too dangerous since connecting and disconnecting nodes have lots of side effects - for example, custom element reactions and MutationObserver callbacks. Some of these could be wrapped, but there are others which are pretty drastic and unavoidable, such as <iframe> discarding its document when disconnected.

2. Use (or implement enough of) a selector engine that lets us hook into its traversal and make it use Shady DOM's user-facing tree instead of the real tree.

   This would probably be pretty slow but reasonably correct. However, I haven't found any existing selector engines that would let us hook into the DOM traversal part yet, so we'd probably have to write our own, which doesn't seem worth the complexity given that (a) doing this robustly would require actually parsing selectors, and (b) it can't be made forward-compatible with any other special selectors that might show up in the future. (AFAIK, these are the same reasons why jQuery itself stopped using a custom selector engine.)

3. Rewrite just :scope in selectors passed to Shady DOM's wrappers (probably in a way similar to the :scope-for-querySelector polyfill mentioned in the original issue post) so that they don't depend on a particular context element and Shady DOM's wrappers can continue to call matches on each relevant element.

   This again means either parsing selectors or being very brittle. This option is less correct than the last one because we're using the browser to match the given selectors against the flattened tree.

4. Change Shady DOM's general behavior for unassigned nodes by having it insert them into the tree (hidden in a display: none; container or something), have the wrappers call into the native querySelector and querySelectorAll, and filter the results to only nodes in the correct root.

   This option would be faster than the earlier ones because the browser would be doing all the selector matching, but also seems like it would also be pretty dangerous to ship to existing users.

5. Have the wrappers call into the native querySelector and querySelectorAll and filter the results to only nodes in the correct root. (Same as above, minus unassigned nodes.)

   This is the strategy from Change QSA patch to use native QSA and filter the results shadydom#210 but that was implemented for just querySelector in Performance optimizations shadydom#216 with a non-standard flag to a standard function, which is not usually the API shape we're shooting for.

   Like the previous two options, this still hands selectors over to the browser for matching without doing anything special with the tree, so those selectors have to match in the flattened tree. Also, it's less correct than the last option because it won't find unassigned nodes since they're not in the flattened tree, but the selectors you could currently use to find a disconnected node with the wrapped querySelector and querySelectorAll are pretty limited specifically because they're not in the flattened tree.

Given the costs involved with the more correct options and that using Shady DOM already requires some awareness of the flat tree with regards to selectors because Shady CSS can't transform all of them perfectly, we think the best choice is to add a flag to Shady DOM that enables the last option (i.e. the wrappers just call into the native functions and filter). This means that the same rules for selectors passed to these functions in Shady DOM would apply (i.e. they're matched against the flat tree), but it would trade the ability to potentially select unassigned nodes for the ability to use context-element-dependent selectors since the given selector would now be matched using the actual node you called the function on as the scoping root.

Footnotes

1. Shady DOM does not support declarative shadow dom (i.e. the <template shadowroot="open">) so this shadow root would have to be created and populated imperatively. ↩

2. Shady CSS is the very closely related polyfill / component of Shady DOM that handles style scoping. ↩

3. I'm not sure it would even matter in this case because I don't think Shady CSS modifies child combinators, but let's claim that Shady CSS is totally disabled. ↩

[romainmenke]

Rewrite just :scope in selectors passed to Shady DOM's wrappers (probably in a way similar to the :scope-for-querySelector polyfill mentioned in the original issue post) so that they don't depend on a particular context element and Shady DOM's wrappers can continue to call matches on each relevant element.

This again means either parsing selectors or being very brittle.
This option is less correct than the last one because we're using the browser to match the given selectors against the flattened tree.

Can you give an example that would be incorrect?

My knowledge of shadow dom is limited and I can not currently judge how impactful it is to be incorrect in these cases.

[mgol]

@bicknellr Thanks for the comprehensive proposal. I'll state what I understood from the perspective of the jQuery issue, please correct me if I'm wrong.

My understanding

As I understand, the proposed change requires folks using ShadyDOM in forced mode to enable this new flag to make it work again when updating jQuery to 4.0.0 or newer. I also understand that this flag would make the patched qSA work differently which may be a tough sell for apps in maintenance mode as it may require significant changes to the code base - but at least there's a path for them to do the upgrade; without the patch, it would be simply impossible. Is that correct?

jQuery behavior

jQuery's find is closer to the now removed from the spec queryAll, in that it requires all of the parts of the selector to lie under the queried element as opposed to querySelectorAll which selects globally and just filters out results outside of the element. jQuery still tries to use qSA when possible but it needs to augment the selectors to account for this distinction.

In jQuery <4, the Sizzle selector engine runs a support test for :scope when the library is loaded and based on that it either leverages the pseudo-class or it instead assigns a temporary ID to the context element. That means that as long as ShadyDOM in forced mode is loaded before jQuery, the support test will fail and selection will work; it'll just be slower as all browsers will get those temporary IDs assigned, triggering layout, mutation observers, etc.

In jQuery >=4, we plan to drop all support tests that would only fail in IE and instead to do brand checks via document.documentMode. This means :scope will be used in all non-IE browsers without a support test, making it incompatible with the current ShadyDOM.

Potential changes to jQuery

Optionally, we could create an escape hatch and still define the support test in jQuery 4, just define it via the brand check:

jQuery.support.scope = !document.documentMode;

and then use that test result internally instead of a direct brand check. The out-of-the box behavior would be the same but people using ShadyDOM in forced mode could add the following to their app:

jQuery.support.scope = false;

after jQuery is loaded and all browsers would get the temporary ID treatment.

I'd normally prefer to have an upstream fix instead of adding such escape hatches to jQuery as this can blow up in size if we start considering all cases, but ShadyDOM may potentially have a large enough impact that it might be acceptable, especially that there's no ideal solution in sight.

Pinging the jQuery core team: @timmywil @gibson042 @dmethvin what do you think?

Would that make the changes in ShadyDOM not necessary or are there other concerns that justify a change?

P.S. jQuery qSA usage

P.S. Expand for more details on jQuery qSA usage

Newer jQuery versions try to use qSA when possible on an augmented selector but they still fallback to the Sizzle selector engine when the selection throws an error, indicating an unrecognized selector. The same fallback is applied if our internal support tests indicate the selector would be treated in a buggy way by the current browser (for example, see the support test for buggy "[name='']" treatment in IE 11). This is a binary switch: when the fallback is applied, the whole selector goes through Sizzle and qSA is not used.

Such behavior requires Sizzle to implement all the native behaviors of qSA in all supported browsers which doesn't scale; this is still the behavior in the latest stable jQuery 3.6.0. Therefore, in jQuery 4 we plan to take a different strategy and fully depend on qSA, rewriting buggy selectors to other non-buggy equivalent native selectors and pass those through qSA.

[bicknellr]

@romainmenke:

Can you give an example that would be incorrect?

Sure, let's say we have a tree like this:

<outer-element>
  <template shadowroot="open">
    <div>
      <slot></slot>
    </div>
  </template>
  <inner-element>the text</inner-element>
</outer-element>

and we want to do this:

const innerElement = outerElement.querySelector(':scope > inner-element');

and we expect that to get the <inner-element>. Then, let's say we're also doing this on top of a hypothetical version of Shady DOM that rewrites :scope in selectors passed to the querySelector wrapper to a unique class that it also adds to the context element. Before calling querySelector, Shady DOM will have already flattened the tree above to this:

<outer-element>
  <div>
    <inner-element>the text</inner-element>
  </div>
</outer-element>

Then we make the querySelector call, so Shady DOM's wrapper adds the class:

<outer-element class="very-unique-class">
  <div>
    <inner-element>the text</inner-element>
  </div>
</outer-element>

Then, it rewrites:scope > inner-element to .very-unique-class > inner-element and walks non-shadow-including descendants of <outer-element>, calling .matches('.very-unique-class > inner-element') on each until it finds one. Shady DOM will walk over (only) the <inner-element> but the browser says it doesn't match because, in the real tree, Shady DOM moved <inner-element> to a different parent when it flattened <outer-element>'s fake shadow root into its non-shadow descendants.

The current implementation as well as options 3, 4, and 5 all have this problem because they all use the browser to test selectors against the flat tree.¹ However, unlike the current implementation (or option 2), those options also give up control over the set of candidate elements so that Shady DOM's (native) querySelector callee will be the same as the user's (wrapped) querySelector callee to preserve the meaning of :scope, but this also means that they would no longer find any unassigned nodes that Shady DOM removed from the real tree.

---

I'm finding it hard to keep track of all of this, so here's my current understanding in table form:

	Supports :scope?	No CSS parser?	Finds unassigned elements?	No new tree effects?	A > B works?
0. current implementation		✓	✓	✓
1. Rearrange the tree	✓	✓	✓		✓
2. Custom elector engine	✓		✓	✓	✓
3. Rewrite :scope	✓		✓	✓
4. Insert unassigned nodes, native qSA + filter	✓	✓	✓
5. Just native qSA + filter	✓	✓		✓

In that meeting the other day, our priorities seemed to roughly land here: "No CSS parser?" and "No new tree effects?" are important, "Finds unassigned elements?" is breaking but seems ok if opt-in, and "A > B works?" would just be nice to have.

---

@mgol:

As I understand, the proposed change requires folks using ShadyDOM in forced mode to enable this new flag to make it work again when updating jQuery to 4.0.0 or newer. I also understand that this flag would make the patched qSA work differently which may be a tough sell for apps in maintenance mode as it may require significant changes to the code base - but at least there's a path for them to do the upgrade; without the patch, it would be simply impossible. Is that correct?

That seems like a good assessment.

In jQuery >=4, we plan to drop all support tests that would only fail in IE and instead to do brand checks via document.documentMode. This means :scope will be used in all non-IE browsers without a support test, making it incompatible with the current ShadyDOM.

I'm confused. This makes it sound like jQuery 4 is intended to continue supporting browsers without native :scope but is going to stop deciding which implementation to use based on feature detection and switch to browser sniffing instead?

Would that make the changes in ShadyDOM not necessary or are there other concerns that justify a change?

Yes, I think removing :scope from all selectors before they reach Shady DOM would be enough to work around this specific problem.

Footnotes

1. Working around this is a key compromise of using Shady DOM: your selectors have to be defensive against all shadow trees being 'inlined' into one tree - including those of components you don't control. It's a leaky abstraction, but Shady DOM is generally meant to be used with Shady CSS, which does some amount of selector rewriting in <style>s to make each compound selector only match elements that would be in the relevant shadow root, which gets you a lot further than you might expect. Shady CSS doesn't do this with a real parser, so certain selectors break and they are more likely to break as they become more complicated. It's not something we'd like to expand the scope of given that most people are now using browsers with native Shadow DOM support. ↩

[bicknellr]

(I messed up the table originally: the "3. Rewrite :scope" row shouldn't have included "native qSA + filter".)

[romainmenke]

🤔 This sounds very brittle and there doesn't seem to be a path that is an obvious fix for the original issue.

I need to test a few things on my end, but I might be able to apply all other qSA polyfills (:scope, :has()) first, before Shady DOM.

This way ShadyDOM should be unable to break those polyfills.
This would be technically less correct as polyfills are normally ordered by toposort based on the dependency graph, but I might be able to hack around this.