diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 10c09109fdb..0122f4e4391 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -456,6 +456,78 @@ 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) @@ -482,7 +554,9 @@ describe('reactivity/computed', () => { c3.value - expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c1.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect_Origin, + ) expect(c2.effect._dirtyLevel).toBe( DirtyLevels.MaybeDirty_ComputedSideEffect, ) @@ -502,7 +576,9 @@ describe('reactivity/computed', () => { }) const c2 = computed(() => v.value + c1.value) expect(c2.value).toBe('0foo') - expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c2.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect, + ) expect(c2.value).toBe('1foo') expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned() }) @@ -523,8 +599,12 @@ describe('reactivity/computed', () => { c2.value }) expect(fnSpy).toBeCalledTimes(1) - expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) - expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c1.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect_Origin, + ) + expect(c2.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect, + ) v.value = 2 expect(fnSpy).toBeCalledTimes(2) expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned() @@ -557,7 +637,9 @@ describe('reactivity/computed', () => { expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) c3.value - expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c1.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect_Origin, + ) expect(c2.effect._dirtyLevel).toBe( DirtyLevels.MaybeDirty_ComputedSideEffect, ) @@ -611,11 +693,18 @@ 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(serializeInner(root)).toBe('Hello World World World World') + expect(c.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect_Origin, + ) + expect(serializeInner(root)).toBe('Hello World World World') expect(COMPUTED_SIDE_EFFECT_WARN).toHaveBeenWarned() }) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index da63fe84750..91eac36012f 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -78,7 +78,10 @@ export class ComputedRefImpl { triggerRefValue(self, DirtyLevels.Dirty) } trackRefValue(self) - if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_ComputedSideEffect) { + if ( + self.effect._dirtyLevel >= + DirtyLevels.MaybeDirty_ComputedSideEffect_Origin + ) { 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 index baa75d61644..4898d691703 100644 --- a/packages/reactivity/src/constants.ts +++ b/packages/reactivity/src/constants.ts @@ -23,9 +23,10 @@ export enum ReactiveFlags { } export enum DirtyLevels { - NotDirty = 0, - QueryingDirty = 1, - MaybeDirty_ComputedSideEffect = 2, - MaybeDirty = 3, - Dirty = 4, + NotDirty, + QueryingDirty, + MaybeDirty_ComputedSideEffect_Origin, + MaybeDirty_ComputedSideEffect, + MaybeDirty, + Dirty, } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 1528f4b1d89..727513fc66c 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -76,6 +76,9 @@ export class ReactiveEffect { } 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 @@ -85,6 +88,13 @@ export class ReactiveEffect { 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 @@ -296,6 +306,12 @@ export function triggerEffects( ) { pauseScheduling() for (const effect of dep.keys()) { + if (!dep.computed && effect.computed) { + if (dep.get(effect) === effect._trackId && effect._runnings > 0) { + effect._dirtyLevel = DirtyLevels.MaybeDirty_ComputedSideEffect_Origin + continue + } + } // dep.get(effect) is very expensive, we need to calculate it lazily and reuse the result let tracking: boolean | undefined if ( @@ -303,6 +319,14 @@ export function triggerEffects( (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 (