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

watchEffect within effectScope is removed when component is unmounted #7319

Closed
posva opened this issue Dec 11, 2022 · 4 comments · Fixed by #7330
Closed

watchEffect within effectScope is removed when component is unmounted #7319

posva opened this issue Dec 11, 2022 · 4 comments · Fixed by #7330
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. 🐞 bug Something isn't working scope: reactivity

Comments

@posva
Copy link
Member

posva commented Dec 11, 2022

@posva posva added 🐞 bug Something isn't working scope: reactivity ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. labels Dec 11, 2022
@edison1105
Copy link
Member

edison1105 commented Dec 11, 2022

// no cb -> simple effect
getter = () => {
if (instance && instance.isUnmounted) {
return
}

let scope;
let store;
export function useCustomStore() {
-  if (store) return store; // comment this line will works fine.
  scope = scope || effectScope(true);
  store = scope.run(() => {
    const flag = ref(false);

+ The root cause is that an unmounted instance is cached in `watchEffect`. 
+ When the component is re-mount, we create a new instance of the component. 
+ But because `if (store) return store;` causes `watchEffect` not to be re-executed, 
+ so its internal instance is the component instance that has been unmounted.
    watchEffect(() => {
      console.log('watchEffect flag', flag.value)
    })
    
    watch(flag, () => {
      console.log('watch flag', flag.value)
    })

    return ({
      flag,
    });
  });

  return store
}

Copy link
Member Author

posva commented Dec 11, 2022

But the watchEffect should be attached to the effectScope like a regular watch
The return is intentional to simulate a store

@edison1105
Copy link
Member

@posva
Actuality, the instance in the regular watch callback is also incorrect and is unmounted. It can execute because we don't determine if the instance is unmounted like watchEffect does.

watch(flag, () => {
  console.log('watch flag', flag.value)
})

@LinusBorg
Copy link
Member

LinusBorg commented Dec 11, 2022

I think watch(Effect)() should care about the currentInstance only if the activeEffectScope === instance.scope during the watcher's creation. But currently, the getter created for a watchEffect() will return early if the instance it was created "in" has unmounted.

Would you agree?

@LinusBorg LinusBorg mentioned this issue Dec 12, 2022
chrislone pushed a commit to chrislone/core that referenced this issue Feb 4, 2023
…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
zhangzhonghe pushed a commit to zhangzhonghe/core that referenced this issue Apr 12, 2023
…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
@github-actions github-actions bot locked and limited conversation to collaborators Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. 🐞 bug Something isn't working scope: reactivity
Projects
None yet
3 participants