Repository navigation
assert.deepEqual & post-ES5 Data Types e.g. Map, Set, Iterables, etc #2309
Description
Activity
I guess a major issue is that there are many interpretations of what "deep equals" even means and perhaps making something sensible for ES6 data types would be inconsistent with the existing algorithm.
asserttreats objects as key-value pairs. I don't think we should change that. This is something that can be done in a million different ways so it's probably better to leave that to user land. Also using it in a test is usually a bad idea, since for maintaining backwards compatibility you should check that specific properties exist and no that nothing else doesn't exist.One interesting idea that requires some sort of standardisation is symbol properties that provide custom identity and equality functions (like Java
equalsandhashCode), but it's up to TC39.assert treats objects as key-value pairs
Not all objects though, native Object types that existed in ES5 are special cased:
https://github.com/nodejs/io.js/blob/2a7fd0ad328d5197a9a166650e9eaa51367e3152/lib/assert.js#L152-L165SetandMapare no more or less special thanDateorRegExp.Also using it in a test is usually a bad idea
Note that the tests for Node itself uses
deepEqualextensively.I agree that it probably should be updated to properly deeply evaluate into ES6+ data types.
Not all objects though, native Object types that existed in ES5 are special cased
That is true, my bad. But these are not container types.
Note that the tests for Node itself uses deepEqual extensively
They use it to compare maps and lists of maps mostly. Otherwise it's possible to get unexpected results
Copying in @domenic's comment regarding this from #2315
I don't think we should do this (or anything else in #2309). Assert should remain used only for testing io.js itself, and not try to be a good general assertion library. If io.js tests need this ability enough times that we need to factor it out, we should, but until then, just adding it because it'd be nice isn't a good idea IMO.
The TSC decided this week to lock the assert API and there's a pull request that will likely land in the next several hours to update the documentation to that effect and to suggest that people use userland assertion libraries. Given that, I think it's appropriate to close this issue, but if anyone feels differently, by all means, re-open.
This is disappointing. I enjoy the simplicity of using the built-in
assertlibrary, and the semantics for what deep equal means are well defined for sets and maps (a.size === b.size && everything in a is in b). And I don't see how this change would break backwards compatibility - the behaviour of assert in the face of ES6 types was always undefined.Time to go trawling through npm.
Reacted by Jayden Seric@josephg The info in this issue is out of date. Subsequent to this decision, the locking of the API was reversed. (In fact, no APIs are locked anymore.) Any common-sense improvements to behavior of existing
assertmethods (such asassert.deepEqual()) are likely to be accepted, IMO.Reacted by Chris DeacyGreat, yeah I just read inspect-js/node-deep-equal#28 . If nobody else has done it I'll put together a PR to fix this behaviour.
- added a commit that references this issue
on Apr 3, 2017
Currently
assert.deepEqualsis ignorant of any new native data types introduced in ES6.For example, this should probably throw, yet it does not:
A good method might be to
deepEqualagainst.keys()and.values().deepEqualshould also probably work with generic Iterables too.