Skip to content
Permalink
Browse files

fix(orderBy): do not try to call valueOf/toString on `null`

8bfeddb added changes to make relational operator work as it
normally would in JS --- unfortunately, this broke due to my failure to account for typeof null
being "object".

This refactoring attempts to convert object values to primitives still, in a fashion similar to
the SortCompare (and subsequently the ToString() algorithm) from ES, in order to account for `null`
and also simplify code to some degree.

BREAKING CHANGE:

Previously, if either value being compared in the orderBy comparator was null or undefined, the
order would not change. Now, this order behaves more like Array.prototype.sort, which by default
pushes `null` behind objects, due to `n` occurring after `[` (the first characters of their
stringified forms) in ASCII / Unicode. If `toString` is customized, or does not exist, the
behaviour is undefined.

Closes #10385
Closes #10386
  • Loading branch information
caitp committed Dec 9, 2014
1 parent b3dfb38 commit a097aa95b7c78beab6d1b7d521c25f7d9d7843d9
Showing with 59 additions and 19 deletions.
  1. +27 −19 src/ng/filter/orderBy.js
  2. +32 −0 test/ng/filter/orderBySpec.js
@@ -160,29 +160,37 @@ function orderByFilter($parse) {
? 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 objectToString(value) {
if (value === null) return 'null';
if (typeof value.toString === 'function') {
value = value.toString();
if (isPrimitive(value)) return value;
}
if (typeof value.valueOf === 'function') {
value = value.valueOf();
if (isPrimitive(value)) return value;
}
return '';
}

function compare(v1, v2) {
var t1 = typeof v1;
var t2 = typeof v2;
// Prepare values for Abstract Relational Comparison
// (http://www.ecma-international.org/ecma-262/5.1/#sec-11.8.5):
// If the resulting values are identical, return 0 to prevent
// incorrect re-ordering.
if (t1 === t2 && t1 === "object") {
// If types are both numbers, emulate abstract ToPrimitive() operation
// in order to get primitive values suitable for comparison
t1 = typeof (v1.valueOf ? v1 = v1.valueOf() : v1);
t2 = typeof (v2.valueOf ? v2 = v2.valueOf() : v2);
if (t1 === t2 && t1 === "object") {
// Object.prototype.valueOf will return the original object, by
// default. If we do not receive a primitive value, use ToString()
// instead.
t1 = typeof (v1.toString ? v1 = v1.toString() : v1);
t2 = typeof (v2.toString ? v2 = v2.toString() : v2);

// If the end result of toString() for each item is the same, do not
// perform relational comparison, and do not re-order objects.
if (t1 === t2 && v1 === v2 || t1 === "object") return 0;
}
v1 = objectToString(v1);
v2 = objectToString(v2);
}
if (t1 === t2) {
if (t1 === "string") {
@@ -125,6 +125,22 @@ describe('Filter: orderBy', function() {
});
expect(orderBy(array)).toEqualData(array);
});


it('should sort nulls as Array.prototype.sort', function() {
var array = [
{ id: 2 },
null,
{ id: 3 },
null
];
expect(orderBy(array)).toEqualData([
{ id: 2 },
{ id: 3 },
null,
null
]);
});
});


@@ -252,5 +268,21 @@ describe('Filter: orderBy', function() {
});
expect(orderBy(array)).toEqualData(array);
});


it('should sort nulls as Array.prototype.sort', function() {
var array = [
{ id: 2 },
null,
{ id: 3 },
null
];
expect(orderBy(array)).toEqualData([
{ id: 2 },
{ id: 3 },
null,
null
]);
});
});
});

0 comments on commit a097aa9

Please sign in to comment.
You can’t perform that action at this time.