From 0bced13ee5c53a02d5f10e5db76fe38b6e131440 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 6 Feb 2024 18:44:09 +0800 Subject: [PATCH] fix(reactivity): avoid infinite recursion from side effects in computed getter (#10232) close #10214 --- .../reactivity/__tests__/computed.spec.ts | 38 +++++++++++++++++-- packages/reactivity/src/computed.ts | 12 ++++-- packages/reactivity/src/constants.ts | 5 ++- packages/reactivity/src/effect.ts | 10 ++++- packages/reactivity/src/ref.ts | 9 ++--- 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 5a0beb973e8..c3d0c7f15ed 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -482,8 +482,12 @@ describe('reactivity/computed', () => { c3.value expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) - expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) - expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + expect(c2.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect, + ) + expect(c3.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect, + ) }) it('should work when chained(ref+computed)', () => { @@ -550,8 +554,12 @@ describe('reactivity/computed', () => { c3.value expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) - expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) - expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + expect(c2.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect, + ) + expect(c3.effect._dirtyLevel).toBe( + DirtyLevels.MaybeDirty_ComputedSideEffect, + ) v1.value.v.value = 999 expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) @@ -581,4 +589,26 @@ describe('reactivity/computed', () => { await nextTick() expect(serializeInner(root)).toBe(`2`) }) + + it('should not trigger effect scheduler by recurse computed effect', async () => { + const v = ref('Hello') + const c = computed(() => { + v.value += ' World' + return v.value + }) + const Comp = { + setup: () => { + return () => c.value + }, + } + const root = nodeOps.createElement('div') + + render(h(Comp), root) + await nextTick() + expect(serializeInner(root)).toBe('Hello World') + + v.value += ' World' + await nextTick() + expect(serializeInner(root)).toBe('Hello World World World World') + }) }) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 9eed5bc8399..259d4e32c8a 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -43,7 +43,13 @@ export class ComputedRefImpl { ) { this.effect = new ReactiveEffect( () => getter(this._value), - () => triggerRefValue(this, DirtyLevels.MaybeDirty), + () => + triggerRefValue( + this, + this.effect._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect + ? DirtyLevels.MaybeDirty_ComputedSideEffect + : DirtyLevels.MaybeDirty, + ), ) this.effect.computed = this this.effect.active = this._cacheable = !isSSR @@ -60,8 +66,8 @@ export class ComputedRefImpl { triggerRefValue(self, DirtyLevels.Dirty) } trackRefValue(self) - if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) { - triggerRefValue(self, DirtyLevels.MaybeDirty) + if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_ComputedSideEffect) { + triggerRefValue(self, DirtyLevels.MaybeDirty_ComputedSideEffect) } return self._value } diff --git a/packages/reactivity/src/constants.ts b/packages/reactivity/src/constants.ts index 5e9716dd88e..baa75d61644 100644 --- a/packages/reactivity/src/constants.ts +++ b/packages/reactivity/src/constants.ts @@ -25,6 +25,7 @@ export enum ReactiveFlags { export enum DirtyLevels { NotDirty = 0, QueryingDirty = 1, - MaybeDirty = 2, - Dirty = 3, + MaybeDirty_ComputedSideEffect = 2, + MaybeDirty = 3, + Dirty = 4, } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 91d9105af20..ca90544c0de 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -76,7 +76,10 @@ export class ReactiveEffect { } public get dirty() { - if (this._dirtyLevel === DirtyLevels.MaybeDirty) { + if ( + this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect || + this._dirtyLevel === DirtyLevels.MaybeDirty + ) { this._dirtyLevel = DirtyLevels.QueryingDirty pauseTracking() for (let i = 0; i < this._depsLength; i++) { @@ -309,7 +312,10 @@ export function triggerEffects( effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo)) } effect.trigger() - if (!effect._runnings || effect.allowRecurse) { + if ( + (!effect._runnings || effect.allowRecurse) && + effect._dirtyLevel !== DirtyLevels.MaybeDirty_ComputedSideEffect + ) { effect._shouldSchedule = false if (effect.scheduler) { queueEffectSchedulers.push(effect.scheduler) diff --git a/packages/reactivity/src/ref.ts b/packages/reactivity/src/ref.ts index a3fdde48366..1b9d60ef06b 100644 --- a/packages/reactivity/src/ref.ts +++ b/packages/reactivity/src/ref.ts @@ -49,11 +49,10 @@ export function trackRefValue(ref: RefBase) { ref = toRaw(ref) trackEffect( activeEffect, - ref.dep || - (ref.dep = createDep( - () => (ref.dep = undefined), - ref instanceof ComputedRefImpl ? ref : undefined, - )), + (ref.dep ??= createDep( + () => (ref.dep = undefined), + ref instanceof ComputedRefImpl ? ref : undefined, + )), __DEV__ ? { target: ref,