deepStrictEqual

[piranna]

Added deepStrictEqual as an alias for deepEqual

[keithamus]

LGTM.

@lucasfcosta?

[lucasfcosta]

Thanks for the PR, @piranna!
I thought Node's implementation was different, but I should've payed more attention.

As it is written in our .deepEqual implementation, it compares nested objects recursively, checking their keys one by one until it reaches primitives and does a strict equality check of the prototypes, I didn't think this was included I should've payed more attention to the deep-eql code. Sorry for confusion 😓

Are we going to enable this for the other interfaces too?
Maybe we should be able to write something like: expect(x).to.be.strictly.deep.equal(y). What are your thoughts about this? If we're not going to implement this possibility this LGTM 😄

[piranna]

As it is written in our .deepEqual implementation, it compares nested objects recursively, checking their keys one by one until it reaches primitives and does a strict equality check of the prototypes, I didn't think this was included I should've payed more attention to the deep-eql code. Sorry for confusion

Don't worry, the point of this suggestion was due to my employer requesting me to use chai instead of assert as I was doing, and since it was named deepEqual I though the tests would not be as strict as I was doing them before, I'm happy to know I was wrong :-D Now with the alias anyone can assume than in fact its strictly equal ;-)

Are we going to enable this for the other interfaces too?
Maybe we should be able to write something like: expect(x).to.be.strictly.deep.equal(y). What are your thoughts about this? If we're not going to implement this possibility this LGTM

I'm not sure... From my perspective the strict is part of the name because I use the assert style, but for the ones using the expect one don't know if there would be any advantage... :-/ I can't decide, I'm not used to expect style and don't know what I should expect (pun intended ;-P) about its behaviour.

[lucasfcosta]

@piranna yes, this new alias was a very good idea because it represents the real behavior in a better way especially for people coming from Node assertions 😄

Well, let's wait for @keithamus opinion on the strict word for other interfaces, if he thinks we don't need it we can merge this as it is.

Thanks again for your contribution and for sharing your point of view 🙌

[keithamus]

I don't think we need to add the .strictly keyword for other interfaces - as it doesn't actually do anything. This is just a small tweak to the assert interface to get a bit of parity with the node api - remove surprises for people moving from node's assert. If they're using our expect/should apis thats a pretty clear departure away anyway.

[lucasfcosta]

LGTM
Well, makes total sense, it's all good for me then! 😄

[piranna]

Thanks again for your contribution and for sharing your point of view

You are welcome :-) I'm a bit overheimet by this reception... :-P

[rtoal]

I'm glad to see this discussion happened and that a PR was made. I'm a bit confused by the naming however, and the decision to make an alias. Given that Chai distinguishes equal and strictEqual, then it would make perfect sense to distinguish deepEqual and deepStrictEqual. But this is not what happens. In fact, the reality is somewhat confusing:

chai.assert.equal("3", 3)
chai.assert.notStrictEqual("3", 3)
chai.assert.notEqual([3], [3])
chai.assert.deepEqual([3], [3])
chai.assert.deepEqual(["3"], [3])  // <---------FAILS!!!

This is because, as was pointed out on this thread, that deepEqual is already strict.

I know it's too late to change anything, and I also know that no one has ever (or should ever 😄) request a non-strict deep equals, but I gotta say, I agree with Piranna that the naming was confusing and I assumed at first glance that deepEqual was non-strict by default.

Can't wait for a release with deepStrictEqual because code looks a little better when that is made explicit. Overall, though, great job on the library.

[lucasfcosta]

Hi @rtoal thanks for your input! It makes total sense, but I think it would be more productive to have both, because as you've said it yourself deepEqual should always be strict. This is inherent to its definition.

Also, about the release with deepStrictEqual, it's already out! 🎉 It has been release under the canary tag.

You can install it by running:

npm install chai@canary

Thanks again for participating, our users and contributors are also responsible for the quality of this work, let us know if you have any further doubts or considerations 😄

[rtoal]

Thanks for the tip on the canary release. I had actually searched the page https://github.com/chaijs/chai/releases/tag/4.0.0-canary.1 for the text "653"; I should have actually just grabbed the release or inspected the code.

No other doubts; the project is fantastic overall.

The strict vs. non-strict is really a JavaScript issue, not a Chai issue, as we all know. If only Brendan would have called == similar and === equal but what's done is done. :)

[keithamus]

FYI technically our deep-eql algo is actually more strict than ===; well it has slightly different semantics I guess. We actually use Object.is for equality, meaning deepEql(-0, +0) === false (-0 == +0 && -0 === +0) and deepEql(NaN, NaN) === true (NaN != NaN && NaN !== NaN).