Skip to content
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

Remove POS selectors #293

Closed
timmywil opened this issue Nov 4, 2014 · 17 comments
Closed

Remove POS selectors #293

timmywil opened this issue Nov 4, 2014 · 17 comments

Comments

@timmywil
Copy link
Member

timmywil commented Nov 4, 2014

Of all of the features in Sizzle, positional selectors (:first, :last, :eq, etc.) are the most cumbersome and problematic. If these were removed from Sizzle, it would greatly simplify the codebase, reduce code size drastically, and set Sizzle on the same track as the Selectors spec. Obviously, this would have a major impact on user code, but we could start with a branch and make it an optional replacement in jQuery. Hopefully, we could see positional selectors phase out of Sizzle completely some time in the distant future.

Note: this is a much more significant size decrease than dropping old IE.

@mgol
Copy link
Member

mgol commented Nov 4, 2014

You mean in 3.0?

@timmywil
Copy link
Member Author

timmywil commented Nov 4, 2014

Yea, thanks

@timmywil timmywil changed the title Remove POS selectors in 2.0 Remove POS selectors in 3.0 Nov 4, 2014
@timmywil timmywil changed the title Remove POS selectors in 3.0 Remove POS selectors Nov 4, 2014
@timmywil
Copy link
Member Author

timmywil commented Nov 4, 2014

Actually, may not be 3.0 either.

@gibson042
Copy link
Member

This is a HUGE change, but I'd be much happier with a POS-free Sizzle. How can properly we evaluate the palatability of dropping those selectors, though?

@timmywil
Copy link
Member Author

timmywil commented Nov 7, 2014

@gibson042 I figure we can start by keeping it separate from Sizzle master and see how much traction we can get.

@alexandernst
Copy link

I can't really see any third-party-developers benefit from this. The entire idea behind jQuery is "let me do things easily, whitouht having to implement them myself". If you start stripping features just because jQuery is getting fat, you're basically giving up the idea behind jQuery.

@timmywil
Copy link
Member Author

The benefit is not just size/simplicity. These selectors were created before qSA existed and we've discouraged their use since then. Even if they don't get removed, don't use them. The better alternative is to use POS methods like .first().

@alexandernst
Copy link

Just to be sure I'm not missing anything here. Can't .first() and :first use the same "internal" codepath? And If "yes", why should that be even a thing to discuss? I mean, if .first() and :first perform exactly the same way, why aim at removing the selector?

I can't see any pros, while I can see huge cons, like breaking pretty much every single jquery-based code.

@dmethvin
Copy link
Member

The selector syntax is its own language that must be parsed and executed. That's Sizzle's job. The :first selector is parsed by Sizzle. The .first() method is inside jQuery, not Sizzle. But even if Sizzle called jQuery, it would still need to parse the selector. That's the hard part, not the one line of JavaScript that takes the first element of a collection.

The entire idea behind jQuery is "let me do things easily, without having to implement them myself".

Right, but we would prefer to let people do things that represent good performance and best web practices. A gun can shoot your foot easily, but that is generally not considered best practice. Over time we've removed things like jQuery.browser that are definitely not good practice, yet we have provided comprehensive tools such as jQuery Migrate to ease the transition.

@alexandernst
Copy link

You're mixing good-practices with easy-to-use tools.
Checking the browser with jQuery.browser is a bad practice, and we all agree on that. But stripping features that ease the development because they might add a small performance penalization (prove me that parsing a selector is actually that big performance hit) is a hole another thing.

A selector (instead of a call to a method inside jQuery) that would add a few ms to the total amount of time required to do the thing is hitting the 99.99% of the "yeah, I can live with an extra 15ms in my code" cases. Now, for the other 0.01% of the cases that actually use :first to iterate a bazillion elements, well... they're doing it wrong anyways, the extra 15ms per call is probably not what is causing the "this code is dog slow!" problem.

Long story short: don't try to over-optimize jQuery by stripping useful (and used!) functions just to make it faster and lighter, because in the end of the day you're trading speed for developer's time (which is why jQuery exists in the first place).

@gibson042
Copy link
Member

The problem with positional pseudos is that they can appear anywhere in a selector—#foo p:not(:last) a and #nonexistent:gt(0), #ap:lt(1) and #dl div:first > div:first are all valid, and don't even mean what they appear to. But those embedded positionals destroy the performance of selection and filtering operations.

For cases using only a single positional at the end of a single selector (e.g., #id tag:first)—which I believe to be the vast majority—you'd be far better off replacing the positional with a post-selection traversal like .first() or .eq(0). The same goes for positional embedding too—breaking them into traversal operations sandwiched between selections can improve both clarity and performance.

@paulirish
Copy link
Member

@alexandernst

might add a small performance penalization (prove me that parsing a selector is actually that big performance hit)

Yes there is a big perf penalization. I'm typically not one for JS perf results but: http://jsperf.com/jquery-first-vs-first-vs-eq-0-vs-0/18#bs-chart

I understand this does regress the developer ergonomics somewhat, but your users will appreciate it.

@dmethvin
Copy link
Member

dmethvin commented Jun 9, 2015

Although, with the new "check the cache first" change that is in 3.0 the penalty goes from about 8x to only 2x: http://jsperf.com/jquery-first-vs-first-vs-eq-0-vs-0/19

@dmethvin
Copy link
Member

dmethvin commented Nov 6, 2015

So it seems like we probably won't be able to remove positional selectors in 3.0 since we haven't come to a decision and haven't communicated it yet. Could we agree to deprecate them for 3.0 so jQuery Migrate could give console warnings? That would be a good start.

@mgol
Copy link
Member

mgol commented Nov 7, 2015

@dmethvin We had a long talk about it with @gibson042 at the summit and @gibson042's plan was to first prototype something outside of Sizzle, directly inside jQuery - basically to have the selector-native module that's not completely useless (i.e. with correct scoping rules) but without browser compat baggage of Sizzle; this module would require qSA as opposed to Sizzle but would still be able to fix broken selectors like the one in iOS8 - but not by falling back to custom traversal logic but by rewriting the selector to one that's not buggy and still use qSA on it. Something like that might very well never make sense directly in Sizzle taking into account its wide browser compat so we might as well stop using Sizzle in jQuery if this new direction proves effective.

In light of that, deprecating those selectors in Sizzle might not make sense but we could deprecate them in jQuery.

This is definitely not making it into 3.0.0, it's too late for such experiments.

@gibson042 Please correct me if I messed your intentions.

@gibson042
Copy link
Member

Not at all @mzgol; that's a good summary. I think Sizzle and especially jQuery should deprecate positional selectors. It doesn't mean they'll come out anytime soon, but it does mean that they're on notice and officially discouraged, and subject to removal in any future major release.

@mgol
Copy link
Member

mgol commented May 14, 2019

This will not happen in Sizzle but it will in jQuery. I migrated the issue to jquery/jquery#4396.

@mgol mgol closed this as completed May 14, 2019
@mgol mgol removed this from the 3.0.0 milestone Feb 28, 2020
@mgol mgol closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants