Vue-test-utils: Using a non deprecated function should not print a deprecation warning

Created on 6 May 2020  ·  29Comments  ·  Source: vuejs/vue-test-utils

Version

1.0.0

Reproduction link

https://github.com/maxarndt/repro-vue-test-utils-deprecation-warning

Steps to reproduce

  1. Clone repro repo
  2. npm i
  3. npm test

What is expected?

Using a non deprecated function as .destroy() should not print deprecation warnings.

What is actually happening?

Using the above mentioned function prints a deprecation warning.


The implementation of .destroy() needs to be updated to use the successor of isVueInstance.

enhancement

Most helpful comment

I was thinking about this a bit recently too. I think some of our deprecations make perfect sense, but this one a little bit too aggressive. The general idea was "don't reinvent the wheel, leave it up to your test runner" where possible.

If you really enjoy this method, you could just add it back in yourself; import { Wrapper } fro '@vue/test-utils' and modify the prototype. We actually have a plugin system in V2 of this library to make it easier to add your own methods.

That said, we do have exists for v-if, having visible for v-show might not be the worst idea. I really like the parallel of v-if -> exists, v-show -> visible.

I think the other deprecations still make sense, though. I would like to get another opinion from @afontcu or @dobromir-hristov, and finally if anyone else is reading this and feels strong about porting visible from jest-dom to VTU, please thumbs up this.

All 29 comments

We can also add isVisible to this list (as it uses isEmpty for which a deprecation warning is thrown).

The error also has a mistake in it: "isVueInstance is deprecated and will removed in the next major version" should be "isVueInstance is deprecated and will be removed in the next major version"

Honestly I'm disappointed by this release, it feels like their own code wasn't tested?

Ah, I forgot we call isVueInstance in destroy. I will fix this one now.

Ideally we would have moved into release candidates to avoid missing things, but after literally years in beta and about 12 months of no-one maintaining the repo, and with Vue 3 in the near future, it felt a bit silly to drag it out any longer.

Please review so I can do a patch: https://github.com/vuejs/vue-test-utils/pull/1533

This is fixed in the latest version (1.0.2). Try installing that one.

Lmk if any others come up. And yes, we should have had better tests around the deprecations. Focus has largely been elsewhere, mainly around Vue 3, but that's no excuse. Let's make sure we keep the standards high.

Thanks I'll give it a whirl in a bit to see if it's resolved.

I appreciate all the hard work though, just found it odd that the internal build has these warnings suppressed on purpose, that seems to sort of defeat their purpose a bit ;)

Cool. LMK if other things sneaked through.

So far it's not going too well, it looks like some of the deprecations don't have any solutions.

Example:
```js
// This throws 2 warnings: find, and isVisible
expect(wrapper.find(Loader).isVisible()).toBe(true)

// This fails, because findComponent returns an object, and toBeVisible (as suggested by vue-test-utils) expects markup
expect(wrapper.findComponent(Loader)).toBeVisible()

Hm, tricky one. I always search for a DOM node and assert against that, so I did not catch this case. The two deprecations make sense individually but not when shown together.

The general idea moving forward to favor finding and asserting against DOM nodes, rather than components (DOM node are more reliable - there are, and likely always be, some edge cases with finding components).

Edit: people smarter than me pointed out expect(wrapper.findComponent(Foo.element)).toBeVisible(). I will update the deprecation to suggest element as well.

https://github.com/vuejs/vue-test-utils/pull/1534 This should improve the message @TheDutchCoder

Ah ok, .element makes sense, as I was confused why there was such an emphasis on finding components over DOM nodes before and now that thought has switched?

I'm okay with either, as long as I can write tests of course :)

Thanks for the heads up, I will upgrade and update my tests.

Maybe this is related. But when I'm running this test:
expect(wrapper.find('#element-id')).not.toBeVisible();
im getting the error: TypeError: expect(...).not.toBeVisible is not a function

I'm trying to apply as stated in the deprecation warning, but i'm not getting any further? Can anyone help?

Hi @p0psicles! toBeVisible is part of jest-dom, you might want to install that library to use its assertions.

As you might already have noticed I'm not as well versed in front end testing as you guys. The documentation on using jest-dom icw vue-test-utils is really minimal. As it's mostly focused on react. Can anyone redirect me to some valuable docs on this? I was doing fine until the deprecation warnings popped up.

@p0psicles no problem!

the deprecation messages are just that – deprecation messages. Your tests will keep doing fine for a while, you don't need to worry about them right now!

That being said, jest-dom is framework-agnostic, so there's nothing on it that makes it React-specific. After you install the library, its matchers become available so you can write expect(...).toBeVisible().

Hope this helps 🙌

If you're here looking for a transition for
[vue-test-utils]: isVisible is deprecated and will be removed in the next major version. Consider a custom matcher such as those provided in jest-dom: https://github.com/testing-library/jest-dom#tobevisible. When using with findComponent, access the DOM element with findComponent(Comp).element.
Let me summary above comments and hope it could save you some time

// old assertion
expect(wrapper.find('.selector').isVisible()).toBeTruthy() // .toBe(true)

// new assertion
import '@testing-library/jest-dom' // leave it in the top of the file or in the test setup file
expect(wrapper.find('.selector').element).toBeVisible()

You should have @testing-library/jest-dom installed.

Yeah with the help of you guys I figured this out.
What does trigger my curiousity is why this Method is removed from vue-test-utils. What is the direction of this Utility?

As @lmlife2016 already said (I hardly remember where and when 🤣), the team wanted to focus on the core values of cli-test-utils.

Hi! Getting visibility right is _harder than expected_, and jest-dom is already doing a solid job. As @hiendv just said, we decided that focusing on Vue-related stuff was smarter than re-inventing the wheel :)

Right decision IMO :+1: When #1557 lands in the official docs, the transition should be smooth. Keep up the good work, guys.

Thanks @hiendv, I added your example to the migration docs.

@lmiller1990 Thank you :D
Anyway, I do notice some deprecation warnings mention this

When using with findComponent, access the DOM element with findComponent(Comp).element`.

This is true but find({string} selector) also returns an object so .element is also needed for find.

Oh, I see. If you have time it'd be great if you can make a PR improving that message. I will do a release soon.

Hi, can you please not deprecate the function? jest-dom is not test framework agnostic (i.e., you need jest already to use it; ava tests will break). The bigger risk is in breaking tests that currently exist that use the function. Perhaps just leave the warning message in with a user's ability to mute the warning?

Can I suggest just using the exact same code from jest-dom in vue-test-utils? That way people don't need to go install another dependency for this. One of the things I like about vue-test-utils it's just one install and pretty much most of my vue testing needs are met. I don't want the abilities to just disappear. Even if methods are imperfect, I can live with imperfect.

https://github.com/testing-library/jest-dom/blob/e4d61c2ef16018197c316135f57f905bf5b2ca2a/src/to-be-visible.js#L26

I was thinking about this a bit recently too. I think some of our deprecations make perfect sense, but this one a little bit too aggressive. The general idea was "don't reinvent the wheel, leave it up to your test runner" where possible.

If you really enjoy this method, you could just add it back in yourself; import { Wrapper } fro '@vue/test-utils' and modify the prototype. We actually have a plugin system in V2 of this library to make it easier to add your own methods.

That said, we do have exists for v-if, having visible for v-show might not be the worst idea. I really like the parallel of v-if -> exists, v-show -> visible.

I think the other deprecations still make sense, though. I would like to get another opinion from @afontcu or @dobromir-hristov, and finally if anyone else is reading this and feels strong about porting visible from jest-dom to VTU, please thumbs up this.

I agree. I appreciate your response and reasoning. Thanks for reconsidering.

@lmiller1990 while we are here speaking about deprecations - could we possibly undeprecate .is() for checking against Component class (not the tag). It is extremely useful when checking that dynamic component was resolved to correct class, and workaround is very ugly

That is a pretty interesting use case - I didn't think about this one at all.

Proposal:

  • port this and bring back visible. This is not perfect but we should have something good for v-show testing. Custom matches are good for fancy things, but v-show is not exactly an unusual use case.
  • I don't really have a strong opinion on is. Personally I'm not a fan of specific component assertions like that, but implementing is in user-land is extremely difficult because it uses matches (see here), so this seems like a strong enough reason to bring it back.

If someone wants to make a PR with these two (or even just one) I think that'd be fine. I think we (the VTU team) had the right idea with some deprecations - I think some of them make sense - but we definitely should have put these through a more rigorous RFC process. Sorry about the inconvenience that we could have avoided with a bit of planning.

What do you all think?

@lmiller1990 I know the VTU team are not big fan of shallowMount :) neither part of GitLab is :). But what we agree (in GitLab) that shallowMount + is is important in "documenting" component intentions like "it should render component Foo" under certain conditions

I'll open PRs for this for review :)

Sounds good. I think we can all agree that the ultimate goal of this library and testing in general is to give you confidence in your application. If is enables you to be more confident in your application, we should include it 👍

@lmiller1990 I believe we could close this one as relevant MRs were merged!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

robcresswell picture robcresswell  ·  3Comments

benm-eras picture benm-eras  ·  3Comments

maerteijn picture maerteijn  ·  3Comments

jonyoder picture jonyoder  ·  3Comments

38elements picture 38elements  ·  3Comments