fix(reactivity): do not track deps in watcher callbacks run inside another effect (#15782)

close #5009

Author: parsajiravand

packages/reactivity/__tests__/watch.spec.ts

@@ -5,6 +5,7 @@ import {
   type WatchOptions,
   type WatchScheduler,
   computed,
+  effect,
   onWatcherCleanup,
   ref,
   watch,
@@ -342,4 +343,55 @@ describe('watch', () => {
     expect(() => (source.value = 2)).not.toThrow()
     expect(callback).toHaveBeenCalledTimes(1)
   })
+
+  // #5009
+  test('should not track deps of cleanup and callback run inside another effect', () => {
+    const source = ref(0)
+    const dep = ref(0)
+    const cb = vi.fn(() => {
+      dep.value
+      onWatcherCleanup(() => dep.value)
+    })
+    watch(source, cb)
+    source.value++
+    expect(cb).toHaveBeenCalledTimes(1)
+
+    const fn = vi.fn(() => {
+      source.value = 10
+    })
+    effect(fn)
+    expect(cb).toHaveBeenCalledTimes(2)
+    expect(fn).toHaveBeenCalledTimes(1)
+
+    dep.value++
+    expect(fn).toHaveBeenCalledTimes(1)
+  })
+
+  test('should restore tracking when cleanup throws inside another effect', () => {
+    const source = ref(0)
+    const dep = ref(0)
+    const err = new Error('cleanup error')
+    watch(source, () => {
+      onWatcherCleanup(() => {
+        throw err
+      })
+    })
+    source.value++
+
+    let dummy
+    let caught
+    effect(() => {
+      try {
+        source.value = 10
+      } catch (e) {
+        caught = e
+      }
+      dummy = dep.value
+    })
+    expect(caught).toBe(err)
+    expect(dummy).toBe(0)
+
+    dep.value++
+    expect(dummy).toBe(1)
+  })
 })

packages/reactivity/src/watch.ts

@@ -251,13 +251,14 @@ export function watch(
           ? (newValue as any[]).some((v, i) => hasChanged(v, oldValue[i]))
           : hasChanged(newValue, oldValue))
       ) {
-        // cleanup before running cb again
-        if (cleanup) {
-          cleanup()
-        }
         const currentWatcher = activeWatcher
-        activeWatcher = effect
+        pauseTracking()
         try {
+          // cleanup before running cb again
+          if (cleanup) {
+            cleanup()
+          }
+          activeWatcher = effect
           const args = [
             newValue,
             // pass undefined as the old value when it's changed for the first time
@@ -275,6 +276,7 @@ export function watch(
               cb!(...args)
         } finally {
           activeWatcher = currentWatcher
+          resetTracking()
         }
       }
     } else {

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

@@ -1424,6 +1424,47 @@ describe('api: watch', () => {
     expect(updated).toHaveBeenCalledTimes(1)
   })
 
+  // #5009
+  test('sync watcher callbacks should not track dependencies', async () => {
+    const a = ref(0)
+    const b = ref(0)
+    const updated = vi.fn()
+
+    const Child = defineComponent({
+      props: ['a'],
+      updated,
+      watch: {
+        a: {
+          handler() {
+            b.value
+          },
+          flush: 'sync',
+        },
+      },
+      render() {
+        return h('div', this.a)
+      },
+    })
+
+    const Parent = defineComponent({
+      render() {
+        return h(Child, { a: a.value })
+      },
+    })
+
+    const root = nodeOps.createElement('div')
+    createApp(Parent).mount(root)
+
+    a.value++
+    await nextTick()
+    expect(updated).toHaveBeenCalledTimes(1)
+
+    b.value++
+    await nextTick()
+    // should not track b as dependency of Child
+    expect(updated).toHaveBeenCalledTimes(1)
+  })
+
   test('watching keypath', async () => {
     const spy = vi.fn()
     const Comp = defineComponent({