diff --git a/src/.jshintrc b/src/.jshintrc index 241e3feccb19..f5b8b6c54bfe 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -55,6 +55,7 @@ "isBlob": false, "isBoolean": false, "isPromiseLike": false, + "hasCustomToString": false, "trim": false, "escapeForRegexp": false, "isElement": false, diff --git a/src/Angular.js b/src/Angular.js index 1030e5410521..27b7cb73090d 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -464,6 +464,11 @@ identity.$inject = []; function valueFn(value) {return function() {return value;};} +function hasCustomToString(obj) { + return isFunction(obj.toString) && obj.toString !== Object.prototype.toString; +} + + /** * @ngdoc function * @name angular.isUndefined diff --git a/src/ng/filter/filter.js b/src/ng/filter/filter.js index 19ec1d4b6b3b..e63279c04d03 100644 --- a/src/ng/filter/filter.js +++ b/src/ng/filter/filter.js @@ -163,10 +163,6 @@ function filterFilter() { }; } -function hasCustomToString(obj) { - return isFunction(obj.toString) && obj.toString !== Object.prototype.toString; -} - // Helper functions for `filterFilter` function createPredicateFn(expression, comparator, matchAgainstAnyProp) { var shouldMatchPrimitives = isObject(expression) && ('$' in expression); diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js index b976062b2b86..22d287f1dc3e 100644 --- a/src/ng/filter/orderBy.js +++ b/src/ng/filter/orderBy.js @@ -176,88 +176,114 @@ orderByFilter.$inject = ['$parse']; function orderByFilter($parse) { return function(array, sortPredicate, reverseOrder) { + if (!(isArrayLike(array))) return array; - sortPredicate = isArray(sortPredicate) ? sortPredicate : [sortPredicate]; + + if (!isArray(sortPredicate)) { sortPredicate = [sortPredicate]; } if (sortPredicate.length === 0) { sortPredicate = ['+']; } - sortPredicate = sortPredicate.map(function(predicate) { - var descending = false, get = predicate || identity; - if (isString(predicate)) { + + var predicates = processPredicates(sortPredicate, reverseOrder); + + // The next three lines are a version of a Swartzian Transform idiom from Perl + // (sometimes called the Decorate-Sort-Undecorate idiom) + // See https://en.wikipedia.org/wiki/Schwartzian_transform + var compareValues = Array.prototype.map.call(array, getComparisonObject); + compareValues.sort(doComparison); + array = compareValues.map(function(item) { return item.value; }); + + return array; + + function getComparisonObject(value, index) { + return { + value: value, + predicateValues: predicates.map(function(predicate) { + return getPredicateValue(predicate.get(value), index); + }) + }; + } + + function doComparison(v1, v2) { + var result = 0; + for (var index=0, length = predicates.length; index < length; ++index) { + result = compare(v1.predicateValues[index], v2.predicateValues[index]) * predicates[index].descending; + if (result) break; + } + return result; + } + }; + + function processPredicates(sortPredicate, reverseOrder) { + reverseOrder = reverseOrder ? -1 : 1; + return sortPredicate.map(function(predicate) { + var descending = 1, get = identity; + + if (isFunction(predicate)) { + get = predicate; + } else if (isString(predicate)) { if ((predicate.charAt(0) == '+' || predicate.charAt(0) == '-')) { - descending = predicate.charAt(0) == '-'; + descending = predicate.charAt(0) == '-' ? -1 : 1; predicate = predicate.substring(1); } - if (predicate === '') { - // Effectively no predicate was passed so we compare identity - return reverseComparator(compare, descending); - } - get = $parse(predicate); - if (get.constant) { - var key = get(); - return reverseComparator(function(a, b) { - return compare(a[key], b[key]); - }, descending); + if (predicate !== '') { + get = $parse(predicate); + if (get.constant) { + var key = get(); + get = function(value) { return value[key]; }; + } } } - return reverseComparator(function(a, b) { - return compare(get(a),get(b)); - }, descending); + return { get: get, descending: descending * reverseOrder }; }); - return slice.call(array).sort(reverseComparator(comparator, reverseOrder)); + } - function comparator(o1, o2) { - for (var i = 0; i < sortPredicate.length; i++) { - var comp = sortPredicate[i](o1, o2); - if (comp !== 0) return comp; - } - return 0; - } - function reverseComparator(comp, descending) { - return descending - ? function(a, b) {return comp(b,a);} - : comp; + function isPrimitive(value) { + switch (typeof value) { + case 'number': /* falls through */ + case 'boolean': /* falls through */ + case 'string': + return true; + default: + return false; } + } - function isPrimitive(value) { - switch (typeof value) { - case 'number': /* falls through */ - case 'boolean': /* falls through */ - case 'string': - return true; - default: - return false; - } + function objectValue(value, index) { + // If `valueOf` is a valid function use that + if (typeof value.valueOf === 'function') { + value = value.valueOf(); + if (isPrimitive(value)) return value; + } + // If `toString` is a valid function and not the one from `Object.prototype` use that + if (hasCustomToString(value)) { + value = value.toString(); + if (isPrimitive(value)) return value; } + // We have a basic object so we use the position of the object in the collection + return index; + } - function objectToString(value) { - if (value === null) return 'null'; - if (typeof value.valueOf === 'function') { - value = value.valueOf(); - if (isPrimitive(value)) return value; - } - if (typeof value.toString === 'function') { - value = value.toString(); - if (isPrimitive(value)) return value; - } - return ''; + function getPredicateValue(value, index) { + var type = typeof value; + if (value === null) { + type = 'string'; + value = 'null'; + } else if (type === 'string') { + value = value.toLowerCase(); + } else if (type === 'object') { + value = objectValue(value, index); } + return { value: value, type: type }; + } - function compare(v1, v2) { - var t1 = typeof v1; - var t2 = typeof v2; - if (t1 === t2 && t1 === "object") { - v1 = objectToString(v1); - v2 = objectToString(v2); - } - if (t1 === t2) { - if (t1 === "string") { - v1 = v1.toLowerCase(); - v2 = v2.toLowerCase(); - } - if (v1 === v2) return 0; - return v1 < v2 ? -1 : 1; - } else { - return t1 < t2 ? -1 : 1; + function compare(v1, v2) { + var result = 0; + if (v1.type === v2.type) { + if (v1.value !== v2.value) { + result = v1.value < v2.value ? -1 : 1; } + } else { + result = v1.type < v2.type ? -1 : 1; } - }; + return result; + } } diff --git a/test/ng/filter/orderBySpec.js b/test/ng/filter/orderBySpec.js index 17cd6b47dbb4..6344bdbc1e96 100644 --- a/test/ng/filter/orderBySpec.js +++ b/test/ng/filter/orderBySpec.js @@ -23,7 +23,7 @@ describe('Filter: orderBy', function() { }); - it('shouldSortArrayInReverse', function() { + it('should reverse collection if `reverseOrder` param is truthy', function() { expect(orderBy([{a:15}, {a:2}], 'a', true)).toEqualData([{a:15}, {a:2}]); expect(orderBy([{a:15}, {a:2}], 'a', "T")).toEqualData([{a:15}, {a:2}]); expect(orderBy([{a:15}, {a:2}], 'a', "reverse")).toEqualData([{a:15}, {a:2}]); @@ -116,7 +116,7 @@ describe('Filter: orderBy', function() { }); - it('should not reverse array of objects with no predicate', function() { + it('should not reverse array of objects with no predicate and reverse is not `true`', function() { var array = [ { id: 2 }, { id: 1 }, @@ -126,6 +126,39 @@ describe('Filter: orderBy', function() { expect(orderBy(array)).toEqualData(array); }); + it('should reverse array of objects with no predicate and reverse is `true`', function() { + var array = [ + { id: 2 }, + { id: 1 }, + { id: 4 }, + { id: 3 } + ]; + var reversedArray = [ + { id: 3 }, + { id: 4 }, + { id: 1 }, + { id: 2 } + ]; + expect(orderBy(array, '', true)).toEqualData(reversedArray); + }); + + + it('should reverse array of objects with predicate of "-"', function() { + var array = [ + { id: 2 }, + { id: 1 }, + { id: 4 }, + { id: 3 } + ]; + var reversedArray = [ + { id: 3 }, + { id: 4 }, + { id: 1 }, + { id: 2 } + ]; + expect(orderBy(array, '-')).toEqualData(reversedArray); + }); + it('should not reverse array of objects with null prototype and no predicate', function() { var array = [2,1,4,3].map(function(id) { @@ -151,6 +184,16 @@ describe('Filter: orderBy', function() { null ]); }); + + + it('should sort array of arrays as Array.prototype.sort', function() { + expect(orderBy([['one'], ['two'], ['three']])).toEqualData([['one'], ['three'], ['two']]); + }); + + + it('should sort mixed array of objects and values in a stable way', function() { + expect(orderBy([{foo: 2}, {foo: {}}, {foo: 3}, {foo: 4}], 'foo')).toEqualData([{foo: 2}, {foo: 3}, {foo: 4}, {foo: {}}]); + }) });