Sync with latest master

[BridgeAR]

Node.js 8 added support for Set and Map and the performance improved a lot in general. Therefore it would probably be good to consider updating this again.

[sindresorhus]

This module was made so AVA could have a consistent base for users. We've since moved away from using the core assert methods for everything except for throws/notThrows, but we plan on moving away from those too. The core asserts are just too locked down and weird for our needs.

So what I'm saying is that I don't really have the time or motivation to update this, but I would gladly take a pull request.

[BridgeAR]

Interesting. Out of curiosity: what is weird about the assert module and how do you think should it behave instead so it would be more useful? I guess one of the things is that you can not get true / false back?

[sindresorhus]

Many things. I can't recall everything right now. For example, the assertions are loose by default, so many uses assert.equal instead of assert.strictEqual and encounter a lot of silent problems because of the looseness. The deep equal assertion is not what users would expect at all (We went through 3 different deep equal assertions in AVA before settling on the one we use now). But the main reason for not using it is that it's really only meant for use by Node.js core itself, but was exposed without much thought and now they can't remove it, and it's locked, so it can't change it much. There are much better assertion libs on npm.

[BridgeAR]

We are definitely on the same page that the loose equality is really bad in many ways. It should probably not be used at all and I am going to open a PR to doc-only deprecate it soon. When using strict equality it does behave nicely though.

One very important point about the assert module - it is not locked anymore and it got improved a lot already plus a few more things that will land soon. The changes include correctness, performance and feature improvements.

I had a look at concordance and the comparison done there is pretty nice in general and it is very similar to the way assert handles it now.

A few things that I personally would handle differently

• Compare Sets and Maps unordered
• Recursion should not stop when a circular ref is found but when both sides are equal / unequal

There is a point of doing that too though and I am thinking about having something like that optional (not for assert directly but for something new).

If I am correct these will be the only differences in deepStrictEqual compared to concordance soon.

I open a PR against core-assert when the mentioned PRs landed.

[sindresorhus]

Compare Sets and Maps unordered

But Set and Map are ordered.

[BridgeAR]

I quote MDN:

It should be noted that a Map which is a map of an object, especially a dictionary of dictionaries, will only map to the object's insertion order—which is random and not ordered.

[BridgeAR]

And the insertion order is something ignored while comparing e.g., plain objects.

[sindresorhus]

http://2ality.com/2015/10/property-traversal-order-es6.html

[BridgeAR]

I am aware of that change and the reasoning but this does not change that I still think it makes sense to ignore the insertion order.

[sindresorhus]

I agree it's probably the most common use-case to not want the order compared, but that could result in silent bugs when it actually does matter. Seems like there might be worth having an additional deepEqual assertion or an option to also compare order.

[sindresorhus]

// @novemberborn

[novemberborn]

I quote MDN:

It should be noted that a Map which is a map of an object, especially a dictionary of dictionaries, will only map to the object's insertion order—which is random and not ordered.

@BridgeAR What does that sentence mean? Is a Map "a map of an object", or are they referring to (somehow) creating a map of an object?

[sindresorhus]

It's now deprecated.