fix: Revert "fix(reactivity): avoid infinite loop when render access …

…a side effect computed (#11135)" This reverts commit 8296e19.
vuejs · Jun 28, 2024 · e0df985 · e0df985
1 parent 6c303ea
commit e0df985
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 134 deletions.
diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts
@@ -456,78 +456,6 @@ describe('reactivity/computed', () => {
     expect(fnSpy).toBeCalledTimes(2)
   })
 
-  it('should mark dirty as MaybeDirty_ComputedSideEffect_Origin', () => {
-    const v = ref(1)
-    const c = computed(() => {
-      v.value += 1
-      return v.value
-    })
-
-    c.value
-    expect(c.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
-    expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
-  })
-
-  it('should not infinite re-run effect when effect access original side effect computed', async () => {
-    const spy = vi.fn()
-    const v = ref(0)
-    const c = computed(() => {
-      v.value += 1
-      return v.value
-    })
-    const Comp = {
-      setup: () => {
-        return () => {
-          spy()
-          return v.value + c.value
-        }
-      },
-    }
-    const root = nodeOps.createElement('div')
-
-    render(h(Comp), root)
-    expect(spy).toBeCalledTimes(1)
-    await nextTick()
-    expect(c.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
-    expect(serializeInner(root)).toBe('2')
-    expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
-  })
-
-  it('should not infinite re-run effect when effect access chained side effect computed', async () => {
-    const spy = vi.fn()
-    const v = ref(0)
-    const c1 = computed(() => {
-      v.value += 1
-      return v.value
-    })
-    const c2 = computed(() => v.value + c1.value)
-    const Comp = {
-      setup: () => {
-        return () => {
-          spy()
-          return v.value + c1.value + c2.value
-        }
-      },
-    }
-    const root = nodeOps.createElement('div')
-
-    render(h(Comp), root)
-    expect(spy).toBeCalledTimes(1)
-    await nextTick()
-    expect(c1.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
-    expect(c2.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect,
-    )
-    expect(serializeInner(root)).toBe('4')
-    expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
-  })
-
   it('should chained recurse effects clear dirty after trigger', () => {
     const v = ref(1)
     const c1 = computed(() => v.value)
@@ -554,9 +482,7 @@ describe('reactivity/computed', () => {
 
     c3.value
 
-    expect(c1.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
+    expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
     expect(c2.effect._dirtyLevel).toBe(
       DirtyLevels.MaybeDirty_ComputedSideEffect,
     )
@@ -576,9 +502,7 @@ describe('reactivity/computed', () => {
     })
     const c2 = computed(() => v.value + c1.value)
     expect(c2.value).toBe('0foo')
-    expect(c2.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect,
-    )
+    expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
     expect(c2.value).toBe('1foo')
     expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
   })
@@ -599,12 +523,8 @@ describe('reactivity/computed', () => {
       c2.value
     })
     expect(fnSpy).toBeCalledTimes(1)
-    expect(c1.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
-    expect(c2.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect,
-    )
+    expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
+    expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
     v.value = 2
     expect(fnSpy).toBeCalledTimes(2)
     expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
@@ -637,9 +557,7 @@ describe('reactivity/computed', () => {
     expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty)
 
     c3.value
-    expect(c1.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
+    expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
     expect(c2.effect._dirtyLevel).toBe(
       DirtyLevels.MaybeDirty_ComputedSideEffect,
     )
@@ -693,18 +611,11 @@ describe('reactivity/computed', () => {
 
     render(h(Comp), root)
     await nextTick()
-    expect(c.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
     expect(serializeInner(root)).toBe('Hello World')
 
     v.value += ' World'
-    expect(c.effect._dirtyLevel).toBe(DirtyLevels.Dirty)
     await nextTick()
-    expect(c.effect._dirtyLevel).toBe(
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin,
-    )
-    expect(serializeInner(root)).toBe('Hello World World World')
+    expect(serializeInner(root)).toBe('Hello World World World World')
     expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned()
   })
 

diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts
@@ -78,10 +78,7 @@ export class ComputedRefImpl<T> {
       triggerRefValue(self, DirtyLevels.Dirty)
     }
     trackRefValue(self)
-    if (
-      self.effect._dirtyLevel >=
-      DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
-    ) {
+    if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_ComputedSideEffect) {
       if (__DEV__ && (__TEST__ || this._warnRecursive)) {
         warn(COMPUTED_SIDE_EFFECT_WARN, `\n\ngetter: `, this.getter)
       }

diff --git a/packages/reactivity/src/constants.ts b/packages/reactivity/src/constants.ts
@@ -23,10 +23,9 @@ export enum ReactiveFlags {
 }
 
 export enum DirtyLevels {
-  NotDirty,
-  QueryingDirty,
-  MaybeDirty_ComputedSideEffect_Origin,
-  MaybeDirty_ComputedSideEffect,
-  MaybeDirty,
-  Dirty,
+  NotDirty = 0,
+  QueryingDirty = 1,
+  MaybeDirty_ComputedSideEffect = 2,
+  MaybeDirty = 3,
+  Dirty = 4,
 }
diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts
@@ -76,9 +76,6 @@ export class ReactiveEffect<T = any> {
   }
 
   public get dirty() {
-    // treat original side effect computed as not dirty to avoid infinite loop
-    if (this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect_Origin)
-      return false
     if (
       this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect ||
       this._dirtyLevel === DirtyLevels.MaybeDirty
@@ -88,13 +85,6 @@ export class ReactiveEffect<T = any> {
       for (let i = 0; i < this._depsLength; i++) {
         const dep = this.deps[i]
         if (dep.computed) {
-          // treat chained side effect computed as dirty to force it re-run
-          // since we know the original side effect computed is dirty
-          if (
-            dep.computed.effect._dirtyLevel ===
-            DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
-          )
-            return true
           triggerComputed(dep.computed)
           if (this._dirtyLevel >= DirtyLevels.Dirty) {
             break
@@ -308,30 +298,11 @@ export function triggerEffects(
   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 (!dep.computed && effect.computed) {
-      if (
-        effect._runnings > 0 &&
-        (tracking ??= dep.get(effect) === effect._trackId)
-      ) {
-        effect._dirtyLevel = DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
-        continue
-      }
-    }
-
     if (
       effect._dirtyLevel < dirtyLevel &&
       (tracking ??= dep.get(effect) === effect._trackId)
     ) {
       effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty
-      // always schedule if the computed is original side effect
-      // since we know it is actually dirty
-      if (
-        effect.computed &&
-        effect._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect_Origin
-      ) {
-        effect._shouldSchedule = true
-      }
       effect._dirtyLevel = dirtyLevel
     }
     if (