fix(perf): remove need for node sorting from select completely

Author: dylanb

lib/core/base/context.js

@@ -200,6 +200,7 @@ function validateContext(context) {
  * @param {Object} spec Configuration or "specification" object
  */
 function Context(spec) {
+	//jshint maxstatements:18
 	'use strict';
 	var self = this;
 
@@ -229,4 +230,8 @@ function Context(spec) {
 	if (err instanceof Error) {
 		throw err;
 	}
+	if (!Array.isArray(this.include)) {
+		this.include = Array.from(this.include);
+	}
+	this.include.sort(axe.utils.nodeSorter); // ensure that the order of the include nodes is document order
 }

lib/core/utils/select.js

@@ -68,19 +68,17 @@ function pushNode(result, nodes) {
 }
 
 /**
- * returns true if any of the nodes in the list is a parent of another node in the list
+ * reduces the includes list to only the outermost includes
  * @param {Array} the array of include nodes
- * @return {Boolean}
+ * @return {Array} the modified array of nodes
  */
-function hasOverlappingIncludes(includes) {
-	let list = includes.slice();
-	while (list.length > 1) {
-		let last = list.pop();
-		if (list[list.length - 1].actualNode.contains(last.actualNode)) {
-			return true;
+function reduceIncludes(includes) {
+	return includes.reduce((res, el) => {
+		if (!res.length || !res[res.length - 1].actualNode.contains(el.actualNode)) {
+			res.push(el);
 		}
-	}
-	return false;
+		return res;
+	}, []);
 }
 
 /**
@@ -94,10 +92,6 @@ axe.utils.select = function select(selector, context) {
 	'use strict';
 
 	var result = [], candidate;
-	if (!Array.isArray(context.include)) {
-		context.include = Array.from(context.include);
-	}
-	context.include.sort(axe.utils.nodeSorter); // ensure that the order of the include nodes is document order
 	if (axe._selectCache) { // if used outside of run, it will still work
 		for (var j = 0, l = axe._selectCache.length; j < l; j++) {
 			// First see whether the item exists in the cache
@@ -112,18 +106,16 @@ axe.utils.select = function select(selector, context) {
 			return isNodeInContext(node, context);
 		};
 	})(context);
-	for (var i = 0; i < context.include.length; i++) {
-		candidate = context.include[i];
+	var reducedIncludes = reduceIncludes(context.include);
+	for (var i = 0; i < reducedIncludes.length; i++) {
+		candidate = reducedIncludes[i];
 		if (candidate.actualNode.nodeType === candidate.actualNode.ELEMENT_NODE &&
 			axe.utils.matchesSelector(candidate.actualNode, selector) &&
 			curried(candidate)) {
 			result = pushNode(result, [candidate]);
 		}
 		result = pushNode(result, axe.utils.querySelectorAllFilter(candidate, selector, curried));
 	}
-	if (context.include.length > 1 && hasOverlappingIncludes(context.include)) {
-		result.sort(axe.utils.nodeSorter);
-	}
 	if (axe._selectCache) {
 		axe._selectCache.push({
 			selector: selector,

test/core/base/context.js

@@ -98,6 +98,19 @@ describe('Context', function() {
 				[$id('foo'), $id('bar')]);
 		});
 
+		it('should sort the include nodes in document order', function() {
+			fixture.innerHTML = '<div id="foo"><div id="bar"></div></div><div id="baz"></div>';
+
+			var result = new Context([
+				['#foo'],
+				['#baz'],
+				['#bar']
+			]);
+
+			assert.deepEqual(result.include.map(function (n) { return n.actualNode; }),
+				[$id('foo'), $id('bar'), $id('baz')]);
+		});
+
 		it('should remove any null reference', function() {
 			fixture.innerHTML = '<div id="foo"><div id="bar"></div></div>';
 

test/core/base/rule.js

@@ -521,9 +521,18 @@ describe('Rule', function() {
 							evaluate: function() {},
 							id: 'cats'
 						}]
+					}, {
+						checks: {
+							cats: {
+								run: function (node, options, resolve) {
+									success = true;
+									resolve(true);
+								}
+							}
+						}
 					});
 					rule.run({
-						include: axe.utils.getFlattenedTree(document)[0]
+						include: [axe.utils.getFlattenedTree(document)[0]]
 					}, {}, noop, isNotCalled);
 					assert.isTrue(success);
 
@@ -538,9 +547,18 @@ describe('Rule', function() {
 							evaluate: function() {},
 							id: 'cats'
 						}]
+					}, {
+						checks: {
+							cats: {
+								run: function (node, options, resolve) {
+									success = true;
+									resolve(true);
+								}
+							}
+						}
 					});
 					rule.run({
-						include: axe.utils.getFlattenedTree(document)[0]
+						include: [axe.utils.getFlattenedTree(document)[0]]
 					}, {}, function() {
 						success = true;
 					}, isNotCalled);

test/core/utils/select.js

@@ -125,30 +125,19 @@ describe('axe.utils.select', function () {
 
 	});
 
-	it('should sort by DOM order', function () {
-		fixture.innerHTML = '<div id="one"><div id="target1" class="bananas"></div></div>' +
-			'<div id="two"><div id="target2" class="bananas"></div></div>';
-
-		var result = axe.utils.select('.bananas', { include: [axe.utils.getFlattenedTree($id('two'))[0],
-			axe.utils.getFlattenedTree($id('one'))[0]] });
-
-		assert.deepEqual(result.map(function (n) { return n.actualNode; }),
-			[$id('target1'), $id('target2')]);
-
-	});
-
-	it('should sort by DOM order on overlapping elements', function () {
+	it('should not return duplicates on overlapping includes', function () {
 		fixture.innerHTML = '<div id="zero"><div id="one"><div id="target1" class="bananas"></div></div>' +
 			'<div id="two"><div id="target2" class="bananas"></div></div></div>';
 
-		var result = axe.utils.select('.bananas', { include: [axe.utils.getFlattenedTree($id('one'))[0],
-			axe.utils.getFlattenedTree($id('zero'))[0]] });
+		var result = axe.utils.select('.bananas', { include: [axe.utils.getFlattenedTree($id('zero'))[0],
+				axe.utils.getFlattenedTree($id('one'))[0]] });
 
 		assert.deepEqual(result.map(function (n) { return n.actualNode; }),
-			[$id('target1'), $id('target1'), $id('target2')]);
-		assert.equal(result.length, 3);
+			[$id('target1'), $id('target2')]);
+		assert.equal(result.length, 2);
 
 	});
+
 	it ('should return the cached result if one exists', function () {
 		fixture.innerHTML = '<div id="zero"><div id="one"><div id="target1" class="bananas"></div></div>' +
 			'<div id="two"><div id="target2" class="bananas"></div></div></div>';