Tape: comparing empty array vs empty object with deepEqual

[tcoopman]

doing t.deepEqual({}, []) evaluates as true. I would expect it to be false.

[ljharb]

assert.deepEqual({}, []) also passes - this is admittedly odd, since the "length" property isn't on the object, but all of their enumerable own properties are indeed equal - consider assert.deepEqual({0:1}, [1]). If you want to compare types as well, you'd want a combination of Array.isArray, typeof, etc.

[xeodou]

@ljharb Thanks for your work on this project.
Can i ask how is going on for this ticket, it's kind of wired for t.deepEqual({}, []),even they are not the same type in meaning. I know they are both object but in other words they totally different.

[ljharb]

deepEqual isn't meant to be a strict comparison. Since assert.deepEqual({}, []) passes as well, I think it's best for tape to stay in line with assert's API.

[xeodou]

@ljharb Thanks. That's helpful.

[jmm]

@ljharb

deepEqual isn't meant to be a strict comparison. Since assert.deepEqual({}, []) passes as well, I think it's best for tape to stay in line with assert's API.

It's worth noting that t.deepEqual() _already_ deviates from assert.deepEqual()'s API. Example:

var assert = require("assert");
var test = require("tape");

var a = [1];
var b = ["1"];

test(function (t) {
  assert.deepEqual(a, b, "not assert.deepEqual");
  t.deepEqual(a, b, "not t.deepEqual");
});
// not ok 1 not t.deepEqual

For the other readers it's also notable that the deep equal logic does not actually live in this package, FWIW.

Also, Node has assert.deepStrictEqual() that would throw here, and at least one implementation on npm: deep-strict-equal. (I presume it works in the browser?) However, even that wouldn't reject sparse arrays that only differ by length, example:

var a = [1];
var b = [1];
b.length = 2;

^^ In reference to:

this is admittedly odd, since the "length" property isn't on the object

[jmm]

Oh yeah, forgot to mention why I wound up here in the first place: there are other similar cases, such as t.deepEqual({}, function () {}). Again, assert.deepStrictEqual() rejects that (because it compares prototype equality). EDIT: nope, my mistake -- I had something screwed up.

[ljharb]

It sounds like the only issue here is that tape's deepEqual method works like assert.deepStrictEqual, but not the same as assert.deepEqual?

If that is the only deviation from assert, then the remaining question is "should we add deepStrictEqual to tape and make deepEqual loose? Or, should leave it as-is?

[BridgeAR]

@ljharb the deep-equal package is pretty much a copy of a very old Node.js deepEqual version but it was not updated for a very long time now. Quite a few things were improved in the original version since then.

@substack would you mind updating that? I would also be willing to open a PR for it if someone looks into it.

[gitowiec]

I wish someone could clean up NPM repository from such modules. Knowledge in this thread is useless for me. And from the newbe person point of view (such as me) this package is faulty (by copying behaviour of faulty one). Faulty packages shouldn't be hosted on public registry many depend on.

[ljharb]

@gitowiec there's nothing faulty about this package, and there's no need to be hostile.

[ljharb]

With v5, and deep-equal v2, tape now matches node's assert.