fix(watch): ensure watchers respect detached scope

fix #4158

Author: yyx990803

packages/runtime-core/__tests__/apiWatch.spec.ts

@@ -25,7 +25,8 @@ import {
   TriggerOpTypes,
   triggerRef,
   shallowRef,
-  Ref
+  Ref,
+  effectScope
 } from '@vue/reactivity'
 import { watchPostEffect } from '../src/apiWatch'
 
@@ -848,7 +849,7 @@ describe('api: watch', () => {
   })
 
   // https://github.com/vuejs/vue-next/issues/2381
-  test('$watch should always register its effects with itw own instance', async () => {
+  test('$watch should always register its effects with its own instance', async () => {
     let instance: ComponentInternalInstance | null
     let _show: Ref<boolean>
 
@@ -889,14 +890,14 @@ describe('api: watch', () => {
     expect(instance!).toBeDefined()
     expect(instance!.scope.effects).toBeInstanceOf(Array)
     // includes the component's own render effect AND the watcher effect
-    expect(instance!.scope.effects!.length).toBe(2)
+    expect(instance!.scope.effects.length).toBe(2)
 
     _show!.value = false
 
     await nextTick()
     await nextTick()
 
-    expect(instance!.scope.effects![0].active).toBe(false)
+    expect(instance!.scope.effects[0].active).toBe(false)
   })
 
   test('this.$watch should pass `this.proxy` to watch source as the first argument ', () => {
@@ -1024,4 +1025,26 @@ describe('api: watch', () => {
     expect(plus.value).toBe(true)
     expect(count).toBe(0)
   })
+
+  // #4158
+  test('watch should not register in owner component if created inside detached scope', () => {
+    let instance: ComponentInternalInstance
+    const Comp = {
+      setup() {
+        instance = getCurrentInstance()!
+        effectScope(true).run(() => {
+          watch(
+            () => 1,
+            () => {}
+          )
+        })
+        return () => ''
+      }
+    }
+    const root = nodeOps.createElement('div')
+    createApp(Comp).mount(root)
+    // should not record watcher in detached scope and only the instance's
+    // own update effect
+    expect(instance!.scope.effects.length).toBe(1)
+  })
 })

packages/runtime-core/src/apiWatch.ts

@@ -25,7 +25,9 @@ import {
 import {
   currentInstance,
   ComponentInternalInstance,
-  isInSSRComponentSetup
+  isInSSRComponentSetup,
+  setCurrentInstance,
+  unsetCurrentInstance
 } from './component'
 import {
   ErrorCodes,
@@ -157,8 +159,7 @@ export function watch<T = any, Immediate extends Readonly<boolean> = false>(
 function doWatch(
   source: WatchSource | WatchSource[] | WatchEffect | object,
   cb: WatchCallback | null,
-  { immediate, deep, flush, onTrack, onTrigger }: WatchOptions = EMPTY_OBJ,
-  instance = currentInstance
+  { immediate, deep, flush, onTrack, onTrigger }: WatchOptions = EMPTY_OBJ
 ): WatchStopHandle {
   if (__DEV__ && !cb) {
     if (immediate !== undefined) {
@@ -184,6 +185,7 @@ function doWatch(
     )
   }
 
+  const instance = currentInstance
   let getter: () => any
   let forceTrigger = false
   let isMultiSource = false
@@ -340,8 +342,7 @@ function doWatch(
     }
   }
 
-  const scope = instance && instance.scope
-  const effect = new ReactiveEffect(getter, scheduler, scope)
+  const effect = new ReactiveEffect(getter, scheduler)
 
   if (__DEV__) {
     effect.onTrack = onTrack
@@ -366,8 +367,8 @@ function doWatch(
 
   return () => {
     effect.stop()
-    if (scope) {
-      remove(scope.effects!, effect)
+    if (instance && instance.scope) {
+      remove(instance.scope.effects!, effect)
     }
   }
 }
@@ -392,7 +393,15 @@ export function instanceWatch(
     cb = value.handler as Function
     options = value
   }
-  return doWatch(getter, cb.bind(publicThis), options, this)
+  const cur = currentInstance
+  setCurrentInstance(this)
+  const res = doWatch(getter, cb.bind(publicThis), options)
+  if (cur) {
+    setCurrentInstance(cur)
+  } else {
+    unsetCurrentInstance()
+  }
+  return res
 }
 
 export function createPathGetter(ctx: any, path: string) {

packages/runtime-core/src/renderer.ts

@@ -2304,9 +2304,8 @@ function baseCreateRenderer(
       instance.emit('hook:beforeDestroy')
     }
 
-    if (scope) {
-      scope.stop()
-    }
+    // stop effects in component scope
+    scope.stop()
 
     // update may be null if a component is unmounted before its async
     // setup has resolved.