toBeVisible should imply toBeInTheDocument

[calebeby]

Describe the feature you'd like:

I think that it would make sense for toBeVisible to perform, in addition to its CSS-based checks, checks that confirm that the element is in the document.

test('toBeVisible with removed element', () => {
  document.body.innerHTML = "<div id='el'>content</div>"
  const el = document.getElementById('el')
  expect(el).toBeVisible()
  el.remove()
  expect(el).not.toBeVisible() // this line fails, element is "visible"
})

If we think about it from a CSS standpoint, yes, there is no CSS that is preventing the element from being visible. But from the user's perspective, the element is definitely not visible. For this reason I think it makes sense to change the behavior of .toBeVisible to include the checks from .toBeInTheDocument.

[gnapse]

Makes sense. Are you up to contribute this fix? If not I may get to it next week.

[calebeby]

Yep, I can make a PR. One other thing I have a question about:

.toBeInTheDocument has a special case for null values:

expect(null).not.toBeInTheDocument() // passes

Should .toBeVisible also have that case?

expect(null).not.toBeVisible()

[gnapse]

toBeInTheDocument has that special case because a common use is to use a queryBy* or even document.querySelector and if they fail to return what you queried for, .toBeInTheDocument's most expected behavior would be to indeed let it pass, because that means the queried element was not found.

However, I'd say toBeVisible should not do the same. Can you check if it is letting it pass today? I believe it isn't, and in that case even more so, it should not change its functionality in that way.

[calebeby]

Yeah right now expect(null).not.toBeVisible() does not pass.

[gbirke]

Patch #339 broke some Vue tests for me. The tests look like this:

const wrapper = mount( SomeComponent, {localVue} );
let myInput = wrapper.find( 'input#foo' );
expect( myInput.element ).toBeVisible();

With version 5.11.10 I get "Received element is not visible (element is not in the document)" errors.

Using Vue 2.6.11 with @vue/test-utils 1.1.3

[gnapse]

@gbirke what's mount and wrapper in your example above?

Normally this library is used alongside the corresponding testing-library for the frontend framework you use. If you're using Vue, then you'd be using https://github.com/testing-library/vue-testing-library, and if you are, then the way to render a component using that library looks different than in your example. At least according to what is documented in https://github.com/testing-library/vue-testing-library#a-basic-example

Can you clarify that before we can discuss this further?

[gnapse]

I re-read your message and deducted that this what you mention as @vue/test-utils. I'm not sure how this testing library works. But I am assuming the problem may be because this library does not mount its root element inside a document. In which case there's little we can do. jest-dom assumes elements are living inside an actual document.

It would help if you created a minimal repo in codesandbox to investigate.

[gbirke]

I have created a minimal setup with vue-cli, adding the unit test module. Then I installed jest-dom and wrote a test that checks if the h3 element is visible. You can find it here: https://codesandbox.io/s/fuhrf

If you add console.log to the test, you will see that h3.element.getRootNode({composed: true}) doesn't return the document node (instead, it returns a Vue node), causing const isInDocument = element.ownerDocument === element.getRootNode({composed: true}) in isVisible to fail.

Checking for visibility with jest-dom is the recommended way of doing this, accoring to the Vue testing documentation

[gnapse]

Looking at that linked documentation, shouldn't you be using isVisible?

import { mount } from '@vue/test-utils'
import Foo from './Foo.vue'

const wrapper = mount(Foo)
expect(wrapper.isVisible()).toBe(true)
expect(wrapper.find('.is-not-visible').isVisible()).toBe(false)

Like I said, it seems that testing library operates in a way that is incompatible with how jest-dom works. jest-dom assumes and expects that the DOM structure being tested is attached to a host document.

[GadQ]

@gnapse Thank you! I wanted to report an issue, but you're totally right and it worked!

[gbirke]

Quoting from the linked documentation:

isVisible is deprecated and will be removed in future releases.
Consider a custom matcher such as those provided in jest-dom

That's why I set up the test the way it is. You can raise an issue with @vue/test-utils to un-deprecate isVisible or explain the situation with element.ownerDocument === element.getRootNode({composed: true}).

For now I'll just stick with version 5.11.9 of jest-dom. If there are important updates to jest-dom I might use the testing library with render instead of using mount, but as long as @vue/test-utils is the recommended way of testing vue components, I'd rather not.