-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
fix(reactivity): ensure watch(Effect) can run independent of unmounted instance if created in a detatched effectScope (fix #7319) #7330
Conversation
…d instance if created in a detatched effectScope
/cc @posva this should solve your problem in pinia. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I think it would be even better to have 2 different count variables as it would make the test more robust
Good call, will do. |
…d instance if created in a detatched effectScope (fix vuejs#7319) (vuejs#7330) * fix(reactivity): ensure watch(Effect) can run independent of unmounted instance if created in a detatched effectScope * test: use separate counters for each watcher to make test more robust
…d instance if created in a detatched effectScope (fix vuejs#7319) (vuejs#7330) * fix(reactivity): ensure watch(Effect) can run independent of unmounted instance if created in a detatched effectScope * test: use separate counters for each watcher to make test more robust
I think this PR may have hidden issues, especially for undetached effectScope. There are many other functions depends on currentInstance, like
If we need keep the short circuit for some reason, I think the better fix is: getter = () => {
- if (instance && instance.isUnmounted) {
+ if (currentScope && !currentScope.active) {
return
}
} I also tried, all unit tests passed. cc @yyx990803 |
@yangmingshan PR welcome so we can take a better look and run ecosystem-ci if needed. |
close #7319