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

Creating a vm in a detached EffectScope before any other vm is initialized breaks tracking of the activeEffectScope #12825

Closed
Aaron-Pool opened this issue Oct 12, 2022 · 2 comments

Comments

@Aaron-Pool
Copy link

Aaron-Pool commented Oct 12, 2022

Version

2.7.12

Reproduction link

Super slim repro on stackblitz

Steps to reproduce

Just open the stackblitz reproduction and check the console. You'll see:

[Vue warn]: onScopeDispose() is called when there is no active effect scope to be associated with.

In general though, you can cause the issue like so:

  1. Create a detached effect scope before creating a regular vue VM
  2. Do something in the run block of that scope (like lazily import a file with a store definition) that causes a Vue VM to be created
  3. Call onScopeDispose()

What is expected?

The active scope tracking should be maintained, and not get "tricked" by not having an ancestor VM.

What is actually happening?

When setCurrentInstance(vm) is called, it passes prev as the vm argument, which is null. This leads to .off being called on the detached EffectScope that Vue 2.7 creates when a VM is initialized. This is not the detached scope that we created manually, mind you. And since .off is not supposed to be called on a detached EffectScope (it says this in explicitly in code comments), bad stuff happens and Vue's global activeEffectScope becomes undefined.


This doesn't currently happen in our app at runtime (though I think it easily could), it happens during our test suite. We try to be as lazy as possible in our test environment when it comes to the code that each test imports, because it substantially helps jest run time, especially when running individual test files. Because of this, we use the lazy option of the commonjs babel plugin. This means the Vuex store that would normally be instantiated before the detached effect scope ends up getting created after the detached effect scope. But I haven't seen anything in the Vue docs to say I shouldn't be able to create a detached effect scope before any other Vue VM, so I think this is a legitimate bug?

I'd be willing to work on a PR to fix this, but I'd need guidance, since my limited knowledge base of Vue's internals would probably mean any solutions I would try might break other things.

Update: Just adding a reference to related issue in Vue 3. Not sure how similar the internals of the effect scope implementation/tracking are, but right here @posva makes a comment that "you would never call off on a detached effect scope". But that does, indeed seem to be exactly what Vue 2.7 does. It seems like the buggy behavior the person who brought this up was worried about does indeed occur in some cases.

@Aaron-Pool
Copy link
Author

Update: I don't actually see the reason why .off() is ever called on currentInstance._scope. From what I can see _scope is only ever assigned to a detached scope. The only assignment to that field I can find is in initMixin, where vm._scope = new EffectScope(true) is called.

@Aaron-Pool
Copy link
Author

Appreciate the quick fix @yyx990803 🎉🎉🎉

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

No branches or pull requests

1 participant