Skip to content

Commit 762580f

Browse files
BobChao87gkalpak
authored andcommitted
fix(orderBy): guarantee stable sort
If a user-provided comparator fails to differentiate between two items, fall back to the built-in comparator (using the tie-breaker predicate). Fixes angular#14881 Closes angular#15914
1 parent a86a319 commit 762580f

File tree

2 files changed

+17
-1
lines changed

2 files changed

+17
-1
lines changed

src/ng/filter/orderBy.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@
5353
* dummy predicate that returns the item's index as `value`.
5454
* (If you are using a custom comparator, make sure it can handle this predicate as well.)
5555
*
56+
* If a custom comparator still can't distinguish between two items, then they will be sorted based
57+
* on their index using the built-in comparator.
58+
*
5659
* Finally, in an attempt to simplify things, if a predicate returns an object as the extracted
5760
* value for an item, `orderBy` will try to convert that object to a primitive value, before passing
5861
* it to the comparator. The following rules govern the conversion:
@@ -599,7 +602,7 @@ function orderByFilter($parse) {
599602
}
600603
}
601604

602-
return compare(v1.tieBreaker, v2.tieBreaker) * descending;
605+
return (compare(v1.tieBreaker, v2.tieBreaker) || defaultCompare(v1.tieBreaker, v2.tieBreaker)) * descending;
603606
}
604607
};
605608

test/ng/filter/orderBySpec.js

+13
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,19 @@ describe('Filter: orderBy', function() {
457457

458458
expect(orderBy(items, expr, reverse, comparator)).toEqual(sorted);
459459
});
460+
461+
it('should use the default comparator to break ties on a provided comparator', function() {
462+
// Some list that won't be sorted "naturally", i.e. should sort to ['a', 'B', 'c']
463+
var items = ['c', 'a', 'B'];
464+
var expr = null;
465+
function comparator() {
466+
return 0;
467+
}
468+
var reversed = ['B', 'a', 'c'];
469+
470+
expect(orderBy(items, expr, false, comparator)).toEqual(items);
471+
expect(orderBy(items, expr, true, comparator)).toEqual(reversed);
472+
});
460473
});
461474

462475
describe('(object as `value`)', function() {

0 commit comments

Comments
 (0)