Jest-dom: toContainElement should accept null or undefined

[gnapse]

I got to this idea when I realized that is not always possible to negate .toContainElement:

expect(container).not.toContainElement(document.querySelector('.button'));

If indeed there's no element with class="button", the above will still fail. But not because the matcher finds the assertion to not be true. It fails because toContainElement does not allow null or undefined as argument.

I wanted to propose changing this matcher so that it will accept null and undefined.

[smacpherson64]

That is a good idea! What should the message say?

[smacpherson64]

@gnapse is the following what you are desiring for?

expect(container).toContainElement(null) // false
expect(container).toContainElement(undefined) //false
expect(container).not.toContainElement(null) // true
expect(container).not.toContainElement(undefined) // true

[gnapse]

Yes. I realize that it may look weird, and that's why I raised this as an issue first, to discuss it. But the thing is, it's weirder not to be able to negate a .toContainElement. Consider the following test around a component that loads data and you want to assert that it first renders a loading indicator, and then it removes it and renders data:

// render stuff
expect(container).toContainElement(queryByText('Loading...'));
// wait for mocked request to resolve
expect(container).not.toContainElement(queryByText('Loading...'));

You cannot do the above because toContainElement will fail even before attempting to assert anything. You'd have to use a different matcher for the negative case. I think is perfectly fine for toContainElement not to pass when given null, and that it passes when it's invoked in the .not form with null.

[jgoz]

@gnapse I agree. I have almost identical code to what you posted, except that the "not" case uses toBeNull now. It would be great to change it to your example, for clarity's sake.

[gnapse]

Ok, seems there is a consensus. I wonder if this is a breaking change? Also, I can take it later this week, but if anyone else wants to give it a crack, go for it.

[smacpherson64]

I can take a crack at this one! I think to be safe we should label it as a breaking change. Technically, the tests would perform the same, but the functionality is changing a bit.

[jgoz]

If you go by the type definitions, it is not a breaking change since it would simply involve allowing null and undefined where they were previously not allowed. Also, any previously usages of toContainElement(null) would be throwing errors, so it's unlikely that they are being used.

[jgoz]

Should this behaviour also extend to toBeInTheDocument?

expect(queryByText('Loading...') /* -> null */).not.toBeInTheDocument()

[gnapse]

Good point! I think it should too.

what shouldn't change is that toContainElement's container argument should really not be null or undefined:

expect(container /* should not be null or undefined! */)
  .toContainElement(element /* could be null or undefined */);