refactor: sync value access for chained computed w/ scheduler

diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts
index 112af2826dab77e9a446aa2df8f280f903bb49fc..632accadc5884e9063e1829b59a8a239b8eb8156 100644 (file)
--- a/packages/reactivity/__tests__/computed.spec.ts
+++ b/packages/reactivity/__tests__/computed.spec.ts
@@ -201,10 +201,29 @@ describe('reactivity/computed', () => {
   })
 
   describe('with scheduler', () => {
-    const p = Promise.resolve()
-    const defer = (fn?: any) => (fn ? p.then(fn) : p)
+    // a simple scheduler similar to the main Vue scheduler
+    const tick = Promise.resolve()
+    const queue: any[] = []
+    let queued = false
+
+    const schedule = (fn: any) => {
+      queue.push(fn)
+      if (!queued) {
+        queued = true
+        tick.then(flush)
+      }
+    }
+
+    const flush = () => {
+      for (let i = 0; i < queue.length; i++) {
+        queue[i]()
+      }
+      queue.length = 0
+      queued = false
+    }
+
     beforeEach(() => {
-      setComputedScheduler(defer)
+      setComputedScheduler(schedule)
     })
 
     afterEach(() => {
@@ -224,7 +243,7 @@ describe('reactivity/computed', () => {
       src.value = 3
       // not called yet
       expect(spy).toHaveBeenCalledTimes(1)
-      await defer()
+      await tick
       // should only trigger once
       expect(spy).toHaveBeenCalledTimes(2)
       expect(spy).toHaveBeenCalledWith(c.value)
@@ -241,16 +260,67 @@ describe('reactivity/computed', () => {
       src.value = 1
       src.value = 2
 
-      await defer()
+      await tick
       // should not trigger
       expect(spy).toHaveBeenCalledTimes(1)
 
       src.value = 3
       src.value = 4
       src.value = 5
-      await defer()
+      await tick
       // should trigger because latest value changes
       expect(spy).toHaveBeenCalledTimes(2)
     })
+
+    test('chained computed value invalidation', 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)
+      expect(effectSpy).toHaveBeenCalledWith(1)
+      expect(c2.value).toBe(1)
+
+      expect(c1Spy).toHaveBeenCalledTimes(1)
+      expect(c2Spy).toHaveBeenCalledTimes(1)
+
+      src.value = 1
+      // 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)
+
+      src.value = 4
+      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)
+    })
   })
 })

diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts
index 514405970a6e822cc89d0c93c2f0f98a7235dfa0..ab4569bafc386bdb357a7fd3e363e4c05f54ffd1 100644 (file)
--- a/packages/reactivity/src/computed.ts
+++ b/packages/reactivity/src/computed.ts
@@ -1,8 +1,9 @@
-import { ReactiveEffect } from './effect'
+import { ReactiveEffect, triggerEffects } 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
@@ -35,6 +36,7 @@ class ComputedRefImpl<T> {
 
   private _value!: T
   private _dirty = true
+  private _changed = false
 
   public readonly effect: ReactiveEffect<T>
 
@@ -46,46 +48,58 @@ class ComputedRefImpl<T> {
     private readonly _setter: ComputedSetter<T>,
     isReadonly: boolean
   ) {
-    let deferFn: () => void
-    let scheduled = false
     this.effect = new ReactiveEffect(getter, () => {
       if (!this._dirty) {
         this._dirty = true
         if (scheduler) {
-          if (!scheduled) {
-            scheduled = true
-            scheduler(
-              deferFn ||
-                (deferFn = () => {
-                  scheduled = false
-                  if (this._dirty) {
-                    this._dirty = false
-                    const newValue = this.effect.run()!
-                    if (this._value !== newValue) {
-                      this._value = newValue
-                      triggerRefValue(this)
-                    }
-                  } else {
-                    triggerRefValue(this)
-                  }
-                })
-            )
+          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)
+              }
+            }
           }
         } else {
           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
+    }
+  }
+
   get value() {
     // the computed ref may get wrapped by other proxies e.g. readonly() #3376
     const self = toRaw(this)
-    if (self._dirty) {
-      self._value = self.effect.run()!
-      self._dirty = false
-    }
+    self._get()
     trackRefValue(self)
     return self._value
   }

diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts
index e602e369d39c6142263c472581bc23c0e4a9f70d..7efb54e0f05f02a7da0135d8a713bb97f7366ed7 100644 (file)
--- a/packages/reactivity/src/effect.ts
+++ b/packages/reactivity/src/effect.ts
@@ -55,6 +55,8 @@ export class ReactiveEffect<T = any> {
   deps: Dep[] = []
 
   // can be attached after creation
+  computed?: boolean
+  allowRecurse?: boolean
   onStop?: () => void
   // dev only
   onTrack?: (event: DebuggerEvent) => void
@@ -64,9 +66,7 @@ export class ReactiveEffect<T = any> {
   constructor(
     public fn: () => T,
     public scheduler: EffectScheduler | null = null,
-    scope?: EffectScope | null,
-    // allow recursive self-invocation
-    public allowRecurse = false
+    scope?: EffectScope | null
   ) {
     recordEffectScope(this, scope)
   }
@@ -303,7 +303,11 @@ export function trigger(
 
   if (deps.length === 1) {
     if (deps[0]) {
-      triggerEffects(deps[0], eventInfo)
+      if (__DEV__) {
+        triggerEffects(deps[0], eventInfo)
+      } else {
+        triggerEffects(deps[0])
+      }
     }
   } else {
     const effects: ReactiveEffect[] = []
@@ -312,16 +316,20 @@ export function trigger(
         effects.push(...dep)
       }
     }
-    triggerEffects(createDep(effects), eventInfo)
+    if (__DEV__) {
+      triggerEffects(createDep(effects), eventInfo)
+    } else {
+      triggerEffects(createDep(effects))
+    }
   }
 }
 
 export function triggerEffects(
-  dep: Dep,
+  dep: Dep | ReactiveEffect[],
   debuggerEventExtraInfo?: DebuggerEventExtraInfo
 ) {
   // spread into array for stabilization
-  for (const effect of [...dep]) {
+  for (const effect of isArray(dep) ? dep : [...dep]) {
     if (effect !== activeEffect || effect.allowRecurse) {
       if (__DEV__ && effect.onTrigger) {
         effect.onTrigger(extend({ effect }, debuggerEventExtraInfo))

diff --git a/packages/runtime-core/src/renderer.ts b/packages/runtime-core/src/renderer.ts
index e021c761dea4ce78ffe605277d62eb8c6e6c2103..cfa175301e35f816f2bd85ca8c7c2093acfe789c 100644 (file)
--- a/packages/runtime-core/src/renderer.ts
+++ b/packages/runtime-core/src/renderer.ts
@@ -1630,15 +1630,14 @@ function baseCreateRenderer(
     const effect = new ReactiveEffect(
       componentUpdateFn,
       () => queueJob(instance.update),
-      instance.scope, // track it in component's effect scope
-      true /* allowRecurse */
+      instance.scope // track it in component's effect scope
     )
 
     const update = (instance.update = effect.run.bind(effect) as SchedulerJob)
     update.id = instance.uid
     // allowRecurse
     // #1801, #2043 component render effects should allow recursive updates
-    update.allowRecurse = true
+    effect.allowRecurse = update.allowRecurse = true
 
     if (__DEV__) {
       effect.onTrack = instance.rtc