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

Vue.nextTick() required with transitions. #52

Closed
autumnwoodberry opened this issue Sep 29, 2017 · 34 comments
Closed

Vue.nextTick() required with transitions. #52

autumnwoodberry opened this issue Sep 29, 2017 · 34 comments
Labels

Comments

@autumnwoodberry
Copy link

Version

1.0.0-beta

Reproduction link

https://github.com/autumnwoodberry/vtu

Steps to reproduce

Nest some v-if:

<transition>
  <div v-if="a">
    <transition v-if="b"></transition>
    <transition v-if="!b"></transition>
  </div>
</transition>
test('output', () => {
    wrapper.setData({
      a: true,
      b: true
    });

    expect(wrapper.vm.a).toBe(true)
    expect(wrapper.vm.b).toBe(true)
    expect(wrapper.html()).toMatchSnapshot()
})

What is expected?

wrapper.html() returns the updated markup.

What is actually happening?

wrapper.html() returns outdated markup unless the test is changed to:

test('output', () => {
    wrapper.setData({
      a: true,
      b: true
    });

    expect(wrapper.vm.a).toBe(true)
    expect(wrapper.vm.b).toBe(true)
    Vue.nextTick(() => {
      expect(wrapper.html()).toMatchSnapshot()
    })
})
@eddyerburgh
Copy link
Member

Thanks for the detailed bug report 😄

This is an issue with transitions, I'll look into it over the next few days

@autumnwoodberry autumnwoodberry changed the title Vue.nextTick() required. Vue.nextTick() required with transitions. Sep 29, 2017
@eddyerburgh
Copy link
Member

I think the solution to this is to stub the transition and transition-group components.

We have 3 options:

  1. leave it to the user to stub using the stubs mount option
  2. Provide a transition and transition-group stub components, for the user to pass in the stubs option
  3. Stub transition and transition group by default inside mount

I'm leaning towards option 2.

I'd be happy with option 3. I don't use transition very often, so I'm not sure if there's be a use case for people testing unstubbed transitions? I'd be interested to hear others thoughts.

@autumnwoodberry
Copy link
Author

I like the "opt-in" option best as well and I think it keeps unexpected internal behavior to a minimum.

It might be nice to have a config somewhere so you can turn it on globally. I use transitions enough that it would be nice to that that option.

@eddyerburgh
Copy link
Member

That's a good idea @autumnwoodberry . We could make a global config, like Vue has.

@autumnwoodberry
Copy link
Author

I'm going to be pretty busy the next few days, so I will just leave a couple more thoughts I had:

  • I'm actualy not sure what wrapper.setData({ a: true }) does vs wrapper.vm.a = true
  • If we made setData() async, or provided an async version or async option for it, it might be a more intuitive solution.
await wrapper.setData({ a: true })

expect(wrapper.vm.a).toBe(true);
expect(wrapper.html()).toMatchSnapshot();
  • Whatever gets decided, I think this should all get mentioned in the setData() docs section.

@eddyerburgh
Copy link
Member

I'm actualy not sure what wrapper.setData({ a: true }) does vs wrapper.vm.a = true

setData sets the data and calls update.

If we made setData() async, or provided an async version or async option for it, it might be a more intuitive solution.

I don't think we should make setData async. Synchronous updating makes unit tests simpler to write and easier to read. I agree, it can be confusing for experienced Vue users who expect async updating, but I think that for the majority of users they would expect setData to update the Vue instance, which it does for everything except transitions. Which is why I'm tempted to stub transitions by default, and make it an opt-out feature.

Whatever gets decided, I think this should all get mentioned in the setData() docs section.

This isn't really a setData issue, it's to do with update. I agree though that this can be confusing.
I think we should link to update in all methods that call update, and add a gotchas section.

@autumnwoodberry
Copy link
Author

Ok yes, I see - that makes perfect sense.

@eddyerburgh
Copy link
Member

eddyerburgh commented Oct 12, 2017

I just released TransitionStub and TransitionGroupStub in 1.0.0-beta.2:

import { TransitionGroupStub, TransitionStub, shallow } from 'vue-tes-utils'

const wrapper = shallow(Component, {
  stubs: {
    'transition': TransitionStub,
    'transition-group': TransitionGroupStub
  }
})

@autumnwoodberry
Copy link
Author

Awesome! I will check it out when I have some spare moments.

@petridw
Copy link

petridw commented Oct 18, 2017

@eddyerburgh I'm using TransitionStub and attempting to assert that elements enter & leave the dom after data change (two elements with a simple v-if/v-else inside a <transition> tag). It seems to work on enter, but not on leaving - the content inside the <transition> tag remains in the wrapper's html and receives the v-leave and v-leave-active classes. I expected the transition stub to just cause inner content to instantly enter/leave, is that understanding off the mark?

@eddyerburgh
Copy link
Member

eddyerburgh commented Oct 18, 2017

Hi @petridw, leave hasn't been implemented. I'll look into it.

Do you have an example I could add as a test?

@petridw
Copy link

petridw commented Oct 18, 2017

@eddyerburgh here's a failing test that should be passing, I think:

component-with-transition.vue

<template>
  <div>
    <transition>
      <div v-if="bool" key="a">{{ trueText }}</div>
      <div v-else key="b">{{ falseText }}</div>
    </transition>
  </div>
</template>

<script>
export default {
  name: "component-with-transition",
  data() {
    return {
      bool: true,
      trueText: "a",
      falseText: "b",
    };
  },
};
</script>

transition.spec.js

import { shallow, TransitionStub } from "vue-test-utils";
import ComponentWithTransition from "@/components/component-with-transition";

const options = {
  stubs: {
    transition: TransitionStub,
  },
};

describe("component-with-transition", () => {
  it("swaps dom nodes properly", () => {
    const wrapper = shallow(ComponentWithTransition, options);
    expect(wrapper.text()).toBe(wrapper.vm.trueText);
    wrapper.setData({ bool: false });
    expect(wrapper.text()).toBe(wrapper.vm.falseText);
  });
});

@eddyerburgh
Copy link
Member

Thanks @petridw, if you remove the keys, the test passes.

The TransitionStub basically behaves the same as an unstubbed transition, except it doesn't add leave classes when the component inside is hidden, and it always returns the child synchronously.

@petridw
Copy link

petridw commented Oct 19, 2017

Interesting, so is there a different way I should be testing components that use transitions with keys?

@eddyerburgh
Copy link
Member

eddyerburgh commented Oct 20, 2017 via email

@petridw
Copy link

petridw commented Oct 20, 2017

@eddyerburgh cool - have a great vacation & thanks for all your work!

@acacha
Copy link

acacha commented Nov 9, 2017

Maybe this is no more and issue? For me it works using shallow instead o mount no need to user any extra stubs like TransitionGroupStub or TransitionStub

@eddyerburgh
Copy link
Member

Thanks for the heads up, I'll look into it

@eddyerburgh
Copy link
Member

The fix for the keyed bug will be released in beta.9 🎉

@jorySsense
Copy link

Transition group stub over writes slots inside,

@eddyerburgh
Copy link
Member

eddyerburgh commented Jan 3, 2018 via email

@denisinvader
Copy link

I can't test component with transition.

In component:

...
<transition
  :name="optionsClass"
  @before-enter="animationFinished = false"
  @after-enter="animationFinished = true"
>
  <div v-show="opened">
...

In tests:

console.log(wrapper.html())
wrapper.find('.u-select__select').trigger('click');  // sets data to display element (v-show)
console.log(wrapper.html());

The second html in console equals the first (with style="display: none").

If I remove <transition> from component it works as expected.

@eddyerburgh
Copy link
Member

@denisinvader please open a new issue with a reproduction

@denisinvader
Copy link

@eddyerburgh I've spot the bug #567

@MatteoGabriele
Copy link

I have the same bug right now. The transition tag doesn't allow me to test when elements are visible or not because it doesn't wait until the end of the animation I supposed.

@SmileLifeIven
Copy link

SmileLifeIven commented Jan 15, 2020

@JessicaSachs Does this bug have been fixed in beta.30?

@JessicaSachs
Copy link
Collaborator

I’m deferring to @lmiller1990 who would’ve made any code changes

@lmiller1990
Copy link
Member

I don't think this has been fixed. This issue tracks it: #1384

This is the next issue I'm going to focus on. There is a work-around in that thread that might help you.

The solution will be likely to stub all transitions by default.

@SmileLifeIven
Copy link

@lmiller1990 @JessicaSachs Thanks for your reply. I change my code follow Guides - Common Tips, but it doesn't work. So I do it as other way and worked.

window.getComputedStyle = () => {
    return {
      transitionDelay: '',
      animationDelay: '',
      transitionDuration: '',
      animationDuration: '',
    };
  };

@SmileLifeIven
Copy link

SmileLifeIven commented Jan 16, 2020

By the way, could you update chinese document I update version to 30 but I don't konw how to config it follow chinese document. So I asked question as you see.

@lmiller1990
Copy link
Member

What do you want changed in the Chinese document? It's easy enough to update code snippets, but like the actual explanation will need updating, too. If you see some improvements you are more than welcome to make them.

@SmileLifeIven
Copy link

I just can't find Mocking Transitions in Chinese document. and others, like 'Lifecycle Hooks', 'Writing asynchronous tests using nextTick (new)' and so on. I'm not sure it's a right document I read.
Thanks for your time.

@lmiller1990
Copy link
Member

Those are recent additions. No-one has updated the Chinese docs, since none of the maintainers of VTU know Chinese, I guess.

Keeping translations up to date is very challenging - would like to know if any tools solve this problem.

@awacode21
Copy link

TransitionStub

for me this causes to fail every test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests