Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: isVisible weird behavior for 2.3.2 version #2016

Closed
andinh-minacolor opened this issue Mar 27, 2023 · 9 comments
Closed

Bug: isVisible weird behavior for 2.3.2 version #2016

andinh-minacolor opened this issue Mar 27, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@andinh-minacolor
Copy link

Describe the bug
I have been using version 2.2.6 for a while and it works great then I tried to update this module to 2.3.2 and got the issue with isVisible function behavior. As provided below, after accessing isVisible for first time it's value will not be changed later even the virtual DOM has been updated correctly.

To Reproduce
This is the component

<template>
  <div v-show="show">
    <span />
    <button @click="onClick"></button>
  </div>
</template>

<script setup lang="ts">
import { ref } from "vue";

const show = ref(true)

const onClick = () => {
  show.value = !show.value
}

</script>

This is the test

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

test('isVisible', async () => {
  const wrapper = mount(check)
  expect(wrapper.find('span').isVisible()).toBe(true)
  await wrapper.find('button').trigger('click')
  console.log("***> wrapper", wrapper.html())
  // this value will always be true, but if I remove the isVisible code above it will be false.
  expect(wrapper.find('span').isVisible()).toBe(false)
})
@andinh-minacolor andinh-minacolor added the bug Something isn't working label Mar 27, 2023
@cexbrayat
Copy link
Member

Hi @andinh-minacolor

I think this is because of a change in jsdom v21: some properties are now updated only if the element is attached to the DOM.

Try adding mount(check, { attachTo: document.body }), and let us know if that fixes the issue.

@andinh-minacolor
Copy link
Author

hi @cexbrayat
Many thanks for your reply. Yeah it is JSDOM issue. Your code provide a fix.

@ikex
Copy link

ikex commented Apr 6, 2023

Hey.

According to the jsdom issue it seems that jsdom behavior is going to stay and works "as expected". For the time being we are using a fix - attachTo: document.body... but I wonder... is there any long-term plan to have it... fixed/implemented on the test-utils side? Or explicit attachTo is the solution we should have?

Is there a specific issue that tracks this and the one I should follow?

Thanks :)

References:
jsdom/jsdom#3502

@cexbrayat
Copy link
Member

We can't fix it on VTU side, so yeah, the solution is to use attachTo 👍

@alecgibson
Copy link
Contributor

I feel like this should be documented somewhere, or ideally even throw if you're trying to use isVisible() on a wrapper that isn't attached to the DOM.

@cexbrayat
Copy link
Member

@alecgibson Great idea: would you mind opening a PR to add a note to the isVisible docs? That would be a great help

alecgibson added a commit to alecgibson/test-utils that referenced this issue Jul 5, 2023
This change updates the documentation to reflect that `isVisible()`
will only work correctly [when attached to the DOM][1] because of an
upstream [`jsdom` issue][2].

[1]: vuejs#2016
[2]: jsdom/jsdom#3502
@alecgibson
Copy link
Contributor

@cexbrayat I've raised a docs PR here: #2122

Would there be any appetite for throwing when trying to use this on an element not connected to the DOM? I always like my frameworks to shout at me if I'm using them incorrectly, but I acknowledge that trying to throw in this case is not necessarily as simple as it may seem.

cexbrayat pushed a commit that referenced this issue Jul 5, 2023
This change updates the documentation to reflect that `isVisible()`
will only work correctly [when attached to the DOM][1] because of an
upstream [`jsdom` issue][2].

[1]: #2016
[2]: jsdom/jsdom#3502
@cexbrayat
Copy link
Member

Thanks for the PR 👍

I'm not sure about throwing, because it works properly in many cases, and the behavior is only different with recent jsdom versions. We talked about attaching to the DOM by default, but we did not go further for now.

@alecgibson
Copy link
Contributor

it works properly in many cases

As a consumer it's hard to know what these cases are, though (and I prefer to know it will always work). Attaching by default sounds relatively sensible (if potentially breaking...?), but if that's the stance that's fine; we'll work around it by patching or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants