fix(reactivity): handle MaybeDirty recurse (#10187)

close #10185

Author: johnsoncodehk

packages/reactivity/__tests__/computed.spec.ts

@@ -10,6 +10,7 @@ import {
   isReadonly,
   reactive,
   ref,
+  shallowRef,
   toRaw,
 } from '../src'
 import { DirtyLevels } from '../src/constants'
@@ -521,6 +522,45 @@ describe('reactivity/computed', () => {
     expect(fnSpy).toBeCalledTimes(2)
   })
 
+  // #10185
+  it('should not override queried MaybeDirty result', () => {
+    class Item {
+      v = ref(0)
+    }
+    const v1 = shallowRef()
+    const v2 = ref(false)
+    const c1 = computed(() => {
+      let c = v1.value
+      if (!v1.value) {
+        c = new Item()
+        v1.value = c
+      }
+      return c.v.value
+    })
+    const c2 = computed(() => {
+      if (!v2.value) return 'no'
+      return c1.value ? 'yes' : 'no'
+    })
+    const c3 = computed(() => c2.value)
+
+    c3.value
+    v2.value = true
+    expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
+    expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
+
+    c3.value
+    expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
+    expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
+    expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
+
+    v1.value.v.value = 999
+    expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
+    expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
+    expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
+
+    expect(c3.value).toBe('yes')
+  })
+
   it('should be not dirty after deps mutate (mutate deps in computed)', async () => {
     const state = reactive<any>({})
     const consumer = computed(() => {

packages/reactivity/src/computed.ts

@@ -1,4 +1,4 @@
-import { type DebuggerOptions, ReactiveEffect, scheduleEffects } from './effect'
+import { type DebuggerOptions, ReactiveEffect } from './effect'
 import { type Ref, trackRefValue, triggerRefValue } from './ref'
 import { NOOP, hasChanged, isFunction } from '@vue/shared'
 import { toRaw } from './reactive'
@@ -44,7 +44,6 @@ export class ComputedRefImpl<T> {
     this.effect = new ReactiveEffect(
       () => getter(this._value),
       () => triggerRefValue(this, DirtyLevels.MaybeDirty),
-      () => this.dep && scheduleEffects(this.dep),
     )
     this.effect.computed = this
     this.effect.active = this._cacheable = !isSSR
@@ -54,10 +53,11 @@ export class ComputedRefImpl<T> {
   get value() {
     // the computed ref may get wrapped by other proxies e.g. readonly() #3376
     const self = toRaw(this)
-    if (!self._cacheable || self.effect.dirty) {
-      if (hasChanged(self._value, (self._value = self.effect.run()!))) {
-        triggerRefValue(self, DirtyLevels.Dirty)
-      }
+    if (
+      (!self._cacheable || self.effect.dirty) &&
+      hasChanged(self._value, (self._value = self.effect.run()!))
+    ) {
+      triggerRefValue(self, DirtyLevels.Dirty)
     }
     trackRefValue(self)
     if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) {

packages/reactivity/src/constants.ts

@@ -24,6 +24,7 @@ export enum ReactiveFlags {
 
 export enum DirtyLevels {
   NotDirty = 0,
-  MaybeDirty = 1,
-  Dirty = 2,
+  QueryingDirty = 1,
+  MaybeDirty = 2,
+  Dirty = 3,
 }

packages/reactivity/src/effect.ts

@@ -77,6 +77,7 @@ export class ReactiveEffect<T = any> {
 
   public get dirty() {
     if (this._dirtyLevel === DirtyLevels.MaybeDirty) {
+      this._dirtyLevel = DirtyLevels.QueryingDirty
       pauseTracking()
       for (let i = 0; i < this._depsLength; i++) {
         const dep = this.deps[i]
@@ -87,7 +88,7 @@ export class ReactiveEffect<T = any> {
           }
         }
       }
-      if (this._dirtyLevel < DirtyLevels.Dirty) {
+      if (this._dirtyLevel === DirtyLevels.QueryingDirty) {
         this._dirtyLevel = DirtyLevels.NotDirty
       }
       resetTracking()
@@ -140,7 +141,7 @@ function preCleanupEffect(effect: ReactiveEffect) {
 }
 
 function postCleanupEffect(effect: ReactiveEffect) {
-  if (effect.deps && effect.deps.length > effect._depsLength) {
+  if (effect.deps.length > effect._depsLength) {
     for (let i = effect._depsLength; i < effect.deps.length; i++) {
       cleanupDepEffect(effect.deps[i], effect)
     }
@@ -291,35 +292,30 @@ export function triggerEffects(
 ) {
   pauseScheduling()
   for (const effect of dep.keys()) {
+    // dep.get(effect) is very expensive, we need to calculate it lazily and reuse the result
+    let tracking: boolean | undefined
     if (
       effect._dirtyLevel < dirtyLevel &&
-      dep.get(effect) === effect._trackId
+      (tracking ??= dep.get(effect) === effect._trackId)
     ) {
-      const lastDirtyLevel = effect._dirtyLevel
+      effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
       effect._dirtyLevel = dirtyLevel
-      if (lastDirtyLevel === DirtyLevels.NotDirty) {
-        effect._shouldSchedule = true
-        if (__DEV__) {
-          effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
-        }
-        effect.trigger()
-      }
     }
-  }
-  scheduleEffects(dep)
-  resetScheduling()
-}
-
-export function scheduleEffects(dep: Dep) {
-  for (const effect of dep.keys()) {
     if (
-      effect.scheduler &&
       effect._shouldSchedule &&
-      (!effect._runnings || effect.allowRecurse) &&
-      dep.get(effect) === effect._trackId
+      (tracking ??= dep.get(effect) === effect._trackId)
     ) {
-      effect._shouldSchedule = false
-      queueEffectSchedulers.push(effect.scheduler)
+      if (__DEV__) {
+        effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo))
+      }
+      effect.trigger()
+      if (!effect._runnings || effect.allowRecurse) {
+        effect._shouldSchedule = false
+        if (effect.scheduler) {
+          queueEffectSchedulers.push(effect.scheduler)
+        }
+      }
     }
   }
+  resetScheduling()
 }