From 82baeefd2cabdec1417b6cdd433846e6d38e0192 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 23 Jan 2024 09:43:24 +0800 Subject: [PATCH 01/15] fix(reactivity): handle `MaybeDirty` recurse --- packages/reactivity/src/computed.ts | 3 +- packages/reactivity/src/effect.ts | 46 +++++++++++------------------ 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 03459c7dfb6..7c3fb23ff4b 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -1,4 +1,4 @@ -import { type DebuggerOptions, ReactiveEffect, scheduleEffects } from './effect' +import { type DebuggerOptions, ReactiveEffect } from './effect' import { type Ref, trackRefValue, triggerRefValue } from './ref' import { NOOP, hasChanged, isFunction } from '@vue/shared' import { toRaw } from './reactive' @@ -44,7 +44,6 @@ 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 diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index a41cd4986f6..4dacbad241c 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -76,7 +76,8 @@ export class ReactiveEffect { } public get dirty() { - if (this._dirtyLevel === DirtyLevels.MaybeDirty) { + while (this._dirtyLevel === DirtyLevels.MaybeDirty) { + this._dirtyLevel = DirtyLevels.NotDirty pauseTracking() for (let i = 0; i < this._depsLength; i++) { const dep = this.deps[i] @@ -87,9 +88,6 @@ export class ReactiveEffect { } } } - if (this._dirtyLevel < DirtyLevels.Dirty) { - this._dirtyLevel = DirtyLevels.NotDirty - } resetTracking() } return this._dirtyLevel >= DirtyLevels.Dirty @@ -291,35 +289,25 @@ export function triggerEffects( ) { pauseScheduling() for (const effect of dep.keys()) { - if ( - effect._dirtyLevel < dirtyLevel && - dep.get(effect) === effect._trackId - ) { - const lastDirtyLevel = effect._dirtyLevel + if (dep.get(effect) !== effect._trackId) { + continue + } + if (effect._dirtyLevel < dirtyLevel) { + effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty effect._dirtyLevel = dirtyLevel - if (lastDirtyLevel === DirtyLevels.NotDirty) { - effect._shouldSchedule = true - if (__DEV__) { - effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo)) - } - effect.trigger() + } + if (effect._shouldSchedule) { + if (__DEV__) { + effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo)) } + 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) && - dep.get(effect) === effect._trackId - ) { + if (effect._shouldSchedule && (!effect._runnings || effect.allowRecurse)) { effect._shouldSchedule = false - queueEffectSchedulers.push(effect.scheduler) + if (effect.scheduler) { + queueEffectSchedulers.push(effect.scheduler) + } } } + resetScheduling() } From 1e94cfb298c8419e734586253b7e55bb822f9d35 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 23 Jan 2024 11:56:02 +0800 Subject: [PATCH 02/15] chore: add test --- .../reactivity/__tests__/computed.spec.ts | 31 +++++++++++++++++++ packages/reactivity/src/constants.ts | 5 +-- packages/reactivity/src/effect.ts | 7 +++-- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 860e4dab154..47ec08ac030 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -11,6 +11,7 @@ import { reactive, ref, toRaw, + shallowRef, } from '../src' import { DirtyLevels } from '../src/constants' @@ -521,6 +522,36 @@ describe('reactivity/computed', () => { expect(fnSpy).toBeCalledTimes(2) }) + // #10185 + it('should not override queried MaybeDirty result', () => { + class Item { + v = ref(0) + } + const v1 = shallowRef() + const v2 = ref(false) + const c1 = computed(() => { + let c = v1.value + if (!v1.value) { + c = new Item() + v1.value = c + } + return c.v.value + }) + const c2 = computed(() => { + if (!v2.value) return 'no' + return c1.value ? 'yes' : 'no' + }) + const c3 = computed(() => c2.value) + + c3.value + v2.value = true + c3.value + v1.value.v.value = 999 + + expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + expect(c3.value).toBe('yes') + }) + 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/constants.ts b/packages/reactivity/src/constants.ts index 1e3483eb30d..5e9716dd88e 100644 --- a/packages/reactivity/src/constants.ts +++ b/packages/reactivity/src/constants.ts @@ -24,6 +24,7 @@ export enum ReactiveFlags { export enum DirtyLevels { NotDirty = 0, - MaybeDirty = 1, - Dirty = 2, + QueryingDirty = 1, + MaybeDirty = 2, + Dirty = 3, } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 4dacbad241c..bdcd2d9c96b 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -76,8 +76,8 @@ export class ReactiveEffect { } public get dirty() { - while (this._dirtyLevel === DirtyLevels.MaybeDirty) { - this._dirtyLevel = DirtyLevels.NotDirty + if (this._dirtyLevel === DirtyLevels.MaybeDirty) { + this._dirtyLevel = DirtyLevels.QueryingDirty pauseTracking() for (let i = 0; i < this._depsLength; i++) { const dep = this.deps[i] @@ -88,6 +88,9 @@ export class ReactiveEffect { } } } + if (this._dirtyLevel === DirtyLevels.QueryingDirty) { + this._dirtyLevel = DirtyLevels.NotDirty + } resetTracking() } return this._dirtyLevel >= DirtyLevels.Dirty From 31947664357222003471c0a131f5ec5e10308ace Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 23 Jan 2024 03:57:04 +0000 Subject: [PATCH 03/15] [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 47ec08ac030..21aeb6f984f 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -10,8 +10,8 @@ import { isReadonly, reactive, ref, - toRaw, shallowRef, + toRaw, } from '../src' import { DirtyLevels } from '../src/constants' From b646f38103d7b151630001eeac6bf244a2306cc7 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 23 Jan 2024 12:24:51 +0800 Subject: [PATCH 04/15] perf: delay `dep.get(effect)` calculate --- packages/reactivity/src/effect.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index bdcd2d9c96b..e135fc33f0f 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -292,23 +292,28 @@ export function triggerEffects( ) { pauseScheduling() for (const effect of dep.keys()) { - if (dep.get(effect) !== effect._trackId) { - continue - } - if (effect._dirtyLevel < dirtyLevel) { + // dep.get(effect) is very expensive, we need to calculate it lazily and reuse the result + let tracking: boolean | undefined + if ( + effect._dirtyLevel < dirtyLevel && + (tracking ??= dep.get(effect) === effect._trackId) + ) { effect._shouldSchedule ||= effect._dirtyLevel === DirtyLevels.NotDirty effect._dirtyLevel = dirtyLevel } - if (effect._shouldSchedule) { + if ( + effect._shouldSchedule && + (tracking ??= dep.get(effect) === effect._trackId) + ) { if (__DEV__) { effect.onTrigger?.(extend({ effect }, debuggerEventExtraInfo)) } effect.trigger() - } - if (effect._shouldSchedule && (!effect._runnings || effect.allowRecurse)) { - effect._shouldSchedule = false - if (effect.scheduler) { - queueEffectSchedulers.push(effect.scheduler) + if (!effect._runnings || effect.allowRecurse) { + effect._shouldSchedule = false + if (effect.scheduler) { + queueEffectSchedulers.push(effect.scheduler) + } } } } From 80547d744ebfdb67553b82cd1fb175ad93359efb Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 23 Jan 2024 12:49:44 +0800 Subject: [PATCH 05/15] chore: remove unneeded condition --- packages/reactivity/src/effect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index e135fc33f0f..91d9105af20 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -141,7 +141,7 @@ function preCleanupEffect(effect: ReactiveEffect) { } function postCleanupEffect(effect: ReactiveEffect) { - if (effect.deps && effect.deps.length > effect._depsLength) { + if (effect.deps.length > effect._depsLength) { for (let i = effect._depsLength; i < effect.deps.length; i++) { cleanupDepEffect(effect.deps[i], effect) } From 1c537761c06b8aa3f3b4cfde3e77fa0a1feaf1d9 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Tue, 23 Jan 2024 16:19:31 +0800 Subject: [PATCH 06/15] Update computed.spec.ts --- packages/reactivity/__tests__/computed.spec.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 21aeb6f984f..5a0beb973e8 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -545,10 +545,19 @@ describe('reactivity/computed', () => { c3.value v2.value = true + expect(c2.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + c3.value - v1.value.v.value = 999 + expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + v1.value.v.value = 999 + expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) + expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty) + expect(c3.value).toBe('yes') }) From 3b37ba12a2f7cd9641cc6be0ac54e850d154fd59 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Mon, 29 Jan 2024 18:52:29 +0800 Subject: [PATCH 07/15] chore: format --- packages/reactivity/src/computed.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 7c3fb23ff4b..9eed5bc8399 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -53,10 +53,11 @@ export class ComputedRefImpl { get value() { // the computed ref may get wrapped by other proxies e.g. readonly() #3376 const self = toRaw(this) - if (!self._cacheable || self.effect.dirty) { - if (hasChanged(self._value, (self._value = self.effect.run()!))) { - triggerRefValue(self, DirtyLevels.Dirty) - } + if ( + (!self._cacheable || self.effect.dirty) && + hasChanged(self._value, (self._value = self.effect.run()!)) + ) { + triggerRefValue(self, DirtyLevels.Dirty) } trackRefValue(self) if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty) { From 8d5bea51ad2fe7869bf8040676fe30ca78aae06f Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Mon, 29 Jan 2024 19:25:11 +0800 Subject: [PATCH 08/15] feat(reactivity): recurse computed effect no longer triggers schedulers --- .../reactivity/__tests__/computed.spec.ts | 30 ++++++++++++++++--- packages/reactivity/src/computed.ts | 15 ++++++++-- packages/reactivity/src/constants.ts | 5 ++-- packages/reactivity/src/effect.ts | 10 +++++-- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 5a0beb973e8..0d3984810db 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -482,8 +482,8 @@ 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_Recurse) + expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_Recurse) }) it('should work when chained(ref+computed)', () => { @@ -550,8 +550,8 @@ 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_Recurse) + expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_Recurse) v1.value.v.value = 999 expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) @@ -581,4 +581,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..4a7b67e18c1 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -43,7 +43,16 @@ export class ComputedRefImpl { ) { this.effect = new ReactiveEffect( () => getter(this._value), - () => triggerRefValue(this, DirtyLevels.MaybeDirty), + () => { + if ( + this.effect._runnings || + this.effect._dirtyLevel === DirtyLevels.MaybeDirty_Recurse + ) { + triggerRefValue(this, DirtyLevels.MaybeDirty_Recurse) + } else { + triggerRefValue(this, DirtyLevels.MaybeDirty) + } + }, ) this.effect.computed = this this.effect.active = this._cacheable = !isSSR @@ -60,8 +69,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_Recurse) { + triggerRefValue(self, DirtyLevels.MaybeDirty_Recurse) } return self._value } diff --git a/packages/reactivity/src/constants.ts b/packages/reactivity/src/constants.ts index 5e9716dd88e..62ffe717f10 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_Recurse = 2, + MaybeDirty = 3, + Dirty = 4, } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 91d9105af20..09ad8d2027f 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_Recurse || + 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_Recurse + ) { effect._shouldSchedule = false if (effect.scheduler) { queueEffectSchedulers.push(effect.scheduler) From 99f044ea9d3c20edebd740450ae1a544c6eb38a8 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Mon, 29 Jan 2024 19:51:09 +0800 Subject: [PATCH 09/15] MaybeDirty_Recurse -> MaybeDirty_ComputedSideEffect --- packages/reactivity/__tests__/computed.spec.ts | 8 ++++---- packages/reactivity/src/computed.ts | 8 ++++---- packages/reactivity/src/constants.ts | 2 +- packages/reactivity/src/effect.ts | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index 0d3984810db..b6c227a7b16 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -482,8 +482,8 @@ describe('reactivity/computed', () => { c3.value expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) - expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_Recurse) - expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_Recurse) + 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 +550,8 @@ describe('reactivity/computed', () => { c3.value expect(c1.effect._dirtyLevel).toBe(DirtyLevels.Dirty) - expect(c2.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_Recurse) - expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_Recurse) + 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) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 4a7b67e18c1..09e98e42b4d 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -46,9 +46,9 @@ export class ComputedRefImpl { () => { if ( this.effect._runnings || - this.effect._dirtyLevel === DirtyLevels.MaybeDirty_Recurse + this.effect._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect ) { - triggerRefValue(this, DirtyLevels.MaybeDirty_Recurse) + triggerRefValue(this, DirtyLevels.MaybeDirty_ComputedSideEffect) } else { triggerRefValue(this, DirtyLevels.MaybeDirty) } @@ -69,8 +69,8 @@ export class ComputedRefImpl { triggerRefValue(self, DirtyLevels.Dirty) } trackRefValue(self) - if (self.effect._dirtyLevel >= DirtyLevels.MaybeDirty_Recurse) { - triggerRefValue(self, DirtyLevels.MaybeDirty_Recurse) + 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 62ffe717f10..baa75d61644 100644 --- a/packages/reactivity/src/constants.ts +++ b/packages/reactivity/src/constants.ts @@ -25,7 +25,7 @@ export enum ReactiveFlags { export enum DirtyLevels { NotDirty = 0, QueryingDirty = 1, - MaybeDirty_Recurse = 2, + MaybeDirty_ComputedSideEffect = 2, MaybeDirty = 3, Dirty = 4, } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index 09ad8d2027f..ca90544c0de 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -77,7 +77,7 @@ export class ReactiveEffect { public get dirty() { if ( - this._dirtyLevel === DirtyLevels.MaybeDirty_Recurse || + this._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect || this._dirtyLevel === DirtyLevels.MaybeDirty ) { this._dirtyLevel = DirtyLevels.QueryingDirty @@ -314,7 +314,7 @@ export function triggerEffects( effect.trigger() if ( (!effect._runnings || effect.allowRecurse) && - effect._dirtyLevel !== DirtyLevels.MaybeDirty_Recurse + effect._dirtyLevel !== DirtyLevels.MaybeDirty_ComputedSideEffect ) { effect._shouldSchedule = false if (effect.scheduler) { From 2f9caa240e056c219451b98dad6870a3b0dcf88f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 29 Jan 2024 11:52:15 +0000 Subject: [PATCH 10/15] [autofix.ci] apply automated fixes --- packages/reactivity/__tests__/computed.spec.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index b6c227a7b16..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_ComputedSideEffect) - expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_ComputedSideEffect) + 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_ComputedSideEffect) - expect(c3.effect._dirtyLevel).toBe(DirtyLevels.MaybeDirty_ComputedSideEffect) + 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) From d84ce6996f5b3dfb27ec41ab6240e55b5b842b04 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Mon, 29 Jan 2024 20:38:55 +0800 Subject: [PATCH 11/15] Update computed.ts --- packages/reactivity/src/computed.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 09e98e42b4d..259d4e32c8a 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -43,16 +43,13 @@ export class ComputedRefImpl { ) { this.effect = new ReactiveEffect( () => getter(this._value), - () => { - if ( - this.effect._runnings || + () => + triggerRefValue( + this, this.effect._dirtyLevel === DirtyLevels.MaybeDirty_ComputedSideEffect - ) { - triggerRefValue(this, DirtyLevels.MaybeDirty_ComputedSideEffect) - } else { - triggerRefValue(this, DirtyLevels.MaybeDirty) - } - }, + ? DirtyLevels.MaybeDirty_ComputedSideEffect + : DirtyLevels.MaybeDirty, + ), ) this.effect.computed = this this.effect.active = this._cacheable = !isSSR From 0728c458ba7a30fdef6797b0906829822fb16e8b Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Sun, 4 Feb 2024 23:55:33 +0800 Subject: [PATCH 12/15] chore: use `??=` --- packages/reactivity/src/ref.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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, From 371063565b1f1c83147816b5c7e2c8f69e855868 Mon Sep 17 00:00:00 2001 From: Johnson Chu Date: Wed, 7 Feb 2024 16:47:19 +0800 Subject: [PATCH 13/15] Try fix #10236 --- .../reactivity/__tests__/computed.spec.ts | 21 ----------- .../__tests__/deferredComputed.spec.ts | 16 --------- .../reactivity/__tests__/effectScope.spec.ts | 2 +- packages/reactivity/src/computed.ts | 4 ++- packages/reactivity/src/effect.ts | 22 ++++++++---- packages/reactivity/src/effectScope.ts | 9 +++-- packages/reactivity/src/index.ts | 1 + .../__tests__/apiSetupHelpers.spec.ts | 35 ------------------- .../runtime-core/__tests__/apiWatch.spec.ts | 6 +++- packages/runtime-core/src/apiWatch.ts | 4 +-- 10 files changed, 33 insertions(+), 87 deletions(-) diff --git a/packages/reactivity/__tests__/computed.spec.ts b/packages/reactivity/__tests__/computed.spec.ts index c3d0c7f15ed..494d63397b7 100644 --- a/packages/reactivity/__tests__/computed.spec.ts +++ b/packages/reactivity/__tests__/computed.spec.ts @@ -122,21 +122,6 @@ describe('reactivity/computed', () => { expect(getter2).toHaveBeenCalledTimes(2) }) - it('should no longer update when stopped', () => { - const value = reactive<{ foo?: number }>({}) - const cValue = computed(() => value.foo) - let dummy - effect(() => { - dummy = cValue.value - }) - expect(dummy).toBe(undefined) - value.foo = 1 - expect(dummy).toBe(1) - cValue.effect.stop() - value.foo = 2 - expect(dummy).toBe(1) - }) - it('should support setter', () => { const n = ref(1) const plusOne = computed({ @@ -218,12 +203,6 @@ describe('reactivity/computed', () => { expect(isReadonly(z.value.a)).toBe(false) }) - it('should expose value when stopped', () => { - const x = computed(() => 1) - x.effect.stop() - expect(x.value).toBe(1) - }) - it('debug: onTrack', () => { let events: DebuggerEvent[] = [] const onTrack = vi.fn((e: DebuggerEvent) => { diff --git a/packages/reactivity/__tests__/deferredComputed.spec.ts b/packages/reactivity/__tests__/deferredComputed.spec.ts index 8e78ba959c3..3df8859a7fc 100644 --- a/packages/reactivity/__tests__/deferredComputed.spec.ts +++ b/packages/reactivity/__tests__/deferredComputed.spec.ts @@ -136,20 +136,4 @@ describe('deferred computed', () => { c2.value expect(effectSpy).toHaveBeenCalledTimes(2) }) - - test('should not compute if deactivated before scheduler is called', () => { - const c1Spy = vi.fn() - const src = ref(0) - const c1 = computed(() => { - c1Spy() - return src.value % 2 - }) - effect(() => c1.value) - expect(c1Spy).toHaveBeenCalledTimes(1) - - c1.effect.stop() - // trigger - src.value++ - expect(c1Spy).toHaveBeenCalledTimes(1) - }) }) diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts index f7e3241ccd6..28a1d4db87d 100644 --- a/packages/reactivity/__tests__/effectScope.spec.ts +++ b/packages/reactivity/__tests__/effectScope.spec.ts @@ -267,8 +267,8 @@ describe('reactivity/effect/scope', () => { r.value++ c!.value await nextTick() + expect(computedSpy).toHaveBeenCalledTimes(3) // should not trigger anymore - expect(computedSpy).toHaveBeenCalledTimes(2) expect(watchSpy).toHaveBeenCalledTimes(1) expect(watchEffectSpy).toHaveBeenCalledTimes(2) }) diff --git a/packages/reactivity/src/computed.ts b/packages/reactivity/src/computed.ts index 259d4e32c8a..c6dd9eda4c5 100644 --- a/packages/reactivity/src/computed.ts +++ b/packages/reactivity/src/computed.ts @@ -52,7 +52,9 @@ export class ComputedRefImpl { ), ) this.effect.computed = this - this.effect.active = this._cacheable = !isSSR + // TODO: How SSR affect computed? + // this.effect.active = this._cacheable = !isSSR + this._cacheable = !isSSR this[ReactiveFlags.IS_READONLY] = isReadonly } diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index ca90544c0de..398e35aba98 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -26,7 +26,6 @@ export type DebuggerEventExtraInfo = { export let activeEffect: ReactiveEffect | undefined export class ReactiveEffect { - active = true deps: Dep[] = [] /** @@ -39,7 +38,6 @@ export class ReactiveEffect { */ allowRecurse?: boolean - onStop?: () => void // dev only onTrack?: (event: DebuggerEvent) => void // dev only @@ -105,9 +103,6 @@ export class ReactiveEffect { run() { this._dirtyLevel = DirtyLevels.NotDirty - if (!this.active) { - return this.fn() - } let lastShouldTrack = shouldTrack let lastEffect = activeEffect try { @@ -123,6 +118,19 @@ export class ReactiveEffect { shouldTrack = lastShouldTrack } } +} + +export class ReactiveSideEffect extends ReactiveEffect { + active = true + + onStop?: () => void + + run() { + if (!this.active) { + return this.fn() + } + return super.run() + } stop() { if (this.active) { @@ -177,7 +185,7 @@ export interface ReactiveEffectOptions extends DebuggerOptions { export interface ReactiveEffectRunner { (): T - effect: ReactiveEffect + effect: ReactiveSideEffect } /** @@ -198,7 +206,7 @@ export function effect( fn = (fn as ReactiveEffectRunner).effect.fn } - const _effect = new ReactiveEffect(fn, NOOP, () => { + const _effect = new ReactiveSideEffect(fn, NOOP, () => { if (_effect.dirty) { _effect.run() } diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts index 6a3eaec9fd5..02601e3036a 100644 --- a/packages/reactivity/src/effectScope.ts +++ b/packages/reactivity/src/effectScope.ts @@ -1,4 +1,4 @@ -import type { ReactiveEffect } from './effect' +import type { ReactiveEffect, ReactiveSideEffect } from './effect' import { warn } from './warning' let activeEffectScope: EffectScope | undefined @@ -11,7 +11,7 @@ export class EffectScope { /** * @internal */ - effects: ReactiveEffect[] = [] + effects: (ReactiveEffect | ReactiveSideEffect)[] = [] /** * @internal */ @@ -82,7 +82,10 @@ export class EffectScope { if (this._active) { let i, l for (i = 0, l = this.effects.length; i < l; i++) { - this.effects[i].stop() + const effect = this.effects[i] + if ('stop' in effect) { + effect.stop() + } } for (i = 0, l = this.cleanups.length; i < l; i++) { this.cleanups[i]() diff --git a/packages/reactivity/src/index.ts b/packages/reactivity/src/index.ts index 1c80fbc752b..43afe5d1792 100644 --- a/packages/reactivity/src/index.ts +++ b/packages/reactivity/src/index.ts @@ -54,6 +54,7 @@ export { pauseScheduling, resetScheduling, ReactiveEffect, + ReactiveSideEffect, type ReactiveEffectRunner, type ReactiveEffectOptions, type EffectScheduler, diff --git a/packages/runtime-core/__tests__/apiSetupHelpers.spec.ts b/packages/runtime-core/__tests__/apiSetupHelpers.spec.ts index 04e9c1c86db..dd6eef5a3f8 100644 --- a/packages/runtime-core/__tests__/apiSetupHelpers.spec.ts +++ b/packages/runtime-core/__tests__/apiSetupHelpers.spec.ts @@ -1,9 +1,7 @@ import { type ComponentInternalInstance, - type ComputedRef, type SetupContext, Suspense, - computed, createApp, defineComponent, getCurrentInstance, @@ -419,38 +417,5 @@ describe('SFC