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.

[romainmenke]

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.

I have tried this and this doesn't work.
ShadyDOM also breaks the native implementation.

If I force a correct loading order and I force a polyfill for :scope in browsers that have native support I get everything to work correctly.

But this is not really a fix, it is another bug :/

---

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)

Can we make a list of selectors that are impacted by this?

• :scope
• ...

A full CSS parser would not be needed by the way.
This code might need a bit of refinement and a bunch of unit tests but it should be ok to use.
expand for snippet

function replaceScopeWithAttr(query, attr) {
	var parts = [];
	var current = '';

	var escaped = false;

	var quoted = false;
	var quotedMark = false;

	var bracketed = 0;

	for (var i = 0; i < query.length; i++) {
		var char = query[i];

		if (escaped) {
			current += char;
			escaped = false;
			continue;
		}

		if (quoted) {
			if (char === quotedMark) {
				quoted = false;
			}

			current += char;
			continue;
		}

		if (current.toLowerCase() === ':scope' && !bracketed && (/^[\[\.\:\\"\s|+>~#/,&]/.test(char || ''))) {
			parts.push(current.slice(0, current.length - 6));
			parts.push('[' + attr + ']');
			current = '';
		}

		switch (char) {
			case ':':
				parts.push(current);
				current = '';
				current += char;
				continue;

			case '\\':
				current += char;
				escaped = true;
				continue;

			case '"':
			case "'":
				current += char;
				quoted = true;
				quotedMark = char;
				continue;

			case '[':
				current += char;
				bracketed++;
				continue;

			case "]":
				current += char;
				if (bracketed > 0) {
					bracketed--
				}

				continue;

			default:
				current += char;
				continue;
		}
	}

	if (current.toLowerCase() === ':scope') {
		parts.push(current.slice(0, current.length - 6));
		parts.push('[' + attr + ']');
		current = '';
	}

	if (parts.length === 0) {
		return query;
	}

	return parts.join('') + current;
}

codepen with the example

[romainmenke]

@blackjackyau Can you clarify how/why your library has a hard dependency on the ShadyDOM polyfill?

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.

Ideally no one depends on a polyfill. In a prefect world the polyfill behaves exactly like the native feature and everyone depends only on the aspects of the native feature.

With the extra information provided by @bicknellr I do not think it is doable to force ShadyDOM even in browsers with native support and also have full support for any additional selectors that have been (and will be) introduced (like :has() recently).

[mgol]

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?

Yes but just for features that don't work correctly in IE only. I know it seems go against the recommended industry practice of not detecting browsers but features (the recommendation that the jQuery team has been involved in advocating a lot as well) but there are some aspects that make it a bit special:

1. A common complaint about browser sniffing is that a browser may fix the issue and the workaround would still apply. Sometimes a fix may not even be possible without breaking the workaround. This doesn't apply here as IE is a dead browser with no development beyond security fixes at this point.
2. userAgent is not touched; IE is detected via document.documentMode which is an API that returns the IE engine version. No other browser will ever implement it as there's lots of code detecting IE via this API and those pages would often break if a browser added the API.
3. We had a lot of support tests that would now apply only to IE 11 from all our browsers supported in jQuery 4. Each of those tests takes space in the jQuery file and downloaded by all users of sites loading jQuery. Those test are often also quite demanding; some of them require an element tested to be present in the document, some trigger a layout; this happens on every page load. Removing that overhead will help less powerful devices.
4. This change in strategy applies only to IE-only issues. Issues detected in other browsers still often result in us writing a support test and basing our workaround on its result.

In this case, :scope has been supported by all modern browsers for a long time, the only exceptions being IE & Edge Legacy. Edge Legacy will not be supported by jQuery 4 and for IE we're doing the document.documentMode check. My proposal was to put that check in a short "support test" so that jQuery users can override it as they can do with the other support test results.

[bicknellr]

@romainmenke:

BTW, I think my characterization of option 3 in my last comment was wrong since it wouldn't have to give up control over the candidate list assuming the wrapper would still manually walk and call matches - i.e. it would still be able to find unassigned nodes.

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)

Can we make a list of selectors that are impacted by this?

I think I was also wrong here: after looking through https://drafts.csswg.org/selectors/, I haven't found anything other than :scope that obviously depends on a reference element.

A full CSS parser would not be needed by the way.
This code might need a bit of refinement and a bunch of unit tests but it should be ok to use.
expand for snippet

True, we don't actually need a full parser, just enough for selector lists, but we still would like to avoid this if we can since we can't really anticipate future selector syntax. @sorvell, WDYT?

@mgol:

My proposal was to put that check in a short "support test" so that jQuery users can override it as they can do with the other support test results.

Yeah, that seems like it would give jQuery users a simple workaround at least.

[romainmenke]

True, we don't actually need a full parser, just enough for selector lists, but we still would like to avoid this if we can since we can't really anticipate future selector syntax.

It is correct that you can not anticipate future selector syntax and that one day this polyfill might cause incorrect results again if people keep applying it in modern browsers.

I don't think this weighs up equally to the current state where this polyfill breaks :scope in browsers that support it natively :)

---

As I understand it, the reported issues here are :

browser does not support :scope or shadow dom :

• a polyfill is applied for both features
• applying the polyfills in the incorrect order breaks the :scope polyfill

browser supports :scope but does not support shadow dom :

• a polyfill is applied for shadow dom
• :scope no longer works correctly

browser supports :scope and shadow dom :

• a polyfill is applied (forcibly) for shadow dom
• :scope no longer works correctly

[mgol]

We discussed this issue in the jQuery Team meeting today, for now we'd like to wait and see if this issue can get a resolution within ShadyDOM as that would work beyond jQuery.

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

@blackjackyau We have a few questions. What does it exactly mean that the library is built with the assumption of the ShadyDOM polyfill? What does the polyfill have that the native APIs do not? What would it take to migrate the library to work with native Shadow DOM?

jQuery wants to avoid adding custom checks for non-standard broken environments unless the impact is large. Given that by default ShadyDOM won't apply in modern browsers, it's not yet clear to us whether the issue is big enough for jQuery to allow an escape hatch here.

_{EDIT: Questions migrated back to jquery/jquery#5032.}

[romainmenke]

Maybe better to split this issue?
It is already a bit confusing and can quickly escalate further :)

[mgol]

Makes sense; those issues were initially tied but now it seems they should be addressed separately. Let's follow-up on the jQuery aspect of it in the original jQuery issue, jquery/jquery#5032. I'll migrate my questions there.

[bicknellr]

@romainmenke:

browser does not support :scope or shadow dom :

• a polyfill is applied for both features
• applying the polyfills in the incorrect order breaks the :scope polyfill

In a vacuum, I think this could just be considered a documentation issue. Both would ideally work but fixing the case where the :scope polyfill is installed before Shady DOM boils down to the same problems as fixing this issue in general (i.e. the walk will still break this case).

browser supports :scope but does not support shadow dom:

• a polyfill is applied for shadow dom
• :scope no longer works correctly

This is the core issue; specifically, when Shady DOM enables itself and breaks existing :scope support.

browser supports :scope and shadow dom :

• a polyfill is applied (forcibly) for shadow dom
• :scope no longer works correctly

In a vacuum again, I think that making sure their app works without Shady DOM and doesn't force-enable it is a more fundamental issue than :scope being broken.

We don't want anyone to unconditionally force the polyfills if possible because they lose the automatic perf improvement from using the built-in implementation when available and are prone to accidentally becoming reliant on bugs or non-standard behaviors they introduce. The best way to go about using them is to let the polyfills decide when to enable themselves and use only the subset of features that work in both. Given that, I don't think these three are really separate issues from the perspective of finding a fix for Shady DOM.

---

I spent a little time looking at parsing selector lists yesterday and limiting the scope to just those doesn't really seem to cut down on the complexity that much. Lexing and parsing alone are quite involved. (I also just realized while looking through that page that CSS comments are permitted in selector lists given to querySelector (or any other for that matter).)

One thing to consider is that you can detect if Shady DOM is enabled with ShadyDOM.inUse. You could use this to decide if you should force-enable a :scope polyfill on top of Shady DOM if the polyfill's means of detecting :scope support isn't directly broken by Shady DOM.

[romainmenke]

One thing to consider is that you can detect if Shady DOM is enabled with ShadyDOM.inUse. You could use this to decide if you should force-enable a :scope polyfill on top of Shady DOM if the polyfill's means of detecting :scope support isn't directly broken by Shady DOM.

This is a non-starter for me.
I can not make it my responsibility to patch every feature that is broken by any polyfill.

Issues should be fixed upstream or not at all.

---

You really do not need full lexing and parsing. You only need enough to differentiate between these selectors :

• [:scope]
• [foo=:scope]
• \:scope, :\scope, ....
• ":scope", ':scope'
• :scope-not
• :scope -> only find/replace this

Comments are only a factor because they start a new token.
Replacing /* :scope */ with /* [some-attr] */ will not have adverse effect, but you can take extra steps to avoid this.

• :scope-foo -> not :scope
• :scope/* a comment */-foo -> is :scope

I've updated the snippet above to account for this.

[sorvell]

Option 5. Just native qSA + filter proposed here is really the only viable fix that makes sense right now.

1. If users opt-in to the change it should fix the issue except in corner cases expected to be exceptionally rare. It's really a wash in terms of correctness: we trade not matching undistributed nodes for matching :scope + potentially other newer selectors.
2. The polyfill is in maintenance mode and we're strongly disinclined to add new features / code to it.
3. The code already exists in ShadyDOM, the change would just be to expose it better via an option.

[romainmenke]

Using native qSA sounds good to me!

---

The polyfill is in maintenance mode and we're strongly disinclined to add new features / code to it.

From experience I can tell that it is not realistic to place a polyfill in maintenance mode before every browser that needs it has died. Issues can be discovered years after initial release.

Maybe best to clearly document that these polyfills are no longer actively maintained?

It might also be interesting to find a project that is willing to adopt these polyfills.
core-js, polyfill-library, ... exist and specialise in this area.

[justinfagnani]

Option 6, or maybe 2b, would be to split complex selectors into simple/compound selectors, and walk the logical tree matching the elements against the simple selectors with .matches(). This would be only partially a custom selector engine.

[bicknellr]

I still need to make a release; I'll post again once that's done.

[bicknellr]

Sorry, I hit a lot of snags while trying to import but I haven't forgotten about this.

[bicknellr]

I've just pushed new releases generated at a75b94e (versions in commit message). Docs for the new querySelectorImplementation setting are here:

polyfills/packages/shadydom/src/shadydom.js

Lines 121 to 195 in a75b94e

	// Use this setting to choose the implementation of `querySelector` and
	// `querySelectorAll`. The options provide different tradeoffs between
	// accuracy and performance, as explained below. More specifically, the
	// logical tree exposed by Shady DOM through the DOM APIs it wraps doesn't
	// match the structure of the tree seen by the browser (the 'real tree'), so
	// the degree to which the browser itself is used to match selectors affects
	// this tradeoff.
	//
	// - `undefined` (default): Shady DOM will walk the logical descendants of
	// the context element and call `matches` with the given selector to find
	// matching elements.
	//
	// This option is a balance between accuracy and performance. This option
	// will be able to match all logical descendants of the context element
	// (i.e. including descendants of unassigned nodes) but selectors are
	// matched against the real tree, so complex selectors may fail to match
	// since their combinators will attempt to match against the structure of
	// the real tree. Generally, you should only pass compound selectors when
	// using the default implementation. This implementation also breaks the
	// semantics of `:scope` because the wrapped `querySelector` call is
	// translated into many `matches` calls on different elements.
	//
	// - `'native'`: Shady DOM's wrapper for (e.g.) `querySelector` will call
	// into the native `querySelector` function and then filter out results that
	// are not in the same shadow root in the logical tree.
	//
	// This is the fastest option. This option will not match elements that
	// are logical descendants of unassigned nodes because they will not be in
	// the real tree, against which the browser matches the selector. Like the
	// default option, this option may not correctly match complex selectors
	// (i.e. those with combinators) due to the structural differences between
	// the real tree and the logical tree. This option preserves the semantics
	// of `:scope` because the context element of the call to the wrapper is
	// also the context element of the call to the native function.
	//
	// - `'selectorEngine'`: Shady DOM's wrapper for (e.g.) `querySelector` will
	// partially parse the given selector list and perform the tree-traversal
	// steps of a selector engine against the logical tree.
	//
	// This is the slowest and most accurate option. This option is able to
	// find all logical descendants of the context element by manually walking
	// the tree. This option is also able to correctly match complex selectors
	// because it does not rely on the browser to handle combinators. Despite
	// using `matches` to match compound selectors during selector evaluation,
	// this option preserves the semantics of `:scope` by only considering
	// compound selectors containing `:scope` to be matching if the context
	// element of the `matches` call is also the context element of the
	// wrapper call.
	//
	// However, the parser used for this option is not strictly correct.
	// Notable caveats include:
	//
	// - The parser assumes the selector list it's given is syntactically
	// correct. When given a selector that would not parse under a full
	// implementation, this implementation may fail silently.
	//
	// - Nested selectors are _not_ taken into consideration while walking
	// the tree. Each outer-most compound selector is tested against
	// candidate elements as a whole, without modification. This means that
	// selectors nested in arguments of functional pseudo-classes, such as
	// `a > b` in `:is(a > b)`, are still matched by the browser against the
	// real tree, not Shady DOM's logical tree.
	//
	// - `:scope` is only supported at the top-level. Particularly, if the
	// text `:scope` appears anywhere within a compound selector, then the
	// selector engine will only allow that compound selector to match the
	// context element. Having the text `:scope` inside a string
	// (`x[y=":scope"]`) or an argument to a functional pseudo-selector
	// (`:not(:scope > x)`) will likely break your selector.
	//
	// - The `:host` and `:host-context` pseudo-classes are not supported.
	//
	// - Comments are not supported. (e.g. `a /* b */ c`)
	//
	// - The (draft) column combinator is not supported. (e.g. `a || b`)

[romainmenke]

@bicknellr Thank you for the update!
Already very late here but I tried a few things and it seems that performance is problematic. From ±3 seconds to work through tests to ±50 seconds.

I am only using 'native' which should be the fastest option.

I will try to take a closer look as soon as possible to give you more useful feedback.

Tests are also still failing, but this might be expected.
I haven't checked our existing test suite against the listed exceptions in your docs yet.

[bicknellr]

Whoa, that's a serious perf hit. If you're able to get a reduced repro, please send it along and I'll take a look.

Maybe it's the getRootNode filtering?

[romainmenke]

If I have to guess I would say it is not your changes, or not your changes alone.

I am guessing the technique you used is similar to what I am using for :has().
Walk a tree and check each element (basically brute forcing it).

This works ok-isch when there is one such operation.
But it explodes if there is more than one.
Because each time the :has() polyfill visits an element in the tree and checks it, it will trigger a sub tree walk from your code and vice versa.

If the performance hit is just that, then I can live with that.

I don't think it is reasonable for users to expect this combo to work :

• a highly dynamic APP-like thing with many DOM changes per second
• using :has()
• using shadow dom
• targeting browsers that are close to 10 years old.

[romainmenke]

Without ShadyDOM:

• 656
• 1143

With ShadyDOM:

• 2433
• 4687

These two tests appear to be the most stable and reliable indicators.
Most of the time seems to be spend executing :scope * queries.

The whole test suite and the increase from ±3 seconds to ±50 seconds seem to caused by this change but it is noisy. Garbage collection, browser freezes, test host all come into play. (i.e. we are trashing that test machine and sometimes it's better, sometimes it's worse)

These can be rephrased as : With(out) :has polyfill.
I am 99% sure it is the combo.
As long as I don't mix those two features it works.

The :has polyfill has one benefit. It is able to quickly and cheaply defer work to the native implementation if the query doesn't contain :has.

---

What works and fails is now also less clear cut.
Before this change any additional polyfill for querySelectorAll was broken when adding ShadyDOM.
This was easy to explain to users.

Now it seems that somethings work in some browsers but not in others.

• :has() for example is fully broken in Safari 8
• :has() works fine in Edge 18 but is very slow
• :scope seems to work fine except for the cases outlined in your docs

I haven't done a full matrix of what works and what is broken in which browsers because that is very expensive. I stopped checking after noticing that it sometimes works and sometimes doesn't.

[bicknellr]

Most of the time seems to be spend executing :scope * queries.

The way that the native option works is that it calls directly into the native method on the underlying element and then iterates all results, and filters out any results that don't have the same user-facing root node as the context element (querySelectorAll) or returns the first that does (querySelector).

polyfills/packages/shadydom/src/patches/ParentNode.js

Lines 354 to 364 in a75b94e

	const candidates = Array.prototype.slice.call(
	target[utils.NATIVE_PREFIX + 'querySelectorAll'](selector)
	);
	const root = this[utils.SHADY_PREFIX + 'getRootNode']();
	// This could use `find`, but Closure doesn't polyfill it.
	for (const candidate of candidates) {
	if (candidate[utils.SHADY_PREFIX + 'getRootNode']() == root) {
	return candidate;
	}
	}
	return null;

Querying for :scope * natively in a page that's also not using native Shadow DOM means that selector selects every element below the context node. Then the filter step has to look up the logical root node of each of those candidate nodes, which may involve walking up the ancestors until reaching a root if the candidate had never been moved around before and this wasn't already cached (seems likely during testing):

polyfills/packages/shadydom/src/patches/Node.js

Lines 553 to 556 in a75b94e

	let parent = this[utils.SHADY_PREFIX + 'parentNode'];
	root = parent
	? parent[utils.SHADY_PREFIX + 'getRootNode'](options)
	: this;

If the tests often query for :scope * on trees that Shady DOM has never seen before, I could see that filtering being expensive :/ , but I hope it would cost at most O(descendants) due to the caching. I'm curious if you repeated the queries a lot if the difference between the tests before and after using the native option would decrease due to the caching.

Now it seems that somethings work in some browsers but not in others.

Just to be clear, you're specifically referring to a case where you've set querySelectorImplementation to either native or selectorEngine, right?

[romainmenke]

Just to be clear, you're specifically referring to a case where you've set querySelectorImplementation to either native or selectorEngine, right?

I tried selectorEngine but it was worse. native also has the right properties and tradeoffs for our case.

The way that the native option works is that it calls directly into the native method on the underlying element and then iterates all results, and filters out any results that don't have the same user-facing root node as the context element (querySelectorAll) or returns the first that does (querySelector).

Ha! Yeah, that won't ever be compatible with how the :has() polyfill works and that is fine :D

1. determine the query scope element or fallback to document.documentElement
2. parse .any :has(:scope <any complex selector>) .other and extract <any complex selector>
3. visit each child node of the scope element
4. call element.querySelector(<any complex selector>) on each visited child
5. if the result is not null -> add [marker] to that child
6. replace :has(<any complex selector>) with [marker]
7. return the result of document.querySelector(.any [marker] .other)

These steps are a bit simplified and a bunch more logic was needed to support relative selector syntax :has(> .foo).

We also make use of :scope in multiple places.

---

So each call to querySelector and friends quickly escalates to something that is unworkable.

But as I said before, I think this is fine.

I don't think developers should expect all these features to work together in very old browsers. If you need to target browsers so old that they don't support shadow dom, then maybe don't use :has() (or the other way around).

We can add a warning on our end if we detect that both of these features are used in the same bundle and that it will have a performance impact.

---

Please let me know if you want to try and improve the performance issues, but for me at least this is fine.

I really wanted :scope to work because it is from the same generation of features and because it is such a logical fit for web components. I don't really care about :has()

Thank you again for all the time and effort you poured into this!