Skip to content

assert: Add support for Map and Set in deepEqual

Rodrigo Muino Tomonari requested to merge github/fork/josephg/assert-sets-and-maps into master

assert.deepEqual and assert.deepStrictEqual currently return true for any pair of Maps and Sets regardless of content. This patch adds support in deepEqual and deepStrictEqual to verify the contents of Maps and Sets.

Unfortunately because there's no way to pairwise fetch set values or map values which are equivalent but not reference-equal, this change currently only supports reference equality checking in set values and map keys. Equivalence checking could be done, but it would be an O(n^2) operation, and worse, it would get slower exponentially if maps and sets were nested.

Update: Based on conversation below, the implementation now incurs an O(n^2) cost if the sets or maps are not equal, because to be correct we need to check all pairs of values in sets, and all pairs of keys in a map. This may become exponentially more expensive with input which does deep equality checking on nested maps. But given these checks are almost always only done during testing, and on small objects its probably the right approach.

Update 2: Based on a clever suggestion by @TimothyGu the O(n^2) cost is now only paid when you're either in non-strict mode or when you're using set values & map keys which have typeof object.

Note that this change breaks compatibility with previous versions of deepEqual and deepStrictEqual if consumers were depending on all maps and sets to be seen as equivalent. The old behaviour was never documented, but nevertheless there are certainly some tests out there somewhere which depend on it.

Support had been stalled because the assert API was frozen, but assert was recently unfrozen in CTC#63

Fixes: https://github.com/nodejs/node/issues/2309 Refs: https://github.com/substack/tape/issues/342 Refs: https://github.com/nodejs/node/pull/2315 Refs: https://github.com/nodejs/CTC/issues/63

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)
  • Assert

Merge request reports