Comment not allowed in `toBeEmptyDOMElement`

[ycmjason]

• @testing-library/jest-dom version:5.11.10
• node version: 14.15.4
• npm (or yarn) version: -

Relevant code or config:

expect(document.createComment()).toBeEmptyDOMElement()

What you did:

Trying to test a component rendered as comment to be empty.

What happened:

received value must be an HTMLElement or an SVGElement.
Received has type:  object
Received has value: <!---->

Suggested solution:

Maybe we should rename toBeEmptyDOMElement to toBeEmptyDOMNode so we allow all html nodes (including text nodes and comments) to be passed into expect().

[gnapse]

Thanks for your report.

This library is focused in the end user's experience, and they do not experience comments in the html. So I'd be hesitant (to say the least) to introduce a breaking change by renaming and changing the behavior of a custom matcher for this reason.

I wonder why you want to test comments in your html. Maybe if you expand a bit more in your use case.

[ycmjason]

sure. for instance vue renders comment when a component return null in the render function.

I would like to test that the component renders to "empty" at a certain condition.

i hope this help explain why i need this.

[gnapse]

Can't you render the component inside a wrapper element, and test that the wrapper element is empty?

[ycmjason]

well i can but that would be a work around because comment node is not supported. it doesn't make sense to add a wrapper conceptually. i think we should be able to test comment node directly..

i respect your decision tho! :)

[gnapse]

Hey, good to understand the use case a bit more. However, as you acknowledge to understand, we cannot change the name of the matcher for this. It already even says in the name that it expects an element and not a node. If at all, we should maybe have a new matcher for nodes, but I'm also hesitant (though more open) to even do that.

I even wonder, how do you query for that comment node so as to have a reference to it that you could pass to a hypothetical .toHaveEmptyNode? How do you query, when testing a vue component, to obtain this comment node?

[ycmjason]

Well. In the vue test util, when you mount a component, the "wrapper object" has an element property which points to the root element.

The root element would point to a comment if the render function returns null.

Doc for reference: https://vue-test-utils.vuejs.org/api/wrapper/#element

(accidentally closed the issue as i was posting this comment 🤣)

[gnapse]

So why does the type in that documentation is HTMLElement? I think a comment or text node is not something that could be of type HTMLElement.

Anyway, at this point I'm just learning vue and not doing anything to try to solve this issue. My stance (maybe as someone that does not use vue thus does not suffer this issue) is to try to have an element to test on.

This library (jest-dom), although technically independent, is aimed at being paired with the framework-specific testing-library variants. That is, it is built thinking that in the case of vue, you'd use it alongside @testing-library/vue. In which case you'd not be using this wrapper and you'd be using the results from calling render from that library, which gives you a container element all the time. So you could do it like this:

import {render, screen, fireEvent} from '@testing-library/vue'
import ComponentThatRendersEmpty from './ComponentThatRendersEmpty'

test('increments value on click', async () => {
  const { container } = render(ComponentThatRendersEmpty)
  expect(container).toBeEmptyDOMElement()
})

I understand if you do not use this system of testing, but this is what jest-dom was built for. And we generally do not want to deal with testing comment nodes, as they are irrelevant for the end user.

[adi518]

I don't understand the reasoning behind excluding comments. Whatever doesn't render anything is considered empty.

[gnapse]

Comments are not elements, they are nodes. And this matcher, for better or worse, was designed to target html elements. It used to be called .toBeEmpty() and we needed to rename it because of the possibility of custom matchers name clash (see #216). Now the name kinda suggests it works only on elements, and not nodes.

We could introduce a .toBeEmptyDOMNode, and that one could be designed to target any node, including elements. And we could eventually deprecate .toBeEmptyElement. I'm not sure to go down that path though. I wonder why we'd need to target comment nodes to check that they're empty. Comments is not something that the user perceives. But I'm open to be convinced otherwise. What are good arguments to provide the ability to target nodes? Ideally good arguments that consider the guiding principle of testing-library.