From a62da807c300a1e09443689ead7f8c01386bef30 Mon Sep 17 00:00:00 2001 From: daiwei Date: Mon, 13 Feb 2023 17:45:37 +0800 Subject: [PATCH 1/5] chore: rebase --- packages/reactivity/src/effectScope.ts | 8 ++++-- .../runtime-core/__tests__/apiWatch.spec.ts | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts index c644d03fae6..3ded91da878 100644 --- a/packages/reactivity/src/effectScope.ts +++ b/packages/reactivity/src/effectScope.ts @@ -81,8 +81,12 @@ export class EffectScope { stop(fromParent?: boolean) { if (this._active) { let i, l - for (i = 0, l = this.effects.length; i < l; i++) { - this.effects[i].stop() + // #5783 + // effects will be changed when a watcher stoped. + // so we need to copy it for iteration. + const effectsToStop = this.effects.slice() + for (i = 0, l = effectsToStop.length; i < l; i++) { + effectsToStop[i].stop() } for (i = 0, l = this.cleanups.length; i < l; i++) { this.cleanups[i]() diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index 1b9f500da3c..c0dc1f4d300 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -1126,6 +1126,33 @@ describe('api: watch', () => { expect(spy).toHaveBeenCalledTimes(2) }) + test('handle nested watcher stop properly', () => { + let instance: ComponentInternalInstance + const Comp = { + setup() { + instance = getCurrentInstance()! + watch( + () => 1, + (val, oldVal, onCleanup) => { + const stop = watch( + () => 2, + () => {} + ) + onCleanup(stop) + }, + { immediate: true } + ) + return () => '' + } + } + const root = nodeOps.createElement('div') + createApp(Comp).mount(root) + expect(instance!.scope.effects.length).toBe(3) + + instance!.scope.stop() + expect(instance!.scope.effects[0].active).toBe(false) + }) + it('watching sources: ref', async () => { const foo = ref([1]) const spy = vi.fn() From a09e5c1de224d9b8749c78e751ea8df2428221bb Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:55:56 +0000 Subject: [PATCH 2/5] [autofix.ci] apply automated fixes --- packages/runtime-core/__tests__/apiWatch.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index c2c304b534a..2ba5d469cd4 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -1300,14 +1300,14 @@ describe('api: watch', () => { (val, oldVal, onCleanup) => { const stop = watch( () => 2, - () => {} + () => {}, ) onCleanup(stop) }, - { immediate: true } + { immediate: true }, ) return () => '' - } + }, } const root = nodeOps.createElement('div') createApp(Comp).mount(root) From 4a6e49741de17578e76a70aaf5e332f5e8aa2bca Mon Sep 17 00:00:00 2001 From: edison Date: Wed, 31 Jul 2024 15:38:50 +0800 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: skirtle <65301168+skirtles-code@users.noreply.github.com> --- packages/reactivity/src/effectScope.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts index 4d326cf9fba..b07a142eedb 100644 --- a/packages/reactivity/src/effectScope.ts +++ b/packages/reactivity/src/effectScope.ts @@ -82,7 +82,7 @@ export class EffectScope { if (this._active) { let i, l // #5783 - // effects will be changed when a watcher stoped. + // effects will be changed when a watcher stopped. // so we need to copy it for iteration. const effectsToStop = this.effects.slice() for (i = 0, l = effectsToStop.length; i < l; i++) { From 96b931a7a33d18ff31402e2bcde46014c8271dd8 Mon Sep 17 00:00:00 2001 From: daiwei Date: Tue, 13 Aug 2024 15:49:16 +0800 Subject: [PATCH 4/5] test: update test case --- packages/reactivity/src/effectScope.ts | 2 +- .../runtime-core/__tests__/apiWatch.spec.ts | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts index 4edc62bf69d..f7a2dc6f279 100644 --- a/packages/reactivity/src/effectScope.ts +++ b/packages/reactivity/src/effectScope.ts @@ -117,7 +117,7 @@ export class EffectScope { if (this._active) { let i, l // #5783 - // effects will be changed when a watcher stopped. + // effects will be changed when a watch stopped. // so we need to copy it for iteration. const effectsToStop = this.effects.slice() for (i = 0, l = effectsToStop.length; i < l; i++) { diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index f4ef71bc720..a4e51ea9e0f 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -1378,11 +1378,9 @@ describe('api: watch', () => { expect(spy).toHaveBeenCalledTimes(2) }) - test('handle nested watcher stop properly', () => { - let instance: ComponentInternalInstance + test('nested watch stop', async () => { const Comp = { setup() { - instance = getCurrentInstance()! watch( () => 1, (val, oldVal, onCleanup) => { @@ -1394,15 +1392,23 @@ describe('api: watch', () => { }, { immediate: true }, ) - return () => '' + return () => 'comp' + }, + } + const toggle = ref(true) + const App = { + setup() { + return () => (toggle.value ? h(Comp) : h('div', 'foo')) }, } + const root = nodeOps.createElement('div') - createApp(Comp).mount(root) - expect(instance!.scope.effects.length).toBe(3) + createApp(App).mount(root) + expect(serializeInner(root)).toBe('comp') - instance!.scope.stop() - expect(instance!.scope.effects[0].active).toBe(false) + toggle.value = false + await nextTick() + expect(serializeInner(root)).toBe('
foo
') }) it('watching sources: ref', async () => { From 103dd91e321ec10db25ef4c8fc5bfee268ab5ccb Mon Sep 17 00:00:00 2001 From: daiwei Date: Thu, 15 Aug 2024 09:35:32 +0800 Subject: [PATCH 5/5] test: update test case --- .../runtime-core/__tests__/apiWatch.spec.ts | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/packages/runtime-core/__tests__/apiWatch.spec.ts b/packages/runtime-core/__tests__/apiWatch.spec.ts index a4e51ea9e0f..40cb7d1ad59 100644 --- a/packages/runtime-core/__tests__/apiWatch.spec.ts +++ b/packages/runtime-core/__tests__/apiWatch.spec.ts @@ -1378,37 +1378,41 @@ describe('api: watch', () => { expect(spy).toHaveBeenCalledTimes(2) }) - test('nested watch stop', async () => { - const Comp = { - setup() { - watch( - () => 1, - (val, oldVal, onCleanup) => { - const stop = watch( - () => 2, - () => {}, - ) - onCleanup(stop) - }, - { immediate: true }, - ) - return () => 'comp' - }, - } - const toggle = ref(true) - const App = { - setup() { - return () => (toggle.value ? h(Comp) : h('div', 'foo')) - }, - } + test('removing a watcher while stopping its effectScope', async () => { + const count = ref(0) + const scope = effectScope() + let watcherCalls = 0 + let cleanupCalls = 0 - const root = nodeOps.createElement('div') - createApp(App).mount(root) - expect(serializeInner(root)).toBe('comp') + scope.run(() => { + const stop1 = watch(count, () => { + watcherCalls++ + }) + watch(count, (val, old, onCleanup) => { + watcherCalls++ + onCleanup(() => { + cleanupCalls++ + stop1() + }) + }) + watch(count, () => { + watcherCalls++ + }) + }) - toggle.value = false + expect(watcherCalls).toBe(0) + expect(cleanupCalls).toBe(0) + + count.value++ + await nextTick() + expect(watcherCalls).toBe(3) + expect(cleanupCalls).toBe(0) + + scope.stop() + count.value++ await nextTick() - expect(serializeInner(root)).toBe('
foo
') + expect(watcherCalls).toBe(3) + expect(cleanupCalls).toBe(1) }) it('watching sources: ref', async () => {