Skip to content

Commit 33add70

Browse files
committed
fix(watch): fix cleaup callback not running when watcher stops
Resolves: #113
1 parent 3524cf1 commit 33add70

File tree

2 files changed

+50
-7
lines changed

2 files changed

+50
-7
lines changed

src/apis/watch.ts

+18-6
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ function createWatcher(
9191
options: WatcherOption
9292
): () => void {
9393
const flushMode = options.flush;
94-
let cleanup: () => void;
94+
let cleanup: (() => void) | null;
9595
const registerCleanup: CleanupRegistrator = (fn: () => void) => {
9696
cleanup = () => {
9797
try {
@@ -106,7 +106,7 @@ function createWatcher(
106106
if (cb === null) {
107107
const getter = () => (source as SimpleEffect)(registerCleanup);
108108
// cleanup before running getter again
109-
const runBefore = () => {
109+
const runCleanup = () => {
110110
if (cleanup) {
111111
cleanup();
112112
}
@@ -118,7 +118,7 @@ function createWatcher(
118118
deep: options.deep,
119119
// @ts-ignore
120120
sync: true,
121-
before: runBefore,
121+
before: runCleanup,
122122
});
123123
}
124124

@@ -131,7 +131,7 @@ function createWatcher(
131131
immediate: false,
132132
deep: options.deep,
133133
// @ts-ignore
134-
before: runBefore,
134+
before: runCleanup,
135135
});
136136
};
137137

@@ -144,7 +144,11 @@ function createWatcher(
144144

145145
return () => {
146146
hasEnded = true;
147-
stopRef && stopRef();
147+
if (stopRef) {
148+
stopRef();
149+
runCleanup();
150+
cleanup = null;
151+
}
148152
};
149153
}
150154

@@ -187,12 +191,20 @@ function createWatcher(
187191
applyCb(n, o);
188192
};
189193

190-
return vm.$watch(getter, options.lazy ? callback : shiftCallback, {
194+
const stop = vm.$watch(getter, options.lazy ? callback : shiftCallback, {
191195
immediate: !options.lazy,
192196
deep: options.deep,
193197
// @ts-ignore
194198
sync: flushMode === 'sync',
195199
});
200+
201+
return () => {
202+
stop();
203+
if (cleanup) {
204+
cleanup();
205+
cleanup = null;
206+
}
207+
};
196208
}
197209

198210
export function watch<T = any>(

test/apis/watch.spec.js

+32-1
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ describe('api/watch', () => {
513513
return p;
514514
}
515515

516-
it('work with a single getter', done => {
516+
it('work with (single getter)', done => {
517517
const id = ref(1);
518518
const promises = [];
519519
watch(onCleanup => {
@@ -534,6 +534,37 @@ describe('api/watch', () => {
534534
.then(done);
535535
});
536536

537+
it('run cleanup when watch stops (single getter)', done => {
538+
const spy = jest.fn();
539+
const cleanup = jest.fn();
540+
const stop = watch(onCleanup => {
541+
spy();
542+
onCleanup(cleanup);
543+
});
544+
waitForUpdate(() => {
545+
expect(spy).toHaveBeenCalled();
546+
stop();
547+
})
548+
.then(() => {
549+
expect(cleanup).toHaveBeenCalled();
550+
})
551+
.then(done);
552+
});
553+
554+
it('run cleanup when watch stops', () => {
555+
const id = ref(1);
556+
const spy = jest.fn();
557+
const cleanup = jest.fn();
558+
const stop = watch(id, (value, oldValue, onCleanup) => {
559+
spy(value);
560+
onCleanup(cleanup);
561+
});
562+
563+
expect(spy).toHaveBeenCalledWith(1);
564+
stop();
565+
expect(cleanup).toHaveBeenCalled();
566+
});
567+
537568
it('should not collect reactive in onCleanup', done => {
538569
const ref1 = ref(1);
539570
const ref2 = ref(1);

0 commit comments

Comments
 (0)