-
Notifications
You must be signed in to change notification settings - Fork 668
Using a non deprecated function should not print a deprecation warning #1532
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
Comments
We can also add 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 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: #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: // 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 |
#1534 This should improve the message @TheDutchCoder |
Ah ok, 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: I'm trying to apply as stated in the deprecation warning, but i'm not getting any further? Can anyone help? |
Hi @p0psicles! |
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, Hope this helps 🙌 |
If you're here looking for a transition for // 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. |
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 👍 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
This is true but |
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? 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. |
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; That said, we do have 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 |
I agree. I appreciate your response and reasoning. Thanks for reconsidering. |
@lmiller1990 while we are here speaking about deprecations - could we possibly undeprecate |
That is a pretty interesting use case - I didn't think about this one at all. Proposal:
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 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 |
@lmiller1990 I believe we could close this one as relevant MRs were merged! |
Version
1.0.0
Reproduction link
https://github.com/maxarndt/repro-vue-test-utils-deprecation-warning
Steps to reproduce
npm i
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 ofisVueInstance
.The text was updated successfully, but these errors were encountered: