refactor(baseWatch): rename onEffectCleanup to onWatcherCleanup and getCurrentEffect to getCurrentWatcher for clarity

Author: LittleSound

packages/reactivity/__tests__/baseWatch.spec.ts

@@ -5,7 +5,7 @@ import {
   type SchedulerJob,
   type WatchScheduler,
   baseWatch,
-  onEffectCleanup,
+  onWatcherCleanup,
   ref,
 } from '../src'
 
@@ -72,7 +72,7 @@ describe('baseWatch', () => {
     const effect = baseWatch(
       source,
       () => {
-        onEffectCleanup(() => {
+        onWatcherCleanup(() => {
           throw 'oops in cleanup'
         })
         throw 'oops in watch'
@@ -102,7 +102,7 @@ describe('baseWatch', () => {
     ])
   })
 
-  test('baseWatch with onEffectCleanup', async () => {
+  test('baseWatch with onWatcherCleanup', async () => {
     let dummy = 0
     let source: Ref<number>
     const scope = new EffectScope()
@@ -113,8 +113,8 @@ describe('baseWatch', () => {
         source.value
 
         onCleanup(() => (dummy += 2))
-        onEffectCleanup(() => (dummy += 3))
-        onEffectCleanup(() => (dummy += 5))
+        onWatcherCleanup(() => (dummy += 3))
+        onWatcherCleanup(() => (dummy += 5))
       })
     })
     expect(dummy).toBe(0)
@@ -133,7 +133,7 @@ describe('baseWatch', () => {
     expect(dummy).toBe(30)
   })
 
-  test('nested calls to baseWatch and onEffectCleanup', async () => {
+  test('nested calls to baseWatch and onWatcherCleanup', async () => {
     let calls: string[] = []
     let source: Ref<number>
     let copyist: Ref<number>
@@ -146,7 +146,7 @@ describe('baseWatch', () => {
       baseWatch(
         () => {
           const current = (copyist.value = source.value)
-          onEffectCleanup(() => calls.push(`sync ${current}`))
+          onWatcherCleanup(() => calls.push(`sync ${current}`))
         },
         null,
         {},
@@ -155,7 +155,7 @@ describe('baseWatch', () => {
       baseWatch(
         () => {
           const current = copyist.value
-          onEffectCleanup(() => calls.push(`post ${current}`))
+          onWatcherCleanup(() => calls.push(`post ${current}`))
         },
         null,
         { scheduler },
@@ -180,6 +180,7 @@ describe('baseWatch', () => {
     scope.stop()
     expect(calls).toEqual(['sync 2', 'post 2'])
   })
+
   test('baseWatch with middleware', async () => {
     let effectCalls: string[] = []
     let watchCalls: string[] = []
@@ -190,7 +191,7 @@ describe('baseWatch', () => {
       () => {
         source.value
         effectCalls.push('effect')
-        onEffectCleanup(() => effectCalls.push('effect cleanup'))
+        onWatcherCleanup(() => effectCalls.push('effect cleanup'))
       },
       null,
       {
@@ -207,7 +208,7 @@ describe('baseWatch', () => {
       () => source.value,
       () => {
         watchCalls.push('watch')
-        onEffectCleanup(() => watchCalls.push('watch cleanup'))
+        onWatcherCleanup(() => watchCalls.push('watch cleanup'))
       },
       {
         scheduler,

packages/reactivity/src/baseWatch.ts

@@ -15,7 +15,7 @@ import type { ComputedRef } from './computed'
 import { ReactiveFlags } from './constants'
 import {
   type DebuggerOptions,
-  type EffectScheduler,
+  EffectFlags,
   ReactiveEffect,
   pauseTracking,
   resetTracking,
@@ -46,7 +46,7 @@ export interface BaseWatchOptions<Immediate = boolean> extends DebuggerOptions {
   immediate?: Immediate
   deep?: boolean
   once?: boolean
-  scheduler?: Scheduler
+  scheduler?: WatchScheduler
   middleware?: BaseWatchMiddleware
   onError?: HandleError
   onWarn?: HandleWarn
@@ -55,22 +55,41 @@ export interface BaseWatchOptions<Immediate = boolean> extends DebuggerOptions {
 // initial value for watchers to trigger on undefined initial values
 const INITIAL_WATCHER_VALUE = {}
 
-export type Scheduler = (
+export type WatchScheduler = (
   job: SchedulerJob,
   effect: ReactiveEffect,
-  isInit: boolean,
+  immediateFirstRun: boolean,
+  hasCb: boolean,
 ) => void
 export type BaseWatchMiddleware = (next: () => unknown) => any
 export type HandleError = (err: unknown, type: BaseWatchErrorCodes) => void
 export type HandleWarn = (msg: string, ...args: any[]) => void
 
-const DEFAULT_SCHEDULER: Scheduler = job => job()
+const DEFAULT_SCHEDULER: WatchScheduler = (
+  job,
+  effect,
+  immediateFirstRun,
+  hasCb,
+) => {
+  if (immediateFirstRun) {
+    !hasCb && effect.run()
+  } else {
+    job()
+  }
+}
 const DEFAULT_HANDLE_ERROR: HandleError = (err: unknown) => {
   throw err
 }
 
 const cleanupMap: WeakMap<ReactiveEffect, (() => void)[]> = new WeakMap()
-let activeEffect: ReactiveEffect | undefined = undefined
+let activeWatcher: ReactiveEffect | undefined = undefined
+
+/**
+ * Returns the current active effect if there is one.
+ */
+export function getCurrentWatcher() {
+  return activeWatcher
+}
 
 /**
  * Registers a cleanup callback on the current active effect. This
@@ -79,15 +98,15 @@ let activeEffect: ReactiveEffect | undefined = undefined
  *
  * @param cleanupFn - The callback function to attach to the effect's cleanup.
  */
-export function onEffectCleanup(cleanupFn: () => void) {
-  if (activeEffect) {
+export function onWatcherCleanup(cleanupFn: () => void, failSilently = false) {
+  if (activeWatcher) {
     const cleanups =
-      cleanupMap.get(activeEffect) ||
-      cleanupMap.set(activeEffect, []).get(activeEffect)!
+      cleanupMap.get(activeWatcher) ||
+      cleanupMap.set(activeWatcher, []).get(activeWatcher)!
     cleanups.push(cleanupFn)
-  } else if (__DEV__) {
+  } else if (__DEV__ && !failSilently) {
     warn(
-      `onEffectCleanup() was called when there was no active effect` +
+      `onWatcherCleanup() was called when there was no active watcher` +
         ` to associate with.`,
     )
   }
@@ -170,17 +189,17 @@ export function baseWatch(
             resetTracking()
           }
         }
-        const currentEffect = activeEffect
-        activeEffect = effect
+        const currentEffect = activeWatcher
+        activeWatcher = effect
         try {
           return callWithAsyncErrorHandling(
             source,
             onError,
             BaseWatchErrorCodes.WATCH_CALLBACK,
-            [onEffectCleanup],
+            [onWatcherCleanup],
           )
         } finally {
-          activeEffect = currentEffect
+          activeWatcher = currentEffect
         }
       }
       if (middleware) {
@@ -198,30 +217,19 @@ export function baseWatch(
     getter = () => traverse(baseGetter())
   }
 
-  const scope = getCurrentScope()
-
   if (once) {
-    if (!cb) {
-      // onEffectCleanup need use effect as a key
-      scope?.effects.push((effect = {} as any))
-      getter()
-      return
-    }
-    if (immediate) {
-      // onEffectCleanup need use effect as a key
-      scope?.effects.push((effect = {} as any))
-      callWithAsyncErrorHandling(
-        cb,
-        onError,
-        BaseWatchErrorCodes.WATCH_CALLBACK,
-        [getter(), isMultiSource ? [] : undefined, onEffectCleanup],
-      )
-      return
-    }
-    const _cb = cb
-    cb = (...args) => {
-      _cb(...args)
-      effect?.stop()
+    if (cb) {
+      const _cb = cb
+      cb = (...args) => {
+        _cb(...args)
+        effect?.stop()
+      }
+    } else {
+      const _getter = getter
+      getter = () => {
+        _getter()
+        effect?.stop()
+      }
     }
   }
 
@@ -250,8 +258,8 @@ export function baseWatch(
           if (cleanup) {
             cleanup()
           }
-          const currentEffect = activeEffect
-          activeEffect = effect
+          const currentWatcher = activeWatcher
+          activeWatcher = effect
           try {
             callWithAsyncErrorHandling(
               cb!,
@@ -265,12 +273,12 @@ export function baseWatch(
                   : isMultiSource && oldValue[0] === INITIAL_WATCHER_VALUE
                     ? []
                     : oldValue,
-                onEffectCleanup,
+                onWatcherCleanup,
               ],
             )
             oldValue = newValue
           } finally {
-            activeEffect = currentEffect
+            activeWatcher = currentWatcher
           }
         }
         if (middleware) {

packages/reactivity/src/index.ts

@@ -80,11 +80,12 @@ export { reactiveReadArray, shallowReadArray } from './arrayInstrumentations'
 export { TrackOpTypes, TriggerOpTypes, ReactiveFlags } from './constants'
 export {
   baseWatch,
-  onEffectCleanup,
+  getCurrentWatcher,
   traverse,
+  onWatcherCleanup,
   BaseWatchErrorCodes,
   type BaseWatchOptions,
   type BaseWatchMiddleware,
-  type Scheduler,
-  type SchedulerJob,
+  type WatchScheduler,
 } from './baseWatch'
+export { type SchedulerJob, SchedulerJobFlags } from './scheduler'

packages/reactivity/src/scheduler.ts

@@ -0,0 +1,30 @@
+export enum SchedulerJobFlags {
+  QUEUED = 1 << 0,
+  PRE = 1 << 1,
+  /**
+   * Indicates whether the effect is allowed to recursively trigger itself
+   * when managed by the scheduler.
+   *
+   * By default, a job cannot trigger itself because some built-in method calls,
+   * e.g. Array.prototype.push actually performs reads as well (#1740) which
+   * can lead to confusing infinite loops.
+   * The allowed cases are component update functions and watch callbacks.
+   * Component update functions may update child component props, which in turn
+   * trigger flush: "pre" watch callbacks that mutates state that the parent
+   * relies on (#1801). Watch callbacks doesn't track its dependencies so if it
+   * triggers itself again, it's likely intentional and it is the user's
+   * responsibility to perform recursive state mutation that eventually
+   * stabilizes (#1727).
+   */
+  ALLOW_RECURSE = 1 << 2,
+  DISPOSED = 1 << 3,
+}
+
+export interface SchedulerJob extends Function {
+  id?: number
+  /**
+   * flags can technically be undefined, but it can still be used in bitwise
+   * operations just like 0.
+   */
+  flags?: SchedulerJobFlags
+}

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

@@ -5,7 +5,7 @@ import {
   defineComponent,
   getCurrentInstance,
   nextTick,
-  onEffectCleanup,
+  onWatcherCleanup,
   reactive,
   ref,
   watch,
@@ -395,17 +395,17 @@ describe('api: watch', () => {
     expect(cleanup).toHaveBeenCalledTimes(2)
   })
 
-  it('onEffectCleanup', async () => {
+  it('onWatcherCleanup', async () => {
     const count = ref(0)
     const cleanupEffect = vi.fn()
     const cleanupWatch = vi.fn()
 
     const stopEffect = watchEffect(() => {
-      onEffectCleanup(cleanupEffect)
+      onWatcherCleanup(cleanupEffect)
       count.value
     })
     const stopWatch = watch(count, () => {
-      onEffectCleanup(cleanupWatch)
+      onWatcherCleanup(cleanupWatch)
     })
 
     count.value++

packages/runtime-core/src/index.ts

@@ -28,7 +28,7 @@ export {
   // effect
   effect,
   stop,
-  onEffectCleanup,
+  onWatcherCleanup,
   ReactiveEffect,
   // effect scope
   effectScope,

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

@@ -2,13 +2,13 @@ import type { Ref } from '@vue/reactivity'
 import {
   EffectScope,
   nextTick,
-  onEffectCleanup,
+  onWatcherCleanup,
   ref,
   watchEffect,
   watchSyncEffect,
 } from '../src'
 
-describe('watchEffect and onEffectCleanup', () => {
+describe('watchEffect and onWatcherCleanup', () => {
   test('basic', async () => {
     let dummy = 0
     let source: Ref<number>
@@ -20,8 +20,8 @@ describe('watchEffect and onEffectCleanup', () => {
         source.value
 
         onCleanup(() => (dummy += 2))
-        onEffectCleanup(() => (dummy += 3))
-        onEffectCleanup(() => (dummy += 5))
+        onWatcherCleanup(() => (dummy += 3))
+        onWatcherCleanup(() => (dummy += 5))
       })
     })
     await nextTick()
@@ -55,11 +55,11 @@ describe('watchEffect and onEffectCleanup', () => {
       double = ref(0)
       watchEffect(() => {
         double.value = source.value * 2
-        onEffectCleanup(() => (dummy += 2))
+        onWatcherCleanup(() => (dummy += 2))
       })
       watchSyncEffect(() => {
         double.value
-        onEffectCleanup(() => (dummy += 3))
+        onWatcherCleanup(() => (dummy += 3))
       })
     })
     await nextTick()