Skip to content

Commit

Permalink
assert: loose deep equal should not compare symbol properties
Browse files Browse the repository at this point in the history
This is the way it's currently documented and that seems appropriate
for loose equal assertions. The change was not intentional.

Fixes: #27652

PR-URL: #27653
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
  • Loading branch information
BridgeAR authored and targos committed May 14, 2019
1 parent 9ed5882 commit 493ead1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
13 changes: 8 additions & 5 deletions lib/internal/util/comparisons.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const {
const {
getOwnNonIndexProperties,
propertyFilter: {
ONLY_ENUMERABLE
ONLY_ENUMERABLE,
SKIP_SYMBOLS
}
} = internalBinding('util');

Expand Down Expand Up @@ -163,8 +164,9 @@ function innerDeepEqual(val1, val2, strict, memos) {
if (val1.length !== val2.length) {
return false;
}
const keys1 = getOwnNonIndexProperties(val1, ONLY_ENUMERABLE);
const keys2 = getOwnNonIndexProperties(val2, ONLY_ENUMERABLE);
const filter = strict ? ONLY_ENUMERABLE : ONLY_ENUMERABLE | SKIP_SYMBOLS;
const keys1 = getOwnNonIndexProperties(val1, filter);
const keys2 = getOwnNonIndexProperties(val2, filter);
if (keys1.length !== keys2.length) {
return false;
}
Expand Down Expand Up @@ -198,8 +200,9 @@ function innerDeepEqual(val1, val2, strict, memos) {
// Buffer.compare returns true, so val1.length === val2.length. If they both
// only contain numeric keys, we don't need to exam further than checking
// the symbols.
const keys1 = getOwnNonIndexProperties(val1, ONLY_ENUMERABLE);
const keys2 = getOwnNonIndexProperties(val2, ONLY_ENUMERABLE);
const filter = strict ? ONLY_ENUMERABLE : ONLY_ENUMERABLE | SKIP_SYMBOLS;
const keys1 = getOwnNonIndexProperties(val1, filter);
const keys2 = getOwnNonIndexProperties(val2, filter);
if (keys1.length !== keys2.length) {
return false;
}
Expand Down
9 changes: 8 additions & 1 deletion test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ assertDeepAndStrictEqual(-0, -0);
const b = new Uint8Array(4);
a[symbol1] = true;
b[symbol1] = false;
assertNotDeepOrStrict(a, b);
assertOnlyDeepEqual(a, b);
b[symbol1] = true;
assertDeepAndStrictEqual(a, b);
// The same as TypedArrays is valid for boxed primitives
Expand All @@ -649,6 +649,13 @@ assertDeepAndStrictEqual(-0, -0);
assertOnlyDeepEqual(boxedStringA, boxedStringB);
boxedStringA[symbol1] = true;
assertDeepAndStrictEqual(a, b);
// Loose equal arrays should not compare symbols.
const arr = [1];
const arr2 = [1];
arr[symbol1] = true;
assertOnlyDeepEqual(arr, arr2);
arr2[symbol1] = false;
assertOnlyDeepEqual(arr, arr2);
}

assert.throws(
Expand Down

0 comments on commit 493ead1

Please sign in to comment.