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

Watch with immediate totally breaks setProps #1419

Closed
aldarund opened this issue Jan 27, 2020 · 8 comments · Fixed by #1618, eddieferrer/sizesquirrel-open#212 or nextcloud/social#1222
Closed
Assignees

Comments

@aldarund
Copy link

aldarund commented Jan 27, 2020

Version

1.0.0-beta.31

Reproduction link

https://github.com/aldarund/vue-test-utils-transition-bug/blob/master/components/watch.test.js
https://github.com/aldarund/vue-test-utils-transition-bug/blob/master/components/watchtest.vue

Steps to reproduce

use watch with immediate as prop

What is expected?

both test run succesfully

What is actually happening?

setProps when there is watch with immediate in component dont do anything, it just dont set prop at all.

@aldarund
Copy link
Author

aldarund commented Jan 27, 2020

There is a test for this usecase, well almost

it('updates watched prop', () => {

But if u do add watch in mounted -> it works.
But it dont work if u define watch as prop, not in mounted, so that test dont cover this issue

@lmiller1990
Copy link
Member

lmiller1990 commented Jan 29, 2020

Hey, thanks for the report. This is definitely a bug. What is your use case for this? There might be a better way than setProps. setProps is really an anti-pattern (the user cannot arbitrarily change the props) and I'm exploring depreciating this method for that reason.

Are you able to share your use case? Happy to provide some alternatives to setProps. Also happy to fix the bug (shouldn't be too difficult) if there is a strong use case this feature.

BTW thanks for your work on Nuxt.js, it is awesome, I love it.

@lmiller1990 lmiller1990 self-assigned this Jan 29, 2020
@aldarund
Copy link
Author

aldarund commented Jan 29, 2020

Well, the usecase pretty simple and general. Test components with different props passed to it. Its not specific to watch in any way.
The only other way that i see is to do full mount each time, but i dont see how its beneficial.
But specifically setProps can be used to see how component reacts on when passed property changed, isnt that how user change it? e.g. change the value in upper level and and see how component react - so for this setProps, and watch

Thanks for your work on test utils too :)

@lmiller1990
Copy link
Member

lmiller1990 commented Jan 29, 2020

The only other way that i see is to do full mount each time, but i dont see how its beneficial.

I think this is ok - then you don't need to await nextTick, you just mount and make your assertion. Here is an example of a refactor in Vuetify where we moved from setProps to a for loop and mounting options - resulted in less code. Let me know what you think.

But specifically setProps can be used to see how component reacts on when passed property changed, isnt that how user change it? e.g. change the value in upper level and and see how component react - so for this setProps, and watch

Kind of - setProps forces a change in the props and does some $forceUpdate. A user would not do this - they click a button (or something) and it would $emit an event, which changes the prop, and the UI updates without $forceUpdate. I don't think this causes any problems in VTU. I think this is a feature we should add to VTU for Vue 3 - some kind of "wrapWithParent" feature, so you can more accurately simulate these situations. Any ideas?

Oddly enough, this passes:

 it('with imeddiate', async () => {
      component.setProps({
        test: 'second'
      })
      await localVue.nextTick()
      component.setProps({
        test: 'second'
      })
      await localVue.nextTick()
      expect(component.vm.test).toBe('second')
    })

I think the problem is if immediate is true, it fires once, and will not fire again until the value changes again. We could probably hack setProps to get this to work, but since most of the focus in on preparing for VTU and Vue 3, I'm not sure I can justify the time to fix this. Would be happy to help out with some ideas on how to fix this if you want to make a PR - message me on discord or try making a PR.

It is likely we will be revisiting watch for the Vue 3 compat; it looks like immediate is going to be the new default behavior (although not called immediate anymore). Although I still think we should depreciate setProps, we will definitely be making this kind of case (async behavior, immediate watchers) a priority for VTU + Vue 3, so I do appreciate all the issues you are reporting - it really helps identify pain points of VTU.

@kspackman
Copy link

I'll second what was said. User input may not be in the component I'm testing, but the result of it may cause the props being passed into my component to update. So testing that the component appropriately responds to changing props is a very valid use case in my opinion, and one that we test a decent amount.

@lmiller1990
Copy link
Member

Yep, setProps is useful and not going anywhere @kspackman 👍 it is available in the Vue 3 integration, found here.

As for this bug, I did not get time to investigate it yet. If you would like to do so, I can probably give you some pointers as to where to start?

AtofStryker added a commit to AtofStryker/vue-test-utils that referenced this issue Jul 22, 2020
setProps in certain cases was being blown away by nextTick intervals. If the property is not up to
date, setProps will be called again to sync the changes.

fix vuejs#1419
@AtofStryker
Copy link
Contributor

@lmiller1990 @aldarund I took a stab at this just as an idea. Maybe this is a terrible idea, but figured we can return a promise wrapping nextTick and checking if the props were set accordingly, and if not, recursively call setProps. This is super hacky, but not sure if there is another way to accomplish this functionality and figured it can't hurt to create a PR 😄

https://github.com/vuejs/vue-test-utils/pull/1618/files

AtofStryker added a commit to AtofStryker/vue-test-utils that referenced this issue Jul 22, 2020
setProps in certain cases was being blown away by nextTick intervals. If the property is not up to
date, setProps will be called again to sync the changes.

fix vuejs#1419
AtofStryker added a commit to AtofStryker/vue-test-utils that referenced this issue Jul 31, 2020
setProps in certain cases was being blown away by nextTick intervals. If the property is not up to
date, setProps will be called again to sync the changes.

fix vuejs#1419
lmiller1990 added a commit that referenced this issue Aug 1, 2020
…als (#1618)

* fix(setprops): allowed for setProps to be synced with nextTick intervals

setProps in certain cases was being blown away by nextTick intervals. If the property is not up to
date, setProps will be called again to sync the changes.

fix #1419

* Update packages/test-utils/src/wrapper.js

Co-authored-by: Bill Glesias <bglesias@gmail.com>

Co-authored-by: Lachlan <lachlan.miller.1990@outlook.com>
@hcunninghampk
Copy link

hcunninghampk commented May 3, 2021

Hello, I'm new to Vue and to Vue testing. I'm trying to follow along with your Vue School classes; I even paid to join. But, nothing works the way your classes state it's supposed to. Right now, I'm trying to follow along with the class/video for testing computed and watched properties, which states to use setProps() to change the property for a temperature. It doesn't work at all. The property is not changed. ( setData() didn't work the same way your VueSchool states either, but I edited the test enough to get it to work.)

I don't really follow much of this thread, being new to Vue. But, in the forked repo for the class, the watched property's immediate is set to true. How do I make this work so I can at least continue learning with your Vue classes? Here is my forked copy of your repo: https://github.com/hcunninghampk/testing-vue-components. I'm on the 2nd branch.

PS -- How much your Vue School instructions differ from the ways methods, functions, etc. have been updated and deprecated is extremely, extremely frustrating. If one is new to Vue, how are we supposed to figure out how to make these classes/videos work, when the classes/videos are not updated -- especially for classes/videos I paid for. Will you please update your classes/videos so they work?

hcunninghampk added a commit to hcunninghampk/testing-vue-components that referenced this issue May 3, 2021
… not work the way Vue states it should. Has an open bug: vuejs/vue-test-utils#1419  * Commented out test.  Skipping does not work well with output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment