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

Commit fe5f585

Browse files
committed
fix(orderBy): gaurantee stable sort
If user-provided comparison function fails to differentiate between two elements, fall back to built-in comparator. Fixes #14881
1 parent 8eb925d commit fe5f585

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

src/ng/filter/orderBy.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,8 @@
4949
* second, or `1` otherwise.
5050
*
5151
* In order to ensure that the sorting will be deterministic across platforms, if none of the
52-
* specified predicates can distinguish between two items, `orderBy` will automatically introduce a
53-
* dummy predicate that returns the item's index as `value`.
54-
* (If you are using a custom comparator, make sure it can handle this predicate as well.)
52+
* specified predicates nor the custom comparator can distinguish between two items, `orderBy` will
53+
* automatically introduce a dummy predicate that returns the item's index as `value`.
5554
*
5655
* Finally, in an attempt to simplify things, if a predicate returns an object as the extracted
5756
* value for an item, `orderBy` will try to convert that object to a primitive value, before passing
@@ -599,7 +598,7 @@ function orderByFilter($parse) {
599598
}
600599
}
601600

602-
return compare(v1.tieBreaker, v2.tieBreaker) * descending;
601+
return (compare(v1.tieBreaker, v2.tieBreaker) || defaultCompare(v1.tieBreaker, v2.tieBreaker)) * descending;
603602
}
604603
};
605604

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)