jquery / sizzle

A sizzlin' hot selector engine.
https://sizzlejs.com
Other
6.29k stars 951 forks source link

Pseudo-selectors named `is` no longer works on Chrome 88 #475

Closed chris-hut closed 3 years ago

chris-hut commented 3 years ago

Hey friends!

I've come across a quite strange error, that I've been able to pin down to naming our pseudo-selector is. We've used this in our tests for years, and it has seemingly broken overnight!

There's obviously a very easy workaround here (don't name your pseudo-selector is), but for future googlers and for my own curiosity I thought I'd post this issue!

Reproduction Steps and 🎻s

Taking the jsfiddle directly from the pseudo-selector wiki, I'm able to get this to run correctly on Chrome 87 and 88.

However, when changing the name of the psuedo-selector to is, the selector is no longer able to find any instances, nor does it ever seem to even be called!

Versions

I'm running a MacOS Big Sur and here's a little table of the versions:

Version ✅/❌
Chrome 87.0.4280.141 âś…
Chrome 88.0.4324.96 ❌
Firefox 85.0 ❌

Looking at the deprecations and removals for Chrome 88, I can't seem to find anything that would make me see why this would change behaviour!

bkardell commented 3 years ago

I would have to look more, but maybe it's the shipping of native :is() ? The system understands that as a native selector now, maybe, and that causes a break in the logic?

https://drafts.csswg.org/selectors-4/#matches

SebastianSimon commented 3 years ago

Has Sizzle ever supported :is? $(".x :is(.y, .z)") works in Firefox Nightly 88.0a1 and Chrome 89.0.4389.90, because jQuery delegates to the built-in document.querySelectorAll if implemented and if the selector is understood by the browser. Take a selector that isn’t, e.g. with :has, and Sizzle is used; and $(".x:has(.y) :is(.y, .z)") throws “Uncaught Error: Syntax error, unrecognized expression: unsupported pseudo: is” (from jQuery).


Is there an active feature request for supporting native :is in Sizzle? And :where while we’re at it?

mgol commented 3 years ago

The reason is that Sizzle first tries to run native qSA and only if it throws it does its own selection. If you add anything non-native (like :contains()), Sizzle will pick it up and the pseudo will be called: https://jsfiddle.net/m_gol/9xg3u48o/3/

This has been how Sizzle works for as long as I remember and it's unlikely to be changed now that Sizzle is close to being archived. The above serves as a workaround if you know you want the selector to go through Sizzle. You can also define a very simple pseudo that will match everything just to make qSA crash on it, e.g.: https://jsfiddle.net/m_gol/9xg3u48o/6/

timmywil commented 3 years ago

@mgol is right. The :is selector was likely added to Chrome in version 88, which would make it so that Sizzle would prefer native to a custom pseudo of the same name. To me, this is a good thing, as you probably shouldn't create a pseudo that already exists in native unless you are providing a polyfill for browsers where the native pseudo is not supported yet.

mgol commented 3 years ago

@timmywil the only issue is someone may create a pseudo with a name picked up by a future standard; at that point there was no bad practice on the part of the pseudo creator. If such a custom pseudo became popular enough, it might even require the proposed native API to change its name. In a way it's similar to how some libraries like MooTools modified native prototypes forcing standards to change.

Fortunately, jQuery/Sizzle custom pseudos seem to be a niche feature so such blocking popularity sounds unlikely. And Timmy is right the best thing one can do in such a case is to rename the pseudo.

timmywil commented 3 years ago

@mgol I agree that case would be unfortunate, but as you concluded it's unlikely to change. And I still prefer that case to the alternative.

SebastianSimon commented 3 years ago

Okay, that clears up two things:

  1. I misunderstood the bug report. I assumed that the custom-implemeted :is was supposed to have the same functionality as the native :is. Now I get why it’s suddenly “not working”.
  2. Sizzle being archived soon means native :is and :where won’t be implemented, right? I wonder what the future impact of legacy jQuery versions without an :is pseudo “polyfilled” by Sizzle will be…
mgol commented 3 years ago
2\. Sizzle being archived soon means native `:is` and `:where` won’t be implemented, right?

That's correct; we'll only make sure it works it in jQuery 4 in browsers that support the selector.