-
Notifications
You must be signed in to change notification settings - Fork 794
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
[WIP] fix(qsa): greatly improve qsa performance #2679
Closed
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
49bd6f6
fix(qsq): greatly improve qsa performance
straker 76a4253
fix
straker 7a08d34
fix Array.from polyfill
straker eeea814
no Set
straker f9989f3
refactor and tests
straker bfa341a
remove log
straker f0e7cfe
fix
straker b65fda1
fix tests
straker abc3bc6
Merge branch 'develop' into qsa-perf
straker 06687d4
resolve eslint
straker 1ff51f4
Merge branch 'develop' into qsa-perf
straker eaab92d
refactor
straker 7039be8
typos
straker f9cfe09
comments
straker File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,4 +1,5 @@ | ||||||||||
import { matchesExpression, convertSelector } from './matches'; | ||||||||||
import { getNodesMatchingSelector } from './selector-cache'; | ||||||||||
|
||||||||||
function createLocalVariables(vNodes, anyLevel, thisLevel, parentShadowId) { | ||||||||||
const retVal = { | ||||||||||
|
@@ -97,6 +98,17 @@ function matchExpressions(domTree, expressions, filter) { | |||||||||
*/ | ||||||||||
function querySelectorAllFilter(domTree, selector, filter) { | ||||||||||
domTree = Array.isArray(domTree) ? domTree : [domTree]; | ||||||||||
|
||||||||||
// see if the passed in node is the root node of the tree and can | ||||||||||
// find nodes using the cache rather than looping through the | ||||||||||
// the entire tree | ||||||||||
const nodes = getNodesMatchingSelector(domTree, selector, filter); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
if (nodes) { | ||||||||||
return nodes; | ||||||||||
} | ||||||||||
|
||||||||||
// if the selector cache is not set up or if not passed the | ||||||||||
// top level node we default back to parsing the whole tree | ||||||||||
const expressions = convertSelector(selector); | ||||||||||
return matchExpressions(domTree, expressions, filter); | ||||||||||
} | ||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,107 @@ | ||||||||||||||||||||||
import { convertSelector, matchesExpression } from './matches'; | ||||||||||||||||||||||
|
||||||||||||||||||||||
let selectorMap = {}; | ||||||||||||||||||||||
|
||||||||||||||||||||||
function cacheSelector(key, vNode) { | ||||||||||||||||||||||
selectorMap[key] = selectorMap[key] || []; | ||||||||||||||||||||||
selectorMap[key].push(vNode); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Cache selector information about a VirtalNode | ||||||||||||||||||||||
* @param {VirtualNode} vNode | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
export function cacheNodeSelectors(vNode, nodeIndex = 0) { | ||||||||||||||||||||||
if (vNode.props.nodeType !== 1) { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
// node index is used for sorting nodes by their DOM order | ||||||||||||||||||||||
// since multiple expressions can find DOM nodes out of order | ||||||||||||||||||||||
vNode._nodeIndex = nodeIndex; | ||||||||||||||||||||||
|
||||||||||||||||||||||
// cache the selector map to the root node of the tree | ||||||||||||||||||||||
if (nodeIndex === 0) { | ||||||||||||||||||||||
selectorMap = {}; | ||||||||||||||||||||||
vNode._selectorMap = selectorMap; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
cacheSelector(vNode.props.nodeName, vNode); | ||||||||||||||||||||||
cacheSelector('*', vNode); | ||||||||||||||||||||||
|
||||||||||||||||||||||
vNode.attrNames.forEach(attrName => { | ||||||||||||||||||||||
cacheSelector(`[${attrName}]`, vNode); | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Get nodes from the selector cache that match the selector | ||||||||||||||||||||||
* @param {VirtualTree[]} domTree flattened tree collection to search | ||||||||||||||||||||||
* @param {String} selector | ||||||||||||||||||||||
* @param {Function} filter function (optional) | ||||||||||||||||||||||
* @return {Mixed} Array of nodes that match the selector or undefined if the selector map is not setup | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
export function getNodesMatchingSelector(domTree, selector, filter) { | ||||||||||||||||||||||
// check to see if the domTree is the root and has the selector | ||||||||||||||||||||||
// map. if not we just return and let our QSA code do the finding | ||||||||||||||||||||||
const selectorMap = domTree[0]._selectorMap; | ||||||||||||||||||||||
if (!selectorMap) { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+47
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
const shadowId = domTree[0].shadowId; | ||||||||||||||||||||||
const expressions = convertSelector(selector); | ||||||||||||||||||||||
let matchedNodes = []; | ||||||||||||||||||||||
|
||||||||||||||||||||||
// find nodes that match just a part of the selector in order | ||||||||||||||||||||||
// to speed up traversing the entire tree | ||||||||||||||||||||||
expressions.forEach(expression => { | ||||||||||||||||||||||
// use the last part of the expression to find nodes as it's more | ||||||||||||||||||||||
// specific. e.g. for `body h1` use `h1` and not `body` | ||||||||||||||||||||||
const exp = expression[expression.length - 1]; | ||||||||||||||||||||||
|
||||||||||||||||||||||
// the expression `[id]` will use `*` as the tag name | ||||||||||||||||||||||
const isGlobalSelector = | ||||||||||||||||||||||
exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; | ||||||||||||||||||||||
let nodes = []; | ||||||||||||||||||||||
|
||||||||||||||||||||||
if (isGlobalSelector && selectorMap['*']) { | ||||||||||||||||||||||
nodes = selectorMap['*']; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
// for `h1[role]` we want to use the tag name as it is more | ||||||||||||||||||||||
// specific than using all nodes with the role attribute | ||||||||||||||||||||||
else if (exp.tag && exp.tag !== '*' && selectorMap[exp.tag]) { | ||||||||||||||||||||||
nodes = selectorMap[exp.tag]; | ||||||||||||||||||||||
} else if (exp.id && selectorMap['[id]']) { | ||||||||||||||||||||||
// when using id selector (#one) we should only select nodes | ||||||||||||||||||||||
// that match the shadowId of the root | ||||||||||||||||||||||
nodes = selectorMap['[id]'].filter(node => node.shadowId === shadowId); | ||||||||||||||||||||||
} else if (exp.classes && selectorMap['[class]']) { | ||||||||||||||||||||||
nodes = selectorMap['[class]']; | ||||||||||||||||||||||
} else if (exp.attributes) { | ||||||||||||||||||||||
// break once we find nodes that match any of the attributes | ||||||||||||||||||||||
for (let i = 0; i < exp.attributes.length; i++) { | ||||||||||||||||||||||
const attrName = exp.attributes[i].key; | ||||||||||||||||||||||
if (selectorMap['['.concat(attrName, ']')]) { | ||||||||||||||||||||||
nodes = selectorMap['['.concat(attrName, ']')]; | ||||||||||||||||||||||
break; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
// now that we have a list of all nodes that match a part of | ||||||||||||||||||||||
// the expression we need to check if the node actually matches | ||||||||||||||||||||||
// the entire expression | ||||||||||||||||||||||
nodes.forEach(node => { | ||||||||||||||||||||||
if (matchesExpression(node, expression) && !matchedNodes.includes(node)) { | ||||||||||||||||||||||
matchedNodes.push(node); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
|
||||||||||||||||||||||
if (filter) { | ||||||||||||||||||||||
matchedNodes = matchedNodes.filter(filter); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
return matchedNodes.sort((a, b) => a._nodeIndex - b._nodeIndex); | ||||||||||||||||||||||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
From the look of it, you might as well pass in the expressions variable instead of the selector. That way you're not converting it twice, and it makes for a more consistent function signature.
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.
Passing an expression makes testing harder as instead of a simple string I need to parse the selector into the expression first then pass it to the function. I don't feel that passing an expression makes it any more consistent either.