From bcdecb7b1a5b6eb55a2b5975f2f73df69c52099a Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 16 Jan 2024 07:25:05 +0800 Subject: [PATCH 1/6] fix(reactivity): re-fix #10114 --- .../reactivity/__tests__/computed.spec.ts | 42 ++++++++++++++++--- packages/reactivity/src/computed.ts | 3 ++ packages/reactivity/src/effect.ts | 5 +-- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index d0c4dc499da..0b383b4d678 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -454,14 +454,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 +467,40 @@ 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) + + v.value = 2 + + expect(c2.value).toBe(2) + }) + + it('should work when chained(ref+computed)', () => { + const value = ref(0) + const consumer = computed(() => { + value.value++ + return 'foo' + }) + const provider = computed(() => value.value + consumer.value) + expect(provider.value).toBe('0foo') + expect(provider.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(provider.value).toBe('1foo') + }) + 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..7c3fb23ff4b 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -59,6 +59,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..e8a3d996752 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -295,10 +295,7 @@ export function triggerEffects( // when recurse effect is running, dep map could have outdated items continue } - if ( - effect._dirtyLevel < dirtyLevel && - !(effect._runnings && !effect.allowRecurse) - ) { + if (effect._dirtyLevel < dirtyLevel) { const lastDirtyLevel = effect._dirtyLevel effect._dirtyLevel = dirtyLevel if (lastDirtyLevel === DirtyLevels.NotDirty) { From 58c8ddfb5c66d93f93b6408b977ca32058c626df Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 16 Jan 2024 08:00:29 +0800 Subject: [PATCH 2/6] Update computed.spec.ts --- packages/reactivity/__tests__/computed.spec.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 0b383b4d678..ef59ad4fc03 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -483,10 +483,6 @@ describe('reactivity/computed', () => { expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) - - v.value = 2 - - expect(c2.value).toBe(2) }) it('should work when chained(ref+computed)', () => { From 33e4a0d5b9ec5006c72f0b287f25fffcaca7fa79 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 16 Jan 2024 11:46:34 +0800 Subject: [PATCH 3/6] fix(reactivity): should trigger effect even computed already dirty --- .../reactivity/__tests__/computed.spec.ts | 37 +++++++++++++++---- packages/reactivity/src/computed.ts | 3 +- packages/reactivity/src/effect.ts | 20 ++++++---- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index ef59ad4fc03..84e2f0fb69c 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -11,6 +11,7 @@ import { reactive, ref, toRaw, + ReactiveEffect, } from '../src' import { DirtyLevels } from '../src/constants' @@ -486,15 +487,37 @@ describe('reactivity/computed', () => { }) it('should work when chained(ref+computed)', () => { - const value = ref(0) - const consumer = computed(() => { - value.value++ + const v = ref(0) + const c1 = computed(() => { + if (v.value === 0) { + v.value = 1 + } return 'foo' }) - const provider = computed(() => value.value + consumer.value) - expect(provider.value).toBe('0foo') - expect(provider.effect._dirtyLevel).toBe(DirtyLevels.Dirty) - expect(provider.value).toBe('1foo') + 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 v2 = computed(() => v.value + c1.value) + + effect(() => { + fnSpy() + v2.value + }) + expect(fnSpy).toBeCalledTimes(1) + v.value = 2 + expect(fnSpy).toBeCalledTimes(2) }) it('should be not dirty after deps mutate (mutate deps in computed)', async () => { diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 7c3fb23ff4b..b32591468a3 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) : void 0), ) this.effect.computed = this this.effect.active = this._cacheable = !isSSR diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index e8a3d996752..a41cd4986f6 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -291,11 +291,10 @@ 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) { + if ( + effect._dirtyLevel < dirtyLevel && + dep.get(effect) === effect._trackId + ) { const lastDirtyLevel = effect._dirtyLevel effect._dirtyLevel = dirtyLevel if (lastDirtyLevel === DirtyLevels.NotDirty) { @@ -306,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() } From 1bdbc3971a9517a6b3f0d2381aa4ea2e28f5be26 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 16 Jan 2024 03:47:26 +0000 Subject: [PATCH 4/6] [autofix.ci] apply automated fixes --- packages/reactivity/__tests__/computed.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 84e2f0fb69c..13e9c52ec26 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, @@ -11,7 +12,6 @@ import { reactive, ref, toRaw, - ReactiveEffect, } from '../src' import { DirtyLevels } from '../src/constants' From 7781131f24cc8f8374667412a780077cee57ff92 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 16 Jan 2024 11:52:17 +0800 Subject: [PATCH 5/6] Update computed.ts --- packages/reactivity/src/computed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index b32591468a3..03459c7dfb6 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -44,7 +44,7 @@ export class ComputedRefImpl { this.effect = new ReactiveEffect( () => getter(this._value), () => triggerRefValue(this, DirtyLevels.MaybeDirty), - () => (this.dep ? scheduleEffects(this.dep) : void 0), + () => this.dep && scheduleEffects(this.dep), ) this.effect.computed = this this.effect.active = this._cacheable = !isSSR From bb7316f46089d9c82cb4eb627afdd639a157f2f6 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 16 Jan 2024 12:22:25 +0800 Subject: [PATCH 6/6] Update computed.spec.ts --- packages/reactivity/__tests__/computed.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 13e9c52ec26..9de6c4cd9ed 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -509,13 +509,15 @@ describe('reactivity/computed', () => { } return 'foo' }) - const v2 = computed(() => v.value + c1.value) + const c2 = computed(() => v.value + c1.value) effect(() => { fnSpy() - v2.value + 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) })