there is nothing deep about `containsAllDeepKeys()`

[ancms2600]

repro: https://codepen.io/anon/pen/jaLWEo?editors=0010

gap in unit test coverage: https://github.com/chaijs/chai/blob/master/test/assert.js#L1077

[meeber]

@ancmikesmullin In Chai, the deep keyword consistently means "perform a deep equality comparison instead of a strict (===) equality comparison". That's the meaning used by the keys assertion, with the primary usage being comparing object-based keys of Sets and Maps using a deep equality comparison.

[ancms2600]

Oh, thanks. That is helpful to know. It seems unfortunate on the part of Chai, however. I argue the deep keyword is more commonly understood to mean recursive.

Exhibit A: https://lodash.com/docs/4.17.4#cloneDeep

I believe the word Chai devs were looking for is loose as in "loose equality" or containsAllKeysLoosely() or containsAllDeepKeysLoosely().

Exhibit B: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness#Loose_equality_using

Except that, since all keys are typeof "string", the comparison operation therefore takes place between two strings, therefore strict equality has no meaningful advantage here (e.g., containsAllKeys() == containsAllKeysLoosely())

Excerpt from the MDN article above:

Loose equality compares two values for equality, after converting both values to a common type. After conversions (one or both sides may undergo conversions), the final equality comparison is performed exactly as === performs it.

[ancms2600]

With this new and improved understanding, I now see Chai has like zero assertions that have to do with recursive analysis. I guess I'll do more than just complain, and offer this snippet of code which might be used for reference to consider a new line of recursive assertion tests.

const containsAllDeepKeys = (actual, expected, breadcrumbs="") => {
    for (let key in expected) {
        let breadcrumb = breadcrumbs +'.'+ key;
        if (!Object.prototype.hasOwnProperty.call(actual, key)) {
            throw new AssertionError(`${breadcrumb} is null or undefined.`, {
                actual: Object.keys(actual).sort(),
                expected: Object.keys(expected).sort(),
                showDiff: true
            });
        }
        else if ("object" === typeof expected[key]) {
            if (typeof actual[key] !== typeof expected[key]) {
                throw new AssertionError(
                    `${breadcrumb} is not an object.\n`+
                    `Expected an object with keys: ${Object.keys(expected[key]).sort().join(", ")}.\n`+
                    `Instead, got type ${typeof actual[key]} with value: ${actual[key]}`);
            }
            else {
                return containsAllDeepKeys(actual[key], expected[key], breadcrumb); // recursive tail call optimization
            }
        }
    }
};

[meeber]

At work so can't go into detail, but I think the deep equality algorithm in Chai consistently does what is expected in terms of a recursive equality algorithm. Obviously in terms of the keys assertion specifically there's ambiguity as to where the recursive equality algorithm takes place. Currently in the keys algorithm it only takes place when performing equality comparisons between the keys of Sets and Maps which can be objects, not just strings. A decision has to be made if the these object-based keys are to be compared with strict equality (===) or deep (recursive) equality. The default behavior is to use strict, and if the 'deep' flag is set to use deep equality instead. This behavior is consistent across Chai.

In your case, you don't care about Set and Map, you just want the key algorithm to apply recursively on objects, and an argument could be made that 'deep' should do that when the value given is an object instead of a map or set. But having this polymorphic behavior of the assertion often leads to new problems and confusions, so it's something that would really need to be analyzed and discussed.

And no, loose (==) equality doesn't come into play here.

[meeber]

At home now. So I think the best way to exemplify what's going on with .deep is with the recently rewritten BDD documentation, particularly this part.

One option would be to add an exception that when .deep is combined with .keys and the target is a non-Set non-Map object, then instead of performing a deep equality comparison, .deep instead performs the keys algorithm recursively. And I think that would make sense in isolation, especially since (as you noted), deep equality isn't even applicable in that case due to string-only keys. But I worry about how it impacts the greater Chai ecosystem in terms of consistency.

There's a history of .deep having different behavior based on which assertion it's paired with, and it used to cause problems. For example, .deep when paired with the .property assertion used to unlock a special syntax for referencing nested properties, as opposed to performing a deep equality comparison. It made sense in isolation, but due to the confusion it caused across assertions, the special syntax was moved to the .nested flag instead in Chai v4.0, and .deep was changed to perform a deep equality comparison when paired with .property. I'm reluctant to make any change that might reintroduce this kind of inconsistency across assertions.

In general, I also tend to be resistant to assertions that have different behavior depending on what type the target is. Take the .include assertion which does different things based on the type. This kind of auto-detection seems convenient on the surface, but is actually pretty dangerous for an assertion library to do. A bug in the user's program may cause the target to have a different type than expected, which may cause the assertion to silently apply different logic than desired, which creates the possibility that the assertion wrongly passes. In order to avoid this kind of bug, the user is required to add additional tests to first type-check their target, as documented here.

Finally, I wonder how common of a use-case it is to use the .keys assertion recursively. To my knowledge this hasn't come up before. I suspect that a .deep.equal assertion would suffice in many cases that a recursive .keys assertion would be considered. This makes me think this feature would be better suited for a plugin, at least until it becomes apparent that it's common enough to be included in core.

[ancms2600]

i am using assert.deepEqual() now. before i didn't want to validate the value just the key structure, but later i wanted to make it specific enough to be able to validate the value, as well. probably you're right that the former case is rare. may as well wait and see if anyone +1's this

[Eoksni]

I don't know if keys is the right word for it, but I think it is useful to have some way to check if two objects have the same tree structure (only keys checked, ignoring values). My use-case is like this - I have two translation files, each of them have correspondence between identifiers and translation strings, they are represented as json files:

en.json

{
  "signup": {
    "title": "My title in English"
  }
}

fr.json

{
  "signup": {
    "title": "My title in French"
  }
}

So here I want to check that every translation is present in both English and French.

I guess there also can be use-cases when you have an array of maps (or objects) and you want to make sure that this array is homogeneous in the sense that all these maps (or objects) have the same structure.

[lucasfcosta]

Hi @Eoksni, thanks for the feedback!

Your suggestion does seem like a valid and not uncommon use case, but I'm a bit concerned about how the syntax for it would look like.

Maybe something like:

// `same` would do the magic here
expect(enTranslation).to.have.same.deep.keys(frTranslation)

For now you can use this module which applies Object.keys recursively.

Also, what do other people think of this suggestion? Would you like to have this in the codebase? Is this a common/valid use case?

[Eoksni]

Yes, I think to.have.same.deep.keys is very descriptive of this use-case.

[keithamus]

Thans for the issue @ancmikesmullin.

We're going to be looking at all assertions a bunch in chai 5, to make sure the are very obvious and don't have hidden surprises. Hopefully chai 5 won't make you reach for Snape memes! (Alan Rickman may he rest in peace).

Anyway, this will be implicitly fixed by chai 5. We'll close this for now as there is nothing really actionable from this, other than "look a bunch at our API and make sure it makes sense".