Unexpected meaning of deep flag in property assertion

[meeber]

There's an inconsistency with the meaning of the deep flag. Consider these examples:

const obj = {a: 1};
expect(obj).to.deep.equal({a: 1});  // Currently passes thanks to deep equality
expect([obj]).to.have.deep.members([{a: 1}]);  // Currently passes thanks to deep equality
expect([obj]).to.deep.include({a: 1});  // Will pass thanks to deep equality once Issue #743 is fixed
expect({foo: obj}).to.deep.include({foo: {a: 1}});  // Will pass thanks to deep equality once Issue #743 is fixed
expect({foo: obj}).to.have.deep.property('foo', {a: 1});  // Fails due to using strict equality

The reason the last example fails is because the deep flag has a different meaning when applied to the .property assertion than it does when applied to any other assertion. Instead of switching from strict to deep equality, the deep flag unlocks the ability for the developer to use dot or bracket notation in their .property assertion:

const obj = {a: 1};
expect({foo: obj}).to.have.deep.property('foo.a', 1);

That's pretty cool, but the inconsistency of the deep flag creates a confusing developer experience and brings sadness to my soul. It also makes it impossible for other assertions such as .include to internally leverage .property by passing along the deep flag, as noted in #743 (comment).

I propose the following breaking change:

• Make the deep flag perform a deep equality comparison when used with the .property assertion.
• Add either a nested or inner flag to inherit the old behavior of the deep flag, allowing dot or bracket notation in the .property assertion.

Examples:

const obj = {a: 1};
expect({foo: obj}).to.have.deep.property('foo', {a: 1});
expect({foo: obj}).to.have.nested.property('foo.a', 1);

const obj2 = {a: {b: 2}};
expect({foo: obj2}).to.have.deep.nested.property('foo.a', {b: 2});

[lucasfcosta]

That's a great idea, I think this is the optimal choice since we can maintain both behaviors and allow users to chose whatever they want to do.
I have just taken a look at the code and it should be pretty easy to do, I think that maybe adding a nested flag and then checking for it whenever interpreting arguments and then using the deep flag only for deciding which equality to apply would be enough.
This should also be very clear into the docs, since it's a quite complex behavior.

I'm +1 for that. Awesome job.

[meeber]

The main problem is that this is a huge breaking change for anyone using deep.property currently. I think it's the correct change in the long term, but it'll be a hassle for anyone needing to replace .deep.property with .nested.property everywhere.

[lucasfcosta]

@meeber Yes, I totally agree, but I think that's the whole point of doing semantic versioning.
I've also read that at Facebook, for example, whenever doing breaking changes at React they try to find ways to enable developers to change code programatically all across the place and that's pretty much what is happening here. It's just a matter of running a RegExp and then replacing occurrences for nested since the behavior will be equivalent to the old deep.

IMO that's the cost of moving forward, and the behavior you just proposed looks much better than what we currently have, that was a great idea I would like to have access to if I were a user.

Whenever 4.0.0 gets released we can also prepare a migration guide for anyone willing to update an old codebase.

I also would love to hear what @keithamus thinks. 😄