fix(reactivity): scheduled effect should not execute if stopped

fix #910

diff --git a/packages/reactivity/__tests__/effect.spec.ts b/packages/reactivity/__tests__/effect.spec.ts
index 3ce0748b5395e07abfd58df879e70b4652b3048b..24ade5b7ca8d432e6ab2afb5a3698d694d142b1b 100644 (file)
--- a/packages/reactivity/__tests__/effect.spec.ts
+++ b/packages/reactivity/__tests__/effect.spec.ts
@@ -678,6 +678,28 @@ describe('reactivity/effect', () => {
     expect(dummy).toBe(3)
   })
 
+  it('stop with scheduler', () => {
+    let dummy
+    const obj = reactive({ prop: 1 })
+    const queue: (() => void)[] = []
+    const runner = effect(
+      () => {
+        dummy = obj.prop
+      },
+      {
+        scheduler: e => queue.push(e)
+      }
+    )
+    obj.prop = 2
+    expect(dummy).toBe(1)
+    expect(queue.length).toBe(1)
+    stop(runner)
+
+    // a scheduled effect should not execute anymore after stopped
+    queue.forEach(e => e())
+    expect(dummy).toBe(1)
+  })
+
   it('events: onStop', () => {
     const onStop = jest.fn()
     const runner = effect(() => {}, {

diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts
index cc6d0a61d90e73e582e59b1d51ae0803f9f2b3bf..6a09640e88fb8b9576d5e815ece2eefaa0b448ed 100644 (file)
--- a/packages/reactivity/src/effect.ts
+++ b/packages/reactivity/src/effect.ts
@@ -10,7 +10,7 @@ type KeyToDepMap = Map<any, Dep>
 const targetMap = new WeakMap<any, KeyToDepMap>()
 
 export interface ReactiveEffect<T = any> {
-  (): T
+  (...args: any[]): T
   _isEffect: true
   active: boolean
   raw: () => T
@@ -75,11 +75,26 @@ export function stop(effect: ReactiveEffect) {
 }
 
 function createReactiveEffect<T = any>(
-  fn: () => T,
+  fn: (...args: any[]) => T,
   options: ReactiveEffectOptions
 ): ReactiveEffect<T> {
   const effect = function reactiveEffect(...args: unknown[]): unknown {
-    return run(effect, fn, args)
+    if (!effect.active) {
+      return options.scheduler ? undefined : fn(...args)
+    }
+    if (!effectStack.includes(effect)) {
+      cleanup(effect)
+      try {
+        enableTracking()
+        effectStack.push(effect)
+        activeEffect = effect
+        return fn(...args)
+      } finally {
+        effectStack.pop()
+        resetTracking()
+        activeEffect = effectStack[effectStack.length - 1]
+      }
+    }
   } as ReactiveEffect
   effect._isEffect = true
   effect.active = true
@@ -89,25 +104,6 @@ function createReactiveEffect<T = any>(
   return effect
 }
 
-function run(effect: ReactiveEffect, fn: Function, args: unknown[]): unknown {
-  if (!effect.active) {
-    return fn(...args)
-  }
-  if (!effectStack.includes(effect)) {
-    cleanup(effect)
-    try {
-      enableTracking()
-      effectStack.push(effect)
-      activeEffect = effect
-      return fn(...args)
-    } finally {
-      effectStack.pop()
-      resetTracking()
-      activeEffect = effectStack[effectStack.length - 1]
-    }
-  }
-}
-
 function cleanup(effect: ReactiveEffect) {
   const { deps } = effect
   if (deps.length) {