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

onScopeDispose is not called inside not immediate watchers #12681

Open
ferferga opened this issue Jan 10, 2025 · 7 comments
Open

onScopeDispose is not called inside not immediate watchers #12681

ferferga opened this issue Jan 10, 2025 · 7 comments

Comments

@ferferga
Copy link
Contributor

Vue version

3.5.13 (also tested on 22dcbf3 in Playground)

Link to minimal reproduction

https://play.vuejs.org/#__PROD__eNqNVE1vGjEQ/SuDLxBpyyptT4hETVOkplI/RJB68WVjhsWJ17bWNqFC/PeObb4LUW/rmTdv5o3fesXurO0vArIBGzrRSuvBoQ/2lmvZWNN6uDeNvYZZaxro9st0igXdHWAFbl4pZV7HOIP1BpkRXAujnYfGBO1xCjcH0J5vA15xPSxzX+pIB4+NVZVHOgEMn4L3RsMnoaR4ueFsz9PZfHJ2OzF1rTD3GJa5JJfn0Rfv5GxfyxmUlB2WB51YwbyjSWey7j87o2kZq0jAmSAGqbD9ab0kJZwNIGViLgn5lmJRSrGNizmKlzPxZ7eMMc5+teiwXSBnu5yv2hp9To8ef+CSvnfJxkyDIvQbyTE6o0KcMcM+Bz2lsQ9wadqHdGNS1xM3WnrUbisqDhqR64TnjK4vLu+S9P24H/ofUx3Xa9rizh2X7bQCox+FsfhFOmscFvBaeTEvDpxRAJHfh7ZF7RP0MPBAfqq0wAtOc7LWlTo22qxSLjqN69SqlzEF9K7g5jaLisVGYV+Zutf9fTe5/zoaw/UAxkF3ulQKJ1P3cu25Mqo6xnaviGCd+pclTObSEQBBKKx0sI5ISJjw6g88oaiCQ5Ae5pUD2TQ4lWTR7f3k8XPrZK1DCVlVf1GpgKR/e6Nnlb0fQGUtyO0uZ/RvTLsFdDr/LroXx7/MIzIa6P+xG6LOCVNaxobmv9dI1OfWWJCBTvYCcbdvvSJTuUgfAN83b0CMljl8/BCs/wIi+bb3

Steps to reproduce

  1. Open the playground and the console. You will see how the immediate "watcher 2" runs and watcher 1 does the same afterwards (because watcher 2 changes the variable it depends)
  2. Toggle the component: you will see how onScopeDispose from watcher 2 runs, but not the one from watcher 1

What is expected?

Both onScopeDispose callbacks are fired properly and both messages are printed to the console, regardless the immediate status of the watcher

What is actually happening?

The onScopeDispose is properly fired on both

System Info

No response

Any additional comments?

No response

@xfontr
Copy link

xfontr commented Jan 11, 2025

I don't have a specific solution for this but yes hopefully a debugging hint.

The immediate watcher is triggered even before the component is created. After that, the component is created. Once that happens, the non-immedate watcher is triggered. I assume that the fact this happens later affects the onScopeDispose actual scope, maybe focusing on the watcher itself rather than the component?

Here's how I tried this out: play.vuejs

Update:

I've tried to handle scopes explicitly with effectScope, but the results are exactly the same. The first time the immediate watcher runs it does get the scope. However, the second time (the mounting and creating of the component are done) it does not get the scope.

Either there's something I'm missing about Vue's scoping (which is very possible), or there's probably an actual issue/bug going on here.

@ferferga
Copy link
Contributor Author

Interestingly enough, the callback runs on the watch exported by @vue/reactivity. See the checks of the linked PR.

@skirtles-code
Copy link
Contributor

I may have misunderstood, but...

This all appears to be working correctly to me. I don't think onScopeDispose is supposed to work inside the callback of a watcher. Do you have a specific use case in mind?

onScopeDispose is like a more general version of onUnmounted. Much like onUnmounted, you can use it alongside a watcher, but you wouldn't use it inside the watcher callback.

So for example, consider this code:

watch(source, cb)
onUnmounted(doTeardown)

This will only work inside a component, because onUnmounted needs to be inside a component. This can be rewritten to this:

watch(source, cb)
onScopeDispose(doTeardown)

This can now be used inside any effect scope, not just inside a component, allowing doTeardown to be called when the watcher is discarded.

The setup function for a component runs inside an effect scope. A watcher with immediate: true will run the callback immediately, so it will pick up the effect scope, but I think that's more by accident than by design. The next time the watcher callback is called there won't be an effect scope present. I don't think that's a bug.

Much like with onUnmounted, I'm not sure why you would want to call onScopeDispose inside a watcher callback. Every time the watcher callback is called it'll be registering another onScopeDispose callback. That seems unlikely to be something you'd want.

@ferferga
Copy link
Contributor Author

@skirtles-code I initially found this for some VueUse composables (specifically useEventListener) that were being invoked inside watchers.

You're assuming all the logic will be bound to a component, but some composables might be intended to be used in both places. A good workaround would be to PR in VueUse and add, alongside their tryOnScopeDispose function a call to tryOnUnmount, but that's not convenient.

Then, it's the problem of making users aware of this. How an user is supposed to guess that onScopeDispose is not intended to be used in watchers, when watchers are bound to the component effect scope? Why immediate watchers work, but normal ones doesn't? If I wouldn't be aware of this bug, for me, it's clear: watchers are bound to the component's effect scope, so onScopeDispose should work there. It's intuitive.

Let me flip your question: why then not make onScopeDispose do nothing if I call it at the top level of setup? If it works there, why it wouldn't in this case?

There is also another case (through I agree is an edge one): I might be creating a different effect scope inside a component and dispose it in a different moment, which has nothing to do with when the component is unmounted.

There's also another problem: the inconsistency between both watchers. Users pulling vue get the "bugged watch" (independently if using them inside components or outside them), while importing it from @vue/reactivity directly works fine.

TLDR: Although it may seem silly, the playground is just a simple reproduction of the issue, but out there there are legitimate use cases for this!

@edison1105
Copy link
Member

The setup function for a component runs inside an effect scope. A watcher with immediate: true will run the callback immediately, so it will pick up the effect scope, but I think that's more by accident than by design. The next time the watcher callback is called there won't be an effect scope present. I don't think that's a bug.

I agree with @skirtles-code. In the DEV mode, there are warnings. see Playground DEV mode, the warning will inform the user that this usage will not work.
This can be considered an edge case. Assuming we fix the immediate watcher, the inability to use onScopeDispose would be a breaking change.

@ferferga
Copy link
Contributor Author

ferferga commented Jan 12, 2025

@edison1105 @skirtles-code Is there any reason/something I'm missing of why the watcher's callback doesn't run inside the effect scope of the component by default?

Wouldn't wrapping all the calls to the callback like this:

scope.run(() => cb)

Be a good solution?

And yes, a warning is printed in DEV (sorry I didn't notice that, definitely good to know!)

@edison1105
Copy link
Member

Is there any reason/something I'm missing of why the watcher's callback doesn't run inside the effect scope of the component by default?

There is no need to track dependencies inside watch callback calls, so the watcher's callback doesn't run inside an effect scope. onScopeDispose should be placed inside the setup function for it to work correctly.

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