From 90f3b4a36dd0bb7e759f9fa1718c8a0bbb01733f Mon Sep 17 00:00:00 2001 From: Alex Liu Date: Tue, 20 Aug 2024 13:52:33 +0800 Subject: [PATCH 1/5] feat(reactivity): accept watch callback return cleanup function --- packages/reactivity/__tests__/watch.spec.ts | 17 +++++++++++++++++ packages/reactivity/src/watch.ts | 7 +++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/reactivity/__tests__/watch.spec.ts b/packages/reactivity/__tests__/watch.spec.ts index 7a4078166b7..460d76e6ef3 100644 --- a/packages/reactivity/__tests__/watch.spec.ts +++ b/packages/reactivity/__tests__/watch.spec.ts @@ -193,4 +193,21 @@ describe('watch', () => { scope.stop() expect(calls).toEqual(['sync 2', 'post 2']) }) + + test('watch callback return cleanup function', async () => { + const fn = vi.fn() + + const scope = new EffectScope() + + scope.run(() => { + const ource = ref(0) + watch(ource, () => fn) + ource.value++ + }) + + scope.stop() + await nextTick() + + expect(fn).toBeCalledTimes(1) + }) }) diff --git a/packages/reactivity/src/watch.ts b/packages/reactivity/src/watch.ts index 2104896b7ae..dc5822e026e 100644 --- a/packages/reactivity/src/watch.ts +++ b/packages/reactivity/src/watch.ts @@ -60,7 +60,7 @@ export interface WatchOptions extends DebuggerOptions { fn: Function | Function[], type: WatchErrorCodes, args?: unknown[], - ) => void + ) => any } export type WatchStopHandle = () => void @@ -257,10 +257,13 @@ export function watch( : oldValue, boundCleanup, ] - call + const cleanup = call ? call(cb!, WatchErrorCodes.WATCH_CALLBACK, args) : // @ts-expect-error cb!(...args) + if (isFunction(cleanup)) { + boundCleanup(cleanup) + } oldValue = newValue } finally { activeWatcher = currentWatcher From 6a2e7677551efe2f76c3b245e20dd635a0add827 Mon Sep 17 00:00:00 2001 From: Alex Liu Date: Tue, 20 Aug 2024 14:31:22 +0800 Subject: [PATCH 2/5] chore: fix typo --- packages/reactivity/__tests__/watch.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/reactivity/__tests__/watch.spec.ts b/packages/reactivity/__tests__/watch.spec.ts index 460d76e6ef3..95f12b19c53 100644 --- a/packages/reactivity/__tests__/watch.spec.ts +++ b/packages/reactivity/__tests__/watch.spec.ts @@ -200,9 +200,9 @@ describe('watch', () => { const scope = new EffectScope() scope.run(() => { - const ource = ref(0) - watch(ource, () => fn) - ource.value++ + const source = ref(0) + watch(source, () => fn) + source.value++ }) scope.stop() From bf92b7aee125e7b8d2298e3ce818ab10bd6a2f86 Mon Sep 17 00:00:00 2001 From: Alex Liu Date: Tue, 20 Aug 2024 21:04:06 +0800 Subject: [PATCH 3/5] feat(watch): support effect watch return cleanup function --- packages/reactivity/__tests__/watch.spec.ts | 22 +++++++++++++++++++++ packages/reactivity/src/watch.ts | 6 +++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/reactivity/__tests__/watch.spec.ts b/packages/reactivity/__tests__/watch.spec.ts index 95f12b19c53..f90e5ebd985 100644 --- a/packages/reactivity/__tests__/watch.spec.ts +++ b/packages/reactivity/__tests__/watch.spec.ts @@ -210,4 +210,26 @@ describe('watch', () => { expect(fn).toBeCalledTimes(1) }) + + test('watch effect return cleanup function', async () => { + const fn = vi.fn() + + const scope = new EffectScope() + + scope.run(() => { + const source = ref(0) + watch(() => { + void source.value + return () => fn() + }) + source.value++ + }) + + await nextTick() + expect(fn).toBeCalledTimes(1) + + scope.stop() + await nextTick() + expect(fn).toBeCalledTimes(2) + }) }) diff --git a/packages/reactivity/src/watch.ts b/packages/reactivity/src/watch.ts index dc5822e026e..85a2a389670 100644 --- a/packages/reactivity/src/watch.ts +++ b/packages/reactivity/src/watch.ts @@ -184,9 +184,13 @@ export function watch( const currentEffect = activeWatcher activeWatcher = effect try { - return call + const cleanup = call ? call(source, WatchErrorCodes.WATCH_CALLBACK, [boundCleanup]) : source(boundCleanup) + + if (isFunction(cleanup)) { + boundCleanup(cleanup) + } } finally { activeWatcher = currentEffect } From 9b33b494e4fe6f544d6f9f8b7eb22b230d983a90 Mon Sep 17 00:00:00 2001 From: Alex Liu Date: Wed, 11 Sep 2024 11:42:44 +0800 Subject: [PATCH 4/5] feat(watch): handle cleanup function of async callback returns --- packages/reactivity/__tests__/watch.spec.ts | 40 +++++++++++++++++++++ packages/reactivity/src/watch.ts | 25 +++++++++---- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/packages/reactivity/__tests__/watch.spec.ts b/packages/reactivity/__tests__/watch.spec.ts index 59e8b19e4a4..71b52727a35 100644 --- a/packages/reactivity/__tests__/watch.spec.ts +++ b/packages/reactivity/__tests__/watch.spec.ts @@ -211,6 +211,25 @@ describe('watch', () => { expect(fn).toBeCalledTimes(1) }) + test('watch async callback return cleanup function', async () => { + const fn = vi.fn() + + const scope = new EffectScope() + + scope.run(() => { + const source = ref(0) + watch(source, async () => fn) + source.value++ + }) + + await nextTick() + + scope.stop() + await nextTick() + + expect(fn).toBeCalledTimes(1) + }) + test('watch effect return cleanup function', async () => { const fn = vi.fn() @@ -232,4 +251,25 @@ describe('watch', () => { await nextTick() expect(fn).toBeCalledTimes(2) }) + + test('watch effect async callback return cleanup function', async () => { + const fn = vi.fn() + + const scope = new EffectScope() + + scope.run(() => { + const source = ref(0) + watch(async () => { + void source.value + return fn + }) + }) + + await nextTick() + + scope.stop() + await nextTick() + + expect(fn).toBeCalledTimes(1) + }) }) diff --git a/packages/reactivity/src/watch.ts b/packages/reactivity/src/watch.ts index e5211605ef6..0ba8b879d03 100644 --- a/packages/reactivity/src/watch.ts +++ b/packages/reactivity/src/watch.ts @@ -7,6 +7,7 @@ import { isMap, isObject, isPlainObject, + isPromise, isSet, remove, } from '@vue/shared' @@ -191,12 +192,18 @@ export function watch( const currentEffect = activeWatcher activeWatcher = effect try { - const cleanup = call + const maybeCleanup = call ? call(source, WatchErrorCodes.WATCH_CALLBACK, [boundCleanup]) : source(boundCleanup) - if (isFunction(cleanup)) { - boundCleanup(cleanup) + if (isFunction(maybeCleanup)) { + boundCleanup(maybeCleanup) + } else if (isPromise(maybeCleanup)) { + maybeCleanup.then(cleanup => { + if (isFunction(cleanup)) { + boundCleanup(cleanup) + } + }) } } finally { activeWatcher = currentEffect @@ -276,12 +283,18 @@ export function watch( : oldValue, boundCleanup, ] - const cleanup = call + const maybeCleanup = call ? call(cb!, WatchErrorCodes.WATCH_CALLBACK, args) : // @ts-expect-error cb!(...args) - if (isFunction(cleanup)) { - boundCleanup(cleanup) + if (isFunction(maybeCleanup)) { + boundCleanup(maybeCleanup) + } else if (isPromise(maybeCleanup)) { + maybeCleanup.then(cleanup => { + if (isFunction(cleanup)) { + boundCleanup(cleanup) + } + }) } oldValue = newValue } finally { From 65e6a10f8ca96713e48c05c5d2976389449fcc79 Mon Sep 17 00:00:00 2001 From: Alex Liu Date: Thu, 19 Sep 2024 18:57:13 +0800 Subject: [PATCH 5/5] fix(watch): catch error from returned promise --- packages/reactivity/src/watch.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/reactivity/src/watch.ts b/packages/reactivity/src/watch.ts index 20d5722b0bc..80eac6cada1 100644 --- a/packages/reactivity/src/watch.ts +++ b/packages/reactivity/src/watch.ts @@ -199,11 +199,13 @@ export function watch( if (isFunction(maybeCleanup)) { boundCleanup(maybeCleanup) } else if (isPromise(maybeCleanup)) { - maybeCleanup.then(cleanup => { - if (isFunction(cleanup)) { - boundCleanup(cleanup) - } - }) + maybeCleanup + .then(cleanup => { + if (isFunction(cleanup)) { + boundCleanup(cleanup) + } + }) + .catch(NOOP) } } finally { activeWatcher = currentEffect @@ -282,11 +284,13 @@ export function watch( if (isFunction(maybeCleanup)) { boundCleanup(maybeCleanup) } else if (isPromise(maybeCleanup)) { - maybeCleanup.then(cleanup => { - if (isFunction(cleanup)) { - boundCleanup(cleanup) - } - }) + maybeCleanup + .then(cleanup => { + if (isFunction(cleanup)) { + boundCleanup(cleanup) + } + }) + .catch(NOOP) } oldValue = newValue } finally {