From c2b274a887f61deb7e0185d1bef3b77d31e991cc Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Thu, 18 Jan 2024 10:46:57 +0800 Subject: [PATCH] fix(reactivity): re-fix #10114 (#10123) --- .../reactivity/__tests__/computed.spec.ts | 63 +++++++++++++++++-- packages/reactivity/src/computed.ts | 6 +- packages/reactivity/src/effect.ts | 17 ++--- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index d0c4dc499da..9de6c4cd9ed 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -2,6 +2,7 @@ import { h, nextTick, nodeOps, render, serializeInner } from '@vue/runtime-test' import { type DebuggerEvent, ITERATE_KEY, + ReactiveEffect, TrackOpTypes, TriggerOpTypes, type WritableComputedRef, @@ -454,14 +455,10 @@ describe('reactivity/computed', () => { expect(fnSpy).toBeCalledTimes(2) }) - it('...', () => { - const fnSpy = vi.fn() + it('should chained recurse effects clear dirty after trigger', () => { const v = ref(1) const c1 = computed(() => v.value) - const c2 = computed(() => { - fnSpy() - return c1.value - }) + const c2 = computed(() => c1.value) c1.effect.allowRecurse = true c2.effect.allowRecurse = true @@ -471,6 +468,60 @@ describe('reactivity/computed', () => { expect(c2.effect._dirtyLevel).toBe(DirtyLevels.NotDirty) }) + it('should chained computeds dirtyLevel update with first computed effect', () => { + const v = ref(0) + const c1 = computed(() => { + if (v.value === 0) { + v.value = 1 + } + return v.value + }) + const c2 = computed(() => c1.value) + const c3 = computed(() => c2.value) + + c3.value + + expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + }) + + it('should work when chained(ref+computed)', () => { + const v = ref(0) + const c1 = computed(() => { + if (v.value === 0) { + v.value = 1 + } + return 'foo' + }) + const c2 = computed(() => v.value + c1.value) + expect(c2.value).toBe('0foo') + expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c2.value).toBe('1foo') + }) + + it('should trigger effect even computed already dirty', () => { + const fnSpy = vi.fn() + const v = ref(0) + const c1 = computed(() => { + if (v.value === 0) { + v.value = 1 + } + return 'foo' + }) + const c2 = computed(() => v.value + c1.value) + + effect(() => { + fnSpy() + c2.value + }) + expect(fnSpy).toBeCalledTimes(1) + expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + v.value = 2 + expect(fnSpy).toBeCalledTimes(2) + }) + it('should be not dirty after deps mutate (mutate deps in computed)', async () => { const state = reactive({}) const consumer = computed(() => { diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 9f4d105c0ea..03459c7dfb6 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -1,4 +1,4 @@ -import { type DebuggerOptions, ReactiveEffect } from './effect' +import { type DebuggerOptions, ReactiveEffect, scheduleEffects } from './effect' import { type Ref, trackRefValue, triggerRefValue } from './ref' import { NOOP, hasChanged, isFunction } from '@vue/shared' import { toRaw } from './reactive' @@ -44,6 +44,7 @@ export class ComputedRefImpl { 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 @@ -59,6 +60,9 @@ export class ComputedRefImpl { } } trackRefValue(self) + if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) { + triggerRefValue(self, DirtyLevels.MaybeDirty) + } return self._value } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 6d13f3b9977..a41cd4986f6 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -291,13 +291,9 @@ export function triggerEffects( ) { pauseScheduling() for (const effect of dep.keys()) { - if (dep.get(effect) !== effect._trackId) { - // when recurse effect is running, dep map could have outdated items - continue - } if ( effect._dirtyLevel < dirtyLevel && - !(effect._runnings && !effect.allowRecurse) + dep.get(effect) === effect._trackId ) { const lastDirtyLevel = effect._dirtyLevel effect._dirtyLevel = dirtyLevel @@ -309,14 +305,21 @@ export function triggerEffects( 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) + (!effect._runnings || effect.allowRecurse) && + dep.get(effect) === effect._trackId ) { effect._shouldSchedule = false queueEffectSchedulers.push(effect.scheduler) } } - resetScheduling() }