Bug: Set compares by reference not value #50
Description
Upstream fast-deep-equal can't handle equal(new Set(["1", {}]), new Set(["1", {}])) because it compares by reference setA.has(setBElement).
Note that when running the correctness tests during yarn benchmark we get differences with lodash.isEqual which is correct:
--- correctness tests: generic and react ---
react-fast-compare
fast-deep-equal
lodash.isEqual
- different result: lodash.isEqual objects objects with different `toString` functions returning same values are equal
- different result: lodash.isEqual Maps empty maps of different class are not equal
- different result: lodash.isEqual Maps not equal maps (same key "order" - instances of different classes)
- different result: lodash.isEqual Sets empty sets of different class are not equal
- different result: lodash.isEqual Sets not equal sets (same value "order" - instances of different classes)
- different result: lodash.isEqual Sets not equal sets (different instances of objects)
Here's a WIP diff against https://github.com/FormidableLabs/react-fast-compare/tree/experiment/es6 branch that could fix that:
diff --git a/index.js b/index.js
index 2f03bfb..6451b7b 100644
--- a/index.js
+++ b/index.js
@@ -56,11 +56,23 @@ function equal(a, b) {
return true;
}
+ // There's an upstream bug in `fast-deep-equal` for nested `Set`s
+ // which our tests do cover.
+ var bIt, bI;
if (hasSet && (a instanceof Set) && (b instanceof Set)) {
if (a.size !== b.size) return false;
it = a.entries();
- for (i = it.next(); !i.done; i = it.next())
- if (!b.has(i.value[0])) return false;
+ aSetLoop: for (i = it.next(); !i.done; i = it.next()) {
+ if (!b.has(i.value[0])) {
+ // NOTE: Modification to fix nested set issue in `fast-deep-equal`
+ // Add manual iteration of all set B values.
+ bIt = b.entries();
+ for (bI = bIt.next(); !bI.done; bI = bIt.next())
+ if (equal(i.value[0], bI.value[0])) continue aSetLoop;
+
+ return false;
+ }
+ }
return true;
}
// END: Modifications
diff --git a/test/node/tests.js b/test/node/tests.js
index 02336e5..3db4c74 100644
--- a/test/node/tests.js
+++ b/test/node/tests.js
@@ -64,9 +64,57 @@ const react = [
}
];
+const extraEs6 = [
+ {
+ description: 'Additional es6 tests',
+ reactSpecific: true,
+ tests: [
+ {
+ description: 'nested Maps with same values are equal',
+ value1: new Map([['one', 1], ['map', new Map([['two', 2]])]]),
+ value2: new Map([['one', 1], ['map', new Map([['two', 2]])]]),
+ equal: true
+ },
+ {
+ description: 'nested Maps with different values are not equal',
+ value1: new Map([['one', 1], ['map', new Map([['two', 2]])]]),
+ value2: new Map([['one', 1], ['map', new Map([['three', 3]])]]),
+ equal: false
+ },
+ // Fails in `fast-deep-equal`
+ {
+ description: 'nested Sets with same values are equal',
+ value1: new Set(['one', new Set(['two'])]),
+ value2: new Set(['one', new Set(['two'])]),
+ equal: true
+ },
+ {
+ description: 'nested Sets with different values are not equal',
+ value1: new Set(['one', new Set(['two'])]),
+ value2: new Set(['one', new Set(['three'])]),
+ equal: false
+ },
+ // Fails in `fast-deep-equal`
+ {
+ description: 'nested Maps + Sets with same values are equal',
+ value1: new Map([['one', 1], ['set', new Set(['one', new Set(['two'])])]]),
+ value2: new Map([['one', 1], ['set', new Set(['one', new Set(['two'])])]]),
+ equal: true
+ },
+ {
+ description: 'nested Maps + Sets with different values are not equal',
+ value1: new Map([['one', 1], ['set', new Set(['one', new Set(['two'])])]]),
+ value2: new Map([['one', 1], ['set', new Set(['one', new Set(['three'])])]]),
+ equal: false
+ }
+ ]
+ }
+];
+
module.exports = {
generic,
es6,
+ extraEs6,
react,
- all: [...generic, ...es6, ...react],
+ all: [...generic, ...es6, ...extraEs6, ...react],
};Could look to open either upstream or in our project.