Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added isEq to _.isEqual for proper equality checking of 0 and -0 in sets and maps #2508

Closed
wants to merge 4 commits into from
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 17 additions & 5 deletions test/objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -559,12 +559,24 @@
var other = {a: 1};
assert.strictEqual(_.isEqual(new Foo, other), false, 'Objects from different constructors are not equal');


// Tricky object cases val comparisions
assert.equal(_.isEqual([0], [-0]), false);
assert.equal(_.isEqual({a: 0}, {a: -0}), false);
assert.equal(_.isEqual([NaN], [NaN]), true);
assert.equal(_.isEqual({a: NaN}, {a: NaN}), true);
assert.equal(_.isEqual([0], [-0]), false, '0 and -0 are not equal as array values');
assert.equal(_.isEqual({a: 0}, {a: -0}), false, '0 and -0 are not equal as object values');
assert.equal(_.isEqual([NaN], [NaN]), true, 'NaN and NaN are equal as array values');
assert.equal(_.isEqual({a: NaN}, {a: NaN}), true, 'NaN and NaN are equal as object values');

// SameValueZero tests for isEq function used for sets and maps
if (typeof Set !== 'undefined') {
var set0 = new Set().add(0);
var setNeg0 = new Set().add(-0);
assert.equal(_.isEqual(set0, setNeg0), true, 'In sets 0 and -0 are equal');
}
if (typeof Map !== 'undefined') {
var map0 = new Map().set(0, 0);
var mapNeg0 = new Map().set(-0, 0);
assert.equal(_.isEqual(map0, mapNeg0), true, 'In maps keys of 0 and -0 are equal');
}


if (typeof Symbol !== 'undefined') {
var symbol = Symbol('x');
Expand Down
40 changes: 34 additions & 6 deletions underscore.js
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@


// Internal recursive comparison function for `isEqual`.
var eq, deepEq;
var eq, isEq, deepEq;
eq = function(a, b, aStack, bStack) {
// Identical objects are equal. `0 === -0`, but they aren't identical.
// See the [Harmony `egal` proposal](http://wiki.ecmascript.org/doku.php?id=harmony:egal).
Expand All @@ -1169,6 +1169,20 @@
return deepEq(a, b, aStack, bStack);
};

// Internal recursive comparison function for `isEqual`.
isEq = function(a, b, aStack, bStack) {
// Performs a SameValueZero comparison to check for equality between two values.
// http://ecma-international.org/ecma-262/6.0/#sec-samevaluezero
// Differs from eq in that 0 and -0 will be considered equal by isEq.
// Used in deepEq for testing Map and Set equality.
if (a === b) return a === b || (a !== a && b !== b);
if (a == null || b == null) return a === b;
if (a !== a) return b !== b;
var type = typeof a;
if (type !== 'function' && type !== 'object' && typeof b != 'object') return false;
return deepEq(a, b, aStack, bStack);
};

// Internal recursive comparison function for `isEqual`.
deepEq = function(a, b, aStack, bStack) {
// Unwrap any wrapped objects.
Expand Down Expand Up @@ -1205,8 +1219,8 @@
if (!areArrays) {
if (typeof a != 'object' || typeof b != 'object') return false;

// Objects with different constructors are not equivalent, but `Object`s or `Array`s
// from different frames are.
// Objects with different constructors are not equivalent, but `Object`s, `Array`s,
// `Map`s, or `Set`s from different frames are.
var aCtor = a.constructor, bCtor = b.constructor;
if (aCtor !== bCtor && !(_.isFunction(aCtor) && aCtor instanceof aCtor &&
_.isFunction(bCtor) && bCtor instanceof bCtor)
Expand All @@ -1218,7 +1232,7 @@
// structures is adapted from ES 5.1 section 15.12.3, abstract operation `JO`.

// Initializing stack of traversed objects.
// It's done here since we only need them for objects and arrays comparison.
// It's done here since we only need them for objects, arrays, maps, and sets comparison.
aStack = aStack || [];
bStack = bStack || [];
var length = aStack.length;
Expand All @@ -1232,7 +1246,7 @@
aStack.push(a);
bStack.push(b);

// Recursively compare objects and arrays.
// Recursively compare objects, arrays, maps and sets.
if (areArrays) {
// Compare array lengths to determine if a deep comparison is necessary.
length = a.length;
Expand All @@ -1241,7 +1255,7 @@
while (length--) {
if (!eq(a[length], b[length], aStack, bStack)) return false;
}
} else {
} else if (className === '[object Object]') {
// Deep compare objects.
var keys = _.keys(a), key;
length = keys.length;
Expand All @@ -1252,7 +1266,21 @@
key = keys[length];
if (!(_.has(b, key) && eq(a[key], b[key], aStack, bStack))) return false;
}
} else {
// Deep compare sets and maps.
var size = a.size;
// Ensure that both objects are of the same size before comparing deep equality.
if (b.size !== size) return false;
while (size--) {
// Deep compare the keys of each member, using SameValueZero (isEq) for the keys
if (!(isEq(a.keys().next().value, b.keys().next().value, aStack, bStack))) return false;
// If the objects are maps deep compare the values. Value equality does not use SameValueZero.
if (className === '[object Map]') {
if (!(eq(a.values().next().value, b.values().next().value, aStack, bStack))) return false;
}
}
}

// Remove the first object from the stack of traversed objects.
aStack.pop();
bStack.pop();
Expand Down