Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(orderBy): sort by identity if no predicate is given #9403

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/ng/filter/orderBy.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* correctly, make sure they are actually being saved as numbers and not strings.
*
* @param {Array} array The array to sort.
* @param {function(*)|string|Array.<(function(*)|string)>} expression A predicate to be
* @param {function(*)|string|Array.<(function(*)|string)>=} expression A predicate to be
* used by the comparator to determine the order of elements.
*
* Can be one of:
Expand All @@ -24,10 +24,13 @@
* is interpreted as a property name to be used in comparisons (for example `"special name"`
* to sort object by the value of their `special name` property). An expression can be
* optionally prefixed with `+` or `-` to control ascending or descending sort order
* (for example, `+name` or `-name`).
* (for example, `+name` or `-name`). If no property is provided, (e.g. `'+'`) then the array
* element itself is used to compare where sorting.
* - `Array`: An array of function or string predicates. The first predicate in the array
* is used for sorting, but when two items are equivalent, the next predicate is used.
*
* If the predicate is missing or empty then it defaults to `'+'`.
*
* @param {boolean=} reverse Reverse the order of the array.
* @returns {Array} Sorted copy of the source array.
*
Expand Down Expand Up @@ -116,15 +119,21 @@ orderByFilter.$inject = ['$parse'];
function orderByFilter($parse){
return function(array, sortPredicate, reverseOrder) {
if (!(isArrayLike(array))) return array;
if (!sortPredicate) return array;
sortPredicate = isArray(sortPredicate) ? sortPredicate: [sortPredicate];
if (sortPredicate.length === 0) { sortPredicate = ['+']; }
sortPredicate = sortPredicate.map(function(predicate){
var descending = false, get = predicate || identity;
if (isString(predicate)) {
if ((predicate.charAt(0) == '+' || predicate.charAt(0) == '-')) {
descending = predicate.charAt(0) == '-';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why == instead of === ?

predicate = predicate.substring(1);
}
if ( predicate === '' ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no spaces

// Effectively no predicate was passed so we compare identity
return reverseComparator(function(a,b) {
return compare(a, b);
}, descending);
}
get = $parse(predicate);
if (get.constant) {
var key = get();
Expand Down
15 changes: 12 additions & 3 deletions test/ng/filter/orderBySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,18 @@ describe('Filter: orderBy', function() {
orderBy = $filter('orderBy');
}));

it('should return same array if predicate is falsy', function() {
var array = [1, 2, 3];
expect(orderBy(array)).toBe(array);
it('should return sorted array if predicate is not provided', function() {
expect(orderBy([2, 1, 3])).toEqual([1, 2, 3]);

expect(orderBy([2, 1, 3], '')).toEqual([1, 2, 3]);
expect(orderBy([2, 1, 3], [])).toEqual([1, 2, 3]);
expect(orderBy([2, 1, 3], [''])).toEqual([1, 2, 3]);

expect(orderBy([2, 1, 3], '+')).toEqual([1, 2, 3]);
expect(orderBy([2, 1, 3], ['+'])).toEqual([1, 2, 3]);

expect(orderBy([2, 1, 3], '-')).toEqual([3, 2, 1]);
expect(orderBy([2, 1, 3], ['-'])).toEqual([3, 2, 1]);
});

it('shouldSortArrayInReverse', function() {
Expand Down