vuejs / vue-test-utils

Component Test Utils for Vue 2
https://vue-test-utils.vuejs.org
MIT License
3.57k stars 669 forks source link

Async watcher in some cases called twice after setProps. #1379

Open motz-art opened 4 years ago

motz-art commented 4 years ago

Version

1.0.0-beta.30

Reproduction link

https://github.com/motz-art/vue-test-utils-watcher-called-twice

Steps to reproduce

After opening repo run

npm i
npm run test:unit

What is expected?

After call to setProps component value watch handler should be called once.

This will match behavior of such component in a real browser.

And console output should be like that:

undefined
setting props
props are set.
good

As a result test will also pass.

What is actually happening?

After a call to setProps there is two calls to value watch handler, first with the value 'good' that is supplied in setProps and second with undefined.

See console output:

undefined
setting props
props are set.
good
undefined

As second call happens right before Promise is resolved, it sets delayedValue to undefined and that leads to failed test.


I don't know why but removing any of the if statements fixes the issue. As well as removing immediate: true from value watcher.

Checked this test in @vue/test-utils@1.0.0-beta.29 there is 3 calls to watch handler, so console output looks like:

undefined
setting props
props are set.
good
undefined
good
JessicaSachs commented 4 years ago

There were a lot of async changes with setProps, setData, and trigger in beta.30. Would you like to take a crack at this bug?

dobromir-hristov commented 4 years ago

Just wrote a unit test for this and it passes, the watcher's handler is called properly.

 it.only('runs the watcher only once', async () => {
    const listener = () => Promise.resolve()
    const spy = sinon.spy(listener)
    const TestComponent = {
      template: '<div/>',
      props: ['prop1'],
      watch: {
        prop1: {
          immediate: true,
          handler: spy
        }
      }
    }
    const wrapper = mountingMethod(TestComponent)
    await wrapper.vm.$nextTick()
    expect(spy.callCount, 'To be called initially once').to.equal(1)

    wrapper.setProps({
      prop1: 'some Data'
    })
    await wrapper.vm.$nextTick()
    expect(spy.callCount, 'to have been called an extra one time after using setProps').to.equal(2)
    expect(spy.getCall(1).calledWith('some Data', undefined)).to.be.true
  })
guilhermewaess commented 4 years ago

I have the following scenario which might be the same problem. If you guys have any idea how to work around it.

And also if I can help with something else, just let me know.

The code:

AppSnackbar:

@Component
export default class AppSnackbar extends Vue {
  visiblityTimer: NodeJS.Timeout | null = null

  @Prop({ type: Boolean, required: true })
  visible: boolean

  @Prop({ type: Number, default: 5000 })
  timeout: number

  @Watch('visible', { immediate: true })
  onVisibilityChange(visible, oldValue) {
    console.log(
      'AppSnackbar -> onVisibilityChange -> newValue',
      visible,
      oldValue
    )
    if (this.visiblityTimer) clearTimeout(this.visiblityTimer)

    if (visible) {
      this.visiblityTimer = setTimeout(() => {
        this.$emit('hide')
      }, this.timeout)
    }
  }
}
</script>

AppSnackbar.spec.js

it('should emit hide event after set timeout', async () => {
    const wrapper = shallowMount(Snackbar, {
      slots: {
        default: 'foo bar',
      },
      propsData: {
        visible: false,
        timeout: 1000,
      },
    })

    wrapper.setProps({ visible: true })
    await wrapper.vm.$nextTick()

    jest.advanceTimersByTime(1000)
    expect(wrapper.emitted().hide).toBeDefined()
  })

The outcome is:

1st console log from immediate
console.log packages/core/components/GlobalComponents/AppSnackbar.vue:30
    AppSnackbar -> (newValue, oldValue) -> false undefined

2nd console log from setProps
console.log packages/core/components/GlobalComponents/AppSnackbar.vue:30
    AppSnackbar -> (newValue, oldValue) -> true false

3rd console log ????????
console.log packages/core/components/GlobalComponents/AppSnackbar.vue:30
    AppSnackbar -> (newValue, oldValue) -> false true

There is no place changing the visible value to false and this is the only test inside the spec.

The version is: 1.0.0-beta.32

JessicaSachs commented 4 years ago

setProps with immediate watchers is buggy. For your test, the best practice is to have visible be true initially on mount.

On Wed, Apr 1, 2020 at 11:50 AM Guilherme Waess notifications@github.com wrote:

I have the following scenario which might be the same problem. If you guys have any idea how to work around it.

And also if I can help with something else, just let me know.

The code:

AppSnackbar:

@Component export default class AppSnackbar extends Vue { visiblityTimer: NodeJS.Timeout | null = null

@Prop({ type: Boolean, required: true }) visible: boolean

@Prop({ type: Number, default: 5000 }) timeout: number

@Watch('visible', { immediate: true }) onVisibilityChange(visible, oldValue) { console.log( 'AppSnackbar -> onVisibilityChange -> newValue', visible, oldValue ) if (this.visiblityTimer) clearTimeout(this.visiblityTimer)

if (visible) {
  this.visiblityTimer = setTimeout(() => {
    this.$emit('hide')
  }, this.timeout)
}

} }

AppSnackbar.spec.js

it('should emit hide event after set timeout', async () => { const wrapper = shallowMount(Snackbar, { slots: { default: 'foo bar', }, propsData: { visible: false, timeout: 1000, }, })

wrapper.setProps({ visible: true })
await wrapper.vm.$nextTick()

jest.advanceTimersByTime(1000)
expect(wrapper.emitted().hide).toBeDefined()

})

The outcome is:

1st console log from immediate console.log packages/core/components/GlobalComponents/AppSnackbar.vue:30 AppSnackbar -> (newValue, oldValue) -> false undefined

2nd console log from setProps console.log packages/core/components/GlobalComponents/AppSnackbar.vue:30 AppSnackbar -> (newValue, oldValue) -> true false

3rd console log ???????? console.log packages/core/components/GlobalComponents/AppSnackbar.vue:30 AppSnackbar -> (newValue, oldValue) -> false true

There is no place changing the visible value to false and this is the only test inside the spec.

The version is: 1.0.0-beta.32

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/vuejs/vue-test-utils/issues/1379#issuecomment-607331055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVL4BE5CC6XSJZ7SXNGOGTRKNPDBANCNFSM4J4L34SQ .

AtofStryker commented 4 years ago

This likely related to #1419 and might be fixed by #1618?