-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Fix #13434: native-API selector module #1180
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
Conversation
What's out: * 6 KB * attribute not equal selector * positional selectors (:first; :eq(n); :odd; etc.) * type selectors (:input; :checkbox; :button; etc.) * state-based selectors (:animated; :visible; :hidden; etc.) * :has(selector) * custom selectors * leading combinators (e.g., $collection.find("> *")) * reliable functionality on XML fragments * requiring all parts of a selector to match elements under context (e.g., $div.find("div > *") now matches children of $div) * matching against non-elements * reliable sorting of disconnected nodes
Also, QSA bug fixes is out. The lack of XML functionality could be a problem, and the context thing is confusing, especially if we want to move towards the new |
The XML issues only manifest in a weird edge case, but could be resolved by bringing Sizzle's conditional into Context, on the other hand, is something that I have strong reservations about because it's such a divergence. @dmethvin pointed out some other libraries that use purely native qSA, but I'm still not sure it's a good fit for us. Regardless, what better way to figure it out than with actual code? |
Ha, we could use that reasoning to do anything. |
To reiterate, though, the problem is that limiting scope is easy for simple selectors, but we can't do the same for compound selectors without duplicating Sizzle's tokenizer. |
I don't think we should release a native API that does not limit scope. This is precisely why |
That said, I think we could release a plugin rather than a built-in build option and try to make it clear that the use of this API requires at least intermediate knowledge of querySelectorAll and at some point in the future will be obsolete. |
I'm down with a plugin, but it should be officially maintained by us and I'd like to be able to build jQuery from the jQuery repo with the plugin.
That was always the point... expert understanding, use at own risk. I rarely use overly complex selectors in my client apps @bocoup—basically people like us are the target users for this functionality. |
Since you have to explicitly leave out Sizzle and include this code I would think it needs to be in the build process and the repo. I am okay with it having different semantics, we are not going to ship a built file with this in it and I suspect it isn't likely to be widely used, but it does give expert users a way to create a minimal build if they want. Can we add the list of missing features in a block comment at the top of the selector-native.js file? |
} | ||
|
||
return results; | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a not-too-distant future...
[ ...Set( document.querySelectorAll("*") ) ]; // unique ;)
I've captured the list of excluded features in a comment at the top of the new file. Now, how to test this? Can't run it through full unit tests, but it's hilarious to watch it try. We've got custom selectors sprinkled all over the tests. |
I cleaned up a lot in 59f5adb... we could fully isolate the "jQuery only" assertions, but I'm not sure it's worth the effort. |
Landed at 1083f82. |
What's out: