.toExcludeKeys not working as expected

[5c077yP]

Hey,

thanks for this great library.

I just came across .toExcludeKeys. And for me it feels that something is wrong here. The docs say:

does not contain any of the provided keys

Which sounds to me, if there is at least one key in the provided keys which is contained in the given object, it should fail.

But then on the other side there is even a test like this

it('does not throw when even one key does not exist', () => {
  expect(() => {
    expect({ a: 1, c: 3 }).toExcludeKeys([ 'a', 'b', 'c' ])
  }).toNotThrow()
})

That looks more like, there needs to be at least on key in keys which is not in object and then it's fine.

Thanks.

[wilkesreid]

I would read it as

Expect said object to exclude keys "a", "b", and "c"

So the fact that the object only excludes key 'b' means the assertion fails.

It makes sense to me.

[ljharb]

I do see the value in having both toExcludeAnyKeys and toExcludeAllKeys. Perhaps it's worth adding the "any" version?

[sfrdmn]

The issue, though, is that the code currently implements toExcludeAnyKeys whereas the expectation is that it implement toExcludeAllKeys

[sfrdmn]

And yeah, via the docs:

does not contain any of the provided keys

Which should be interpreted to mean: for all provided keys, there exist none which are properties of the object. This is definitely not satisfied by the test posted by @5c077yP

[ljharb]

The tests are the source of truth here.

The documentation says "does not contain any of the provided keys" - so it seems the documentation needs updating, to something like "is missing all of the provided keys".

If you want toExcludeAnyKeys behavior - ie, where it will throw if any of the provided keys is not present - then you'd want toNotIncludeKeys - which seems like something worth adding.

[sfrdmn]

But it seems toNotIncludeKeys is already an alias of toExcludeKeys?

[ljharb]

I would see toNotIncludeKeys as something that should throw if any key was included - whereas toExcludeKeys throws only if all keys are included.

[sfrdmn]

That currently isn't the case:

expect/modules/Expectation.js

Line 532 in 5ede172

toNotIncludeKeys: 'toExcludeKeys',

Also, did you mean:

whereas toExcludeKeys throws only if all keys are excluded

?

[ljharb]

No, toExcludeKeys throws when all provided keys are NOT excluded - it passes (ie, does nothing) when all provided keys are excluded.

I didn't realize the alias already existed.

Given that, I think we need toExcludeAnyKeys/toNotIncludeAnyKeys and toIncludeAnyKeys/toNotExcludeAnyKeys etc.

[sfrdmn]

Ah, right sorry. Too many double negatives. 😂 I was thinking "passes" not "throws"

Seems I don't get the naming. For me:

toExcludeAllKeys means "for all provided keys, there exists no corresponding property on the wrapped object"

and

toExcludeAnyKeys means "there exists at least one provided key for which there is no corresponding property on the wrapped object"

toExcludeKeys seems currently to implement the latter

[ljharb]

I think we're using conflicting forms of "any" and "all" here.

To me, "toExcludeAny" means that if any of the keys exist on the object, it throws - if none exist, it passes. In other words, if any of the given keys exists, it's violating "excluded".

To me, "toExcludeAll" means that it only throws when all of the given keys are not excluded.

Either way the naming is confusing, but https://github.com/mjackson/expect/blob/master/modules/__tests__/toExcludeKeys-test.js#L28-L38 implements "it only throws when all excluded keys exist" - and what we want to add is "it throws when any excluded key exists".

[wilkesreid]

expect([a: 0, c: 0]).toExcludeAnyKeys(['a', 'b', 'c'])

Could be read as:

Expect [ a: 0, c: 0 ] to be missing at least one of the following keys: [ 'a', 'b', 'c' ]

This should pass, because [ a: 0, b: 0 ] excludes the key b.

---

expect([a: 0, c: 0]).toExcludeAllKeys(['a', 'b', 'c'])

Could be read as:

Expect [ a: 0, c: 0 ] to be missing all of the following keys: [ 'a', 'b', 'c' ]

Should throw (not pass), because [ a: 0, c: 0 ] only excludes b, but does not exclude a or c

[sfrdmn]

@wilkesreid Right, that's in agreement with what I was saying, but not in agreement with @ljharb's comment just before yours, which gives the opposite definition

IMO, the behavior should be the same as in the list processing functions all (every) and any (some) in many programming languages (assuming the predicate to be "key is excluded")

[ljharb]

I agree - all/every and any/some should mean those things - but although the wording of the documentation for "toExcludeKeys" is confusing, it does not in fact inherently conform to either concept "all" or "any" solely by virtue of its name.

[sfrdmn]

@ljharb For sure :) I was just trying to sort out how we're defining toExcludeAnyKeys and toExcludeAllKeys