Add a querySelectorAll helper function to use :scope when possible #3778
Conversation
| * A wrapper around native querySelectorAll to use :scope when possible. | ||
| * @param {!Element} parent | ||
| * @param {string} selector | ||
| * @returns {!Array.<!Element>} |
|
@mkhatib please also touch base with @cramforce on review, since he's been pushing the wider |
| * @param {string} selector | ||
| * @returns {!Array.<!Element>} | ||
| */ | ||
| export function querySelectorAll(parent, selector) { |
There was a problem hiding this comment.
Definitely would call this scopedQuerySelectorAll
What is your concrete use case? Do you need a polyfill?
There was a problem hiding this comment.
No not a polyfill, @dvoytenko suggested instead of making the call to the native querySelectorAll is to try to move it to a helper method that would use :scope when available.
| export function querySelectorAll(parent, selector) { | ||
| if (scopeSelectorSupported) { | ||
| const scopedSelectors = selector.split(',').map(sel => `:scope ${sel}`); | ||
| return toArray(parent.querySelectorAll(scopedSelectors.join(','))); |
There was a problem hiding this comment.
I guess I was trying to stick to the same return signature to other methods in dom.js, but I think for this one, we can go with the NodeList return instead since that's always what is to be returned. Will change.
|
I'm still struggling to understand why this change is needed. What is the use case? |
|
@cramforce in a preivous PR I was doing the following:
And @dvoytenko suggested that this should be moved to a |
|
I don't think it is actually faster Does scope make sense in We use this elsewhere in |
|
Ah ok, I am not sure tbh. Didn't realize the |
A prequel to #3734
ITI: #3343