Repository navigation
Comparisons don't work as expected with the ES6 / ES2015 Set object #28
Description
Issue
I'm using the expect package from npm, which uses this package under the hood to check for deep equality. In my eyes, the test below should fail (different values and different references), but it is passing:
it('should fail', () => {
const actual = new Set('a');
const expected = new Set('b');
expect(actual).toEqual(expected);
});
If I did something like this with another collection (like arrays), the test wouldn't pass until 1) the references are the same with assert.equal / , expect.toBe or 2) the values are the same with assert.deepEqual / expect.toEqual
Background
I'm using Babel 5 to use Set. I'm using Node 5, so the Set being used should be native.
Am I doing something wrong, or does this look like a bug in the expect package in the way it handles Sets? I've cross posted a Stack Overflow question as I'm not sure.
I originally posted this on the expect package issues, but they sent me here, since that package uses this package ;)
mjackson/expect#47 (comment)
Activity
mjackson commentedon Nov 25, 2015
I guess it's ultimately up to you, @substack, whether you want to follow their lead or not. @domenic suggested that node's
assertmodule should only be used to test node, so it should be left as-is in core, and I agree. Until there's demand for it in core there's no need to include it there. It's not meant to be a general purpose assertion module.In this package, however, I think you do have a little more flexibility.
For starters, in the
assertpackage they currently follow core's lead as well so anyone who wants node core behavior can just userequire('assert').deepEqualand it will work the same everywhere. If people show up here and complain thatdeepEqualdoesn't work the same as in node, you can just point them to theassertpackage and tell them to use that if they want identical behavior.Additionally, including support for more native data types here is an enhancement, so it won't break existing code.
If you rename the repo from
node-deep-equalto justdeep-equal(same as the package) then it should convey more clearly to users that this is a general purpose equality comparison, not just for node.In any case, ultimately we'll need a
deepEquallibrary that knows about additional native types as they are added. They'll need it in core too, eventually.Just a few thoughts. Hope it helps.
domenic commentedon Nov 25, 2015
I'm not really sure in general that deepEqual is the right name for something that compares things based on their private state. You'd want something like "structurallyEqual" or "privateEqual" I think. Map, Set, Date, and Promise are all types which you could invent equality semantics for based on examining whether they have the same internal data, but "deep equal" traditionally means "has the same public properties" which will be true of any two Maps etc.
trevordmiller commentedon Nov 25, 2015
To sum things up: I need a way to write test assertions to compare the values in two ES2015 Sets. I really like @mjackson's
expectpackage, but I can use another package or build a helper comparison function if @substack doesn't want to support this; it just seems weird to me that there is no simple way to compare Sets, which are now native JavaScript objects 😜cc: @statianzo
mjackson commentedon Dec 10, 2015
Update: We don't have this issue anymore in the expect package since we switched to https://github.com/ljharb/is-equal. Thanks for your time :)
trevordmiller commentedon Dec 10, 2015
@mjackson So I can compare Sets in the same way I compare arrays? Awesome! Thank you.
svachalek commentedon May 20, 2016
I encountered this problem using tape today. While the node team decided to take their ball and go home, it seems that tape (and this package) are left on the lonely playground they call "userland".
It's probably small-minded of me but I can't see a good reason a test assertion would want to find all Set objects "equal", and it sure would be nice if this library did the Right Thing rather than imitate its namesake's failings.
mjackson commentedon May 21, 2016
Scott, grow up. Your tone here is ridiculous.
On Fri, May 20, 2016 at 4:40 PM Scott Vachalek notifications@github.com
wrote:
svachalek commentedon May 22, 2016
My tone is light and playful, but sincere. I can switch testing and/or assertion libraries but before I do that, I would like to add a vote on the side of usefulness over semantics. If Set seems like too much of a special case, consider solving it for the iterable protocol.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#iterable
SeeThruHead commentedon Jul 22, 2016
Is there anything in the works for this? I'm doing deepEqual on something that contains a map, having to run my expected and result objects through something that unwraps a map into it's array version is rather cumbersome. Maybe a separate assertion that uses isEqual?