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

Change in async behavior between version 1.0.0-beta.16 and 1.0.0-beta.24 #906

Closed
jlynde opened this issue Aug 7, 2018 · 14 comments
Closed

Comments

@jlynde
Copy link

jlynde commented Aug 7, 2018

Version

1.0.0-beta.24

Reproduction link

https://codesandbox.io/s/3q07pkwm5

Steps to reproduce

See the comments in the HelloWorld.spec.js file of the linked minimal reproduction. Basically, there is one section of code that can be commented or un-commented to show the problem.

This is based on our project code which has many tests that are similar in nature to this reproduction link. These tests are all passing if we use 1.0.0-beta.16 (and 1.0.0-beta.17 ). But when we tried to upgrade to the latest version (1.0.0-beta.24) these tests began to fail.

What is expected?

The tests depend on localVue.nextTick() not being called until the DOM is changed by the component code.

What is actually happening?

The localVue.nextTick() function is being called too soon.

@LinusBorg
Copy link
Member

LinusBorg commented Aug 8, 2018

I'm wondering how this test could have ever passed in a previous version.

    asyncLoad() {
      console.log('ClickableComponent.asyncLoad() was called');
      setTimeout(this.toggleLoaded, 500);
    }

You seem to think that localVue.nextTick() would push the callback code until after the above setTimeout has run, triggered a re-render and updated the DOM.

That's not the case.

Vue.nextTick doesn't wait for any and all async operations you may or may not have triggered in the click event (which may take milliseconds or hours to finish) - it's always just pushing the code behind the next async render cycle of Vue. if there's no immediate re-render on the schedule (as in your example), it means that the code is simply pushed onto the next call stack.

This next callstack will always run before the 500ms Timout has finished.

Am I missing something about the situation that you reproduced here?

@jlynde
Copy link
Author

jlynde commented Aug 8, 2018

I have improved the example a bit. Sorry for the first try. To give a bit more background: In my real component when the user clicks a promise is called. This promise may or may not actually be an async call (depending on whether or not data has been pre-loaded or was loaded by a previous click). In my tests I always pre-load data so the promise resolves immediately. The updated example does this better now and behaves much more like the actual component.

Currently, I have a dependency on @vue/test-utils 1.0.0.-beta.16 and you can see that the test passes. If you update the dependency to 1.0.0-beta.24 you will see that the test fails.

https://codesandbox.io/s/3q07pkwm5

@eddyerburgh
Copy link
Member

Sorry that you're experience issues, but I'm not sure I can help here. Although your reproduction fails on CodeSandBox, I can only replicate locally by setting sync to false, which is expected.

Currently Vue Test Utils sets watchers to run synchronously when sync mode is set to true (which is is by default). This has caused some bugs, and we're intending to change the behavior, but this isn't the issue here #676, but the change was implemented in beta.13.

My advice to you if you're waiting for a promise is to use a setTimeout, since it will always run after every Promise callback in the microtask queue is executed.

@jlynde
Copy link
Author

jlynde commented Aug 9, 2018

I updated the example one more time - this time with my real component which is a recursive component. I am wondering if that could be a factor. I was trying to keep the reconstruction example simpler but I cannot seem to create an example that demonstrates the problem I am seeing but the actual component shows the issue in codesandbox.io.

With version beta.16 the test runs and passes and you can see by the console.log statements that I have added that the wrapper does, in fact, get updated as expected.

However, when you upgrade to beta.24 the test fails and you can see that the wrapper is NOT getting updated at all. EVEN if I switch to using setTimeout instead of nextTick. Nothing I have tried is working to update the wrapper in the newer version so I have not ben able to find any way to execute tests. Note that the console log shows that the component is being triggered but the wrapper just does not get updated with the newer version of @vuew/test-utils.

@eddyerburgh
Copy link
Member

I don't think you saved the latest version in CodeSandBox, I see a TreeView.vue file but it's empty, and the test is passing.

@jlynde
Copy link
Author

jlynde commented Aug 10, 2018

Very strange that you do not see the contents of the file. I just loaded it on a different computer using a different browser and I am not logged into Codesandbox.io at all. Try refreshing the link maybe?

As for the test passing, yes, I expect that because at the moment I have the dependency set for beta.16. I wanted you to see that it works with beta.16 first. All you need to do is update that dependency to beta.24 and you will see it fail and the wrapper not getting updated.

@jlynde
Copy link
Author

jlynde commented Aug 10, 2018

OK. Try this one instead.
https://codesandbox.io/s/37737pk9m

@jlynde
Copy link
Author

jlynde commented Aug 16, 2018

Have you been able to see the issue with the updated example (see previous post)? The test passes with the example project as configured (using the beta.16 version). But if you update the version of @vue/test-utils it will fail.

@LinusBorg
Copy link
Member

Yeah I have been able to reproduce it, but haven't had time round that for looking into why it happens

@jlynde
Copy link
Author

jlynde commented Aug 16, 2018

Thanks for the confirmation.

@AtofStryker
Copy link
Contributor

@LinusBorg @eddyerburgh have there been any updates on this issue?

@eddyerburgh
Copy link
Member

Not on this particular issue, but Vue 2.5.18 will include a sync option that should solve issues like this.

I will try to look into this issue in the next few weeks

@AtofStryker
Copy link
Contributor

@eddyerburgh @LinusBorg I think some of the issues here are pertaining to #676. We were able to upgrade to beta-28, but with some less than graceful fixes. Just figured I would throw an update out there and appreciate all the work you guys do!

@eddyerburgh
Copy link
Member

I've just released beta.29 which sets Vue to run in sync mode.

I'm going to close this issue because the reproduction is too large. @jlynde if you continue to have problems after upgrading to beta.29, please open a new issue with a minimal reproduction.

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

No branches or pull requests

4 participants