test: more test cases for computed w/ scheduler

yyx990803 · yyx990803 · commit 2dd4739089fd · 2021-07-16T14:30:49.000-04:00
diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts
@@ -272,6 +272,68 @@ describe('reactivity/computed', () => {
       expect(spy).toHaveBeenCalledTimes(2)
     })
 
+    test('chained computed trigger', async () => {
+      const effectSpy = jest.fn()
+      const c1Spy = jest.fn()
+      const c2Spy = jest.fn()
+
+      const src = ref(0)
+      const c1 = computed(() => {
+        c1Spy()
+        return src.value % 2
+      })
+      const c2 = computed(() => {
+        c2Spy()
+        return c1.value + 1
+      })
+
+      effect(() => {
+        effectSpy(c2.value)
+      })
+
+      expect(c1Spy).toHaveBeenCalledTimes(1)
+      expect(c2Spy).toHaveBeenCalledTimes(1)
+      expect(effectSpy).toHaveBeenCalledTimes(1)
+
+      src.value = 1
+      await tick
+      expect(c1Spy).toHaveBeenCalledTimes(2)
+      expect(c2Spy).toHaveBeenCalledTimes(2)
+      expect(effectSpy).toHaveBeenCalledTimes(2)
+    })
+
+    test('chained computed avoid re-compute', async () => {
+      const effectSpy = jest.fn()
+      const c1Spy = jest.fn()
+      const c2Spy = jest.fn()
+
+      const src = ref(0)
+      const c1 = computed(() => {
+        c1Spy()
+        return src.value % 2
+      })
+      const c2 = computed(() => {
+        c2Spy()
+        return c1.value + 1
+      })
+
+      effect(() => {
+        effectSpy(c2.value)
+      })
+
+      expect(effectSpy).toHaveBeenCalledTimes(1)
+      src.value = 2
+      src.value = 4
+      src.value = 6
+      await tick
+      // c1 should re-compute once.
+      expect(c1Spy).toHaveBeenCalledTimes(2)
+      // c2 should not have to re-compute because c1 did not change.
+      expect(c2Spy).toHaveBeenCalledTimes(1)
+      // effect should not trigger because c2 did not change.
+      expect(effectSpy).toHaveBeenCalledTimes(1)
+    })
+
     test('chained computed value invalidation', async () => {
       const effectSpy = jest.fn()
       const c1Spy = jest.fn()
@@ -302,25 +364,33 @@ describe('reactivity/computed', () => {
       // value should be available sync
       expect(c2.value).toBe(2)
       expect(c2Spy).toHaveBeenCalledTimes(2)
-      await tick
-      expect(effectSpy).toHaveBeenCalledTimes(2)
-      expect(effectSpy).toHaveBeenCalledWith(2)
-      expect(c1Spy).toHaveBeenCalledTimes(2)
-      expect(c2Spy).toHaveBeenCalledTimes(2)
+    })
 
-      src.value = 2
-      await tick
-      expect(effectSpy).toHaveBeenCalledTimes(3)
-      expect(c1Spy).toHaveBeenCalledTimes(3)
-      expect(c2Spy).toHaveBeenCalledTimes(3)
+    test('sync access of invalidated chained computed should not prevent final effect from running', async () => {
+      const effectSpy = jest.fn()
+      const c1Spy = jest.fn()
+      const c2Spy = jest.fn()
 
-      src.value = 4
+      const src = ref(0)
+      const c1 = computed(() => {
+        c1Spy()
+        return src.value % 2
+      })
+      const c2 = computed(() => {
+        c2Spy()
+        return c1.value + 1
+      })
+
+      effect(() => {
+        effectSpy(c2.value)
+      })
+      expect(effectSpy).toHaveBeenCalledTimes(1)
+
+      src.value = 1
+      // sync access c2
+      c2.value
       await tick
-      expect(effectSpy).toHaveBeenCalledTimes(3)
-      expect(c1Spy).toHaveBeenCalledTimes(4)
-      // in-between chained computed always re-compute, but it does avoid
-      // triggering the final subscribing effect.
-      expect(c2Spy).toHaveBeenCalledTimes(4)
+      expect(effectSpy).toHaveBeenCalledTimes(2)
     })
   })
 })
diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts
@@ -1,9 +1,8 @@
-import { ReactiveEffect, triggerEffects } from './effect'
+import { ReactiveEffect } from './effect'
 import { Ref, trackRefValue, triggerRefValue } from './ref'
 import { isFunction, NOOP } from '@vue/shared'
 import { ReactiveFlags, toRaw } from './reactive'
 import { Dep } from './dep'
-import { TriggerOpTypes } from '@vue/runtime-core'
 
 export interface ComputedRef<T = any> extends WritableComputedRef<T> {
   readonly value: T
@@ -36,8 +35,6 @@ class ComputedRefImpl<T> {
 
   private _value!: T
   private _dirty = true
-  private _changed = false
-
   public readonly effect: ReactiveEffect<T>
 
   public readonly __v_isRef = true;
@@ -48,60 +45,55 @@ class ComputedRefImpl<T> {
     private readonly _setter: ComputedSetter<T>,
     isReadonly: boolean
   ) {
-    this.effect = new ReactiveEffect(getter, () => {
-      if (!this._dirty) {
-        this._dirty = true
-        if (scheduler) {
-          if (this.dep) {
-            const effects: ReactiveEffect[] = []
-            scheduler(() => {
-              if ((this._get(), this._changed)) {
-                if (__DEV__) {
-                  triggerEffects(effects, {
-                    target: this,
-                    type: TriggerOpTypes.SET,
-                    key: 'value',
-                    newValue: this._value
-                  })
-                } else {
-                  triggerEffects(effects)
-                }
-              }
-            })
-            // chained upstream computeds are notified synchronously to ensure
-            // value invalidation in case of sync access; normal effects are
-            // deferred to be triggered in scheduler.
-            for (const e of this.dep) {
-              if (e.computed) {
-                e.scheduler!()
-              } else {
-                effects.push(e)
-              }
+    let compareTarget: any
+    let hasCompareTarget = false
+    let scheduled = false
+    this.effect = new ReactiveEffect(getter, (computedTrigger?: boolean) => {
+      if (scheduler && this.dep) {
+        if (computedTrigger) {
+          compareTarget = this._value
+          hasCompareTarget = true
+        } else if (!scheduled) {
+          const valueToCompare = hasCompareTarget ? compareTarget : this._value
+          scheduled = true
+          hasCompareTarget = false
+          scheduler(() => {
+            if (this._get() !== valueToCompare) {
+              triggerRefValue(this)
             }
+            scheduled = false
+          })
+        }
+        // chained upstream computeds are notified synchronously to ensure
+        // value invalidation in case of sync access; normal effects are
+        // deferred to be triggered in scheduler.
+        for (const e of this.dep) {
+          if (e.computed) {
+            e.scheduler!(true /* computedTrigger */)
           }
-        } else {
-          triggerRefValue(this)
         }
       }
+      if (!this._dirty) {
+        this._dirty = true
+        if (!scheduler) triggerRefValue(this)
+      }
     })
     this.effect.computed = true
     this[ReactiveFlags.IS_READONLY] = isReadonly
   }
 
   private _get() {
     if (this._dirty) {
-      const oldValue = this._value
-      this._changed = oldValue !== (this._value = this.effect.run()!)
       this._dirty = false
+      return (this._value = this.effect.run()!)
     }
+    return this._value
   }
 
   get value() {
+    trackRefValue(this)
     // the computed ref may get wrapped by other proxies e.g. readonly() #3376
-    const self = toRaw(this)
-    self._get()
-    trackRefValue(self)
-    return self._value
+    return toRaw(this)._get()
   }
 
   set value(newValue: T) {
diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts
@@ -29,7 +29,7 @@ export let trackOpBit = 1
  */
 const maxMarkerBits = 30
 
-export type EffectScheduler = () => void
+export type EffectScheduler = (...args: any[]) => any
 
 export type DebuggerEvent = {
   effect: ReactiveEffect
