perf(reactivity): Array shift() should only trigger effect once

packages/reactivity/__tests__/computed.spec.ts

@@ -184,7 +184,7 @@ describe('reactivity/computed', () => {
     // mutate n
     n.value++
     // on the 2nd run, plusOne.value should have already updated.
-    expect(plusOneValues).toMatchObject([1, 2, 2])
+    expect(plusOneValues).toMatchObject([1, 2])
   })
 
   it('should warn if trying to set a readonly computed', () => {
@@ -288,4 +288,93 @@ describe('reactivity/computed', () => {
       oldValue: 2
     })
   })
+
+  it('chained computed value on-demand trigger', () => {
+    const minSpy = vi.fn()
+    const hourSpy = vi.fn()
+
+    const sec = ref(0)
+    const min = computed(() => {
+      minSpy()
+      return Math.floor(sec.value / 60)
+    })
+    const hour = computed(() => {
+      hourSpy()
+      return Math.floor(min.value / 60)
+    })
+
+    for (sec.value = 0; sec.value < 1000; sec.value++) {
+      hour.value
+    }
+
+    expect(minSpy).toHaveBeenCalledTimes(1000)
+    expect(hourSpy).toHaveBeenCalledTimes(17)
+  })
+
+  it('effect callback on-demand trigger', () => {
+    const effectSpy = vi.fn()
+
+    const sec = ref(0)
+    const min = computed(() => {
+      return Math.floor(sec.value / 60)
+    })
+    const hour = computed(() => {
+      return Math.floor(min.value / 60)
+    })
+
+    effect(() => {
+      effectSpy()
+      min.value
+      hour.value
+    })
+
+    for (sec.value = 0; sec.value < 1000; sec.value++) {}
+
+    expect(effectSpy).toHaveBeenCalledTimes(17)
+  })
+
+  it('chained computed value urgent assessment edge case', () => {
+    const cSpy = vi.fn()
+
+    const a = ref<null | { v: number }>({
+      v: 1
+    })
+    const b = computed(() => {
+      return a.value
+    })
+    const c = computed(() => {
+      cSpy()
+      return b.value?.v
+    })
+    const d = computed(() => {
+      if (b.value) {
+        return c.value
+      }
+      return 0
+    })
+
+    d.value
+    a.value!.v = 2
+    a.value = null
+    d.value
+    expect(cSpy).toHaveBeenCalledTimes(1)
+  })
+
+  test('should not continuous effects cause computed dirty race condition', () => {
+    const fnSpy = vi.fn()
+    const v = ref(1)
+    const c = computed(() => v.value)
+
+    effect(() => {
+      c.value
+    })
+    effect(() => {
+      c.value
+      fnSpy()
+    })
+
+    expect(fnSpy).toBeCalledTimes(1)
+    v.value = 2
+    expect(fnSpy).toBeCalledTimes(2)
+  })
 })

packages/reactivity/__tests__/effect.spec.ts

@@ -558,8 +558,8 @@ describe('reactivity/effect', () => {
     expect(output.fx2).toBe(1 + 3 + 3)
     expect(fx1Spy).toHaveBeenCalledTimes(1)
 
-    // Invoked twice due to change of fx1.
-    expect(fx2Spy).toHaveBeenCalledTimes(2)
+    // Invoked once even change of fx1.
+    expect(fx2Spy).toHaveBeenCalledTimes(1)
 
     fx1Spy.mockClear()
     fx2Spy.mockClear()

packages/reactivity/__tests__/reactiveArray.spec.ts

@@ -99,6 +99,20 @@ describe('reactivity/reactive/Array', () => {
     expect(fn).toHaveBeenCalledTimes(1)
   })
 
+  test('shift on Array should only trigger dependency once', () => {
+    const arr = reactive([1, 2, 3])
+    const fn = vi.fn()
+    effect(() => {
+      for (let i = 0; i < arr.length; i++) {
+        arr[i]
+      }
+      fn()
+    })
+    expect(fn).toHaveBeenCalledTimes(1)
+    arr.shift()
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
+
   test('add existing index on Array should not trigger length dependency', () => {
     const array = new Array(3)
     const observed = reactive(array)

packages/reactivity/src/baseHandlers.ts

@@ -17,7 +17,9 @@ import {
   trigger,
   ITERATE_KEY,
   pauseTracking,
-  resetTracking
+  resetTracking,
+  pauseScheduling,
+  resetScheduling
 } from './effect'
 import {
   isObject,
@@ -71,7 +73,9 @@ function createArrayInstrumentations() {
   ;(['push', 'pop', 'shift', 'unshift', 'splice'] as const).forEach(key => {
     instrumentations[key] = function (this: unknown[], ...args: unknown[]) {
       pauseTracking()
+      pauseScheduling()
       const res = (toRaw(this) as any)[key].apply(this, args)
+      resetScheduling()
       resetTracking()
       return res
     }

packages/reactivity/src/computed.ts

@@ -1,8 +1,9 @@
 import { DebuggerOptions, ReactiveEffect } from './effect'
 import { Ref, trackRefValue, triggerRefValue } from './ref'
-import { isFunction, NOOP } from '@vue/shared'
+import { hasChanged, isFunction, NOOP } from '@vue/shared'
 import { ReactiveFlags, toRaw } from './reactive'
 import { Dep } from './dep'
+import { DirtyLevels } from './operations'
 
 declare const ComputedRefSymbol: unique symbol
 
@@ -32,7 +33,7 @@ export class ComputedRefImpl<T> {
   public readonly __v_isRef = true
   public readonly [ReactiveFlags.IS_READONLY]: boolean = false
 
-  public _dirty = true
+  public _scheduled = false
   public _cacheable: boolean
 
   constructor(
@@ -42,9 +43,9 @@ export class ComputedRefImpl<T> {
     isSSR: boolean
   ) {
     this.effect = new ReactiveEffect(getter, () => {
-      if (!this._dirty) {
-        this._dirty = true
-        triggerRefValue(this)
+      if (!this._scheduled) {
+        this._scheduled = true
+        triggerRefValue(this, DirtyLevels.ComputedValueMaybeDirty)
       }
     })
     this.effect.computed = this
@@ -55,17 +56,32 @@ export class ComputedRefImpl<T> {
   get value() {
     // the computed ref may get wrapped by other proxies e.g. readonly() #3376
     const self = toRaw(this)
-    trackRefValue(self)
-    if (self._dirty || !self._cacheable) {
-      self._dirty = false
-      self._value = self.effect.run()!
+    trackRefValue(self, self)
+    if (!self._cacheable || self.effect.dirty) {
+      const newValue = self.effect.run()!
+      const changed = hasChanged(self._value, newValue)
+      self._value = newValue
+      if (changed) {
+        triggerRefValue(self, DirtyLevels.ComputedValueDirty)
+      }
+      self._scheduled = false
     }
     return self._value
   }
 
   set value(newValue: T) {
     this._setter(newValue)
   }
+
+  // #region polyfill _dirty to backward compatibility for Vue <= 3.3
+  get _dirty() {
+    return this.effect.dirty
+  }
+
+  set _dirty(v) {
+    this.effect.dirty = v
+  }
+  // #endregion
 }
 
 /**

packages/reactivity/src/deferredComputed.ts

@@ -38,33 +38,38 @@ class DeferredComputedRefImpl<T> {
     let compareTarget: any
     let hasCompareTarget = false
     let scheduled = false
-    this.effect = new ReactiveEffect(getter, (computedTrigger?: boolean) => {
-      if (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.effect.active && this._get() !== valueToCompare) {
-              triggerRefValue(this)
+    this.effect = new ReactiveEffect(
+      getter,
+      (onScheduled, computedTrigger?: boolean) => {
+        if (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.effect.active && 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 instanceof DeferredComputedRefImpl) {
+              e.scheduler(onScheduled, true /* computedTrigger */)
             }
-            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 instanceof DeferredComputedRefImpl) {
-            e.scheduler!(true /* computedTrigger */)
           }
         }
+        this._dirty = true
       }
-      this._dirty = true
-    })
+    )
     this.effect.computed = this as any
   }
 

packages/reactivity/src/dep.ts

@@ -1,3 +1,4 @@
+import type { ComputedRefImpl } from './computed'
 import { ReactiveEffect, trackOpBit } from './effect'
 
 export type Dep = Set<ReactiveEffect> & TrackedMarkers
@@ -16,12 +17,17 @@ type TrackedMarkers = {
    * newTracked
    */
   n: number
+  computed?: ComputedRefImpl<any>
 }
 
-export const createDep = (effects?: ReactiveEffect[]): Dep => {
+export const createDep = (
+  effects?: ReactiveEffect[],
+  computed?: ComputedRefImpl<any>
+): Dep => {
   const dep = new Set<ReactiveEffect>(effects) as Dep
   dep.w = 0
   dep.n = 0
+  dep.computed = computed
   return dep
 }