fix(utils): prevent watch context leak across async callbacks (#1229)

The `watch` util used a module-level `currentCleanups` to attach nested
watches to their parent. `revalidate` is async and kept this global set
across the `await` of a promise-returning callback, so any unrelated
watch created while the first was suspended had its cleanup attached to
the wrong watch. When that parent later revalidated or cleaned up, it ran
the unrelated watch's cleanup and unsubscribed it, leaving it permanently
unreactive.

Nested watches can only be registered during the synchronous part of the
callback, so restore the parent context immediately after the callback
returns, before awaiting any returned promise.

Author: chatman-media

src/vanilla/utils/watch.ts

@@ -75,12 +75,17 @@ export function watch(
 
     // Setup watch context, this allows us to automatically capture
     // watch cleanups if the watch callback itself has watch calls.
+    // Nested watches can only be created during the synchronous part of the
+    // callback, so the global context is restored right after the callback
+    // returns (before awaiting any returned promise). Keeping it set across
+    // the await would leak this watch's cleanups set into unrelated watches
+    // created while this one is suspended.
     const parent = currentCleanups
     currentCleanups = cleanups
 
-    // Ensures that the parent is reset if the callback throws an error.
+    let promiseOrPossibleCleanup: ReturnType<WatchCallback>
     try {
-      const promiseOrPossibleCleanup = callback((proxyObject) => {
+      promiseOrPossibleCleanup = callback((proxyObject) => {
         proxiesToSubscribe.add(proxyObject)
         // in case the callback is a promise and the watch has ended
         if (alive && !subscriptions.has(proxyObject)) {
@@ -90,23 +95,26 @@ export function watch(
         }
         return proxyObject
       })
-      const couldBeCleanup =
-        promiseOrPossibleCleanup && promiseOrPossibleCleanup instanceof Promise
-          ? await promiseOrPossibleCleanup
-          : promiseOrPossibleCleanup
-
-      // If there's a cleanup, we add this to the cleanups set
-      if (couldBeCleanup) {
-        if (alive) {
-          cleanups.add(couldBeCleanup)
-        } else {
-          cleanup()
-        }
-      }
     } finally {
+      // Restore the parent context before awaiting, so it is not corrupted by
+      // (or does not corrupt) revalidations that run while we are suspended.
       currentCleanups = parent
     }
 
+    const couldBeCleanup =
+      promiseOrPossibleCleanup && promiseOrPossibleCleanup instanceof Promise
+        ? await promiseOrPossibleCleanup
+        : promiseOrPossibleCleanup
+
+    // If there's a cleanup, we add this to the cleanups set
+    if (couldBeCleanup) {
+      if (alive) {
+        cleanups.add(couldBeCleanup)
+      } else {
+        cleanup()
+      }
+    }
+
     // Unsubscribe old subscriptions
     subscriptions.forEach((unsubscribe, proxyObject) => {
       if (!proxiesToSubscribe.has(proxyObject)) {

tests/watch.test.tsx

@@ -164,4 +164,43 @@ describe('watch', () => {
     await vi.advanceTimersByTimeAsync(10000)
     expect(callback).toBeCalledTimes(1)
   })
+
+  it('should not capture unrelated watches created while an async watch is pending (#1183)', async () => {
+    const A = proxy({ value: 0 })
+    const B = proxy({ value: 0 })
+
+    const cbA = vi.fn()
+    const cbB = vi.fn()
+
+    // watch1 is async and suspends at its await. While it is suspended, the
+    // shared watch context must not leak watch1's cleanups set.
+    watch(async (get) => {
+      get(A)
+      await sleep(1000)
+      cbA()
+    })
+
+    // While watch1 is suspended, create an independent watch2. Its cleanup
+    // must not be attached to watch1.
+    await vi.advanceTimersByTimeAsync(500)
+    watch((get) => {
+      get(B)
+      cbB()
+    })
+    expect(cbB).toBeCalledTimes(1)
+
+    await vi.advanceTimersByTimeAsync(500)
+    expect(cbA).toBeCalledTimes(1)
+
+    // Trigger watch1 to revalidate, which runs its own cleanups. If watch2's
+    // cleanup had leaked into watch1, this would unsubscribe watch2.
+    A.value = 1
+    await vi.advanceTimersByTimeAsync(2000)
+
+    // watch2 must still react to B updates.
+    cbB.mockClear()
+    B.value = 1
+    await vi.advanceTimersByTimeAsync(0)
+    expect(cbB).toBeCalledTimes(1)
+  })
 })