From 8297872e253f38d6aaeae66ce509972c74c77111 Mon Sep 17 00:00:00 2001
From: Thorsten <t.luenborg@googlemail.com>
Date: Mon, 30 Mar 2020 19:37:59 +0200
Subject: [PATCH 1/2] fix(watch): component should runCleaup() on unmount

close #293
---
 src/apis/watch.ts       | 21 +++++++++++++++++++--
 test/apis/watch.spec.js | 24 ++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/apis/watch.ts b/src/apis/watch.ts
index 8fe0c5af..e846e9a2 100644
--- a/src/apis/watch.ts
+++ b/src/apis/watch.ts
@@ -139,6 +139,16 @@ function createVueWatcher(
   return vm._watchers[index];
 }
 
+// We have to monkeypatch the teardown function so Vue will run
+// runCleanup() when it tears down the watcher on unmmount.
+function patchWatcherTeardown(watcher: VueWatcher, runCleanup: () => void) {
+  const _teardown = watcher.teardown;
+  watcher.teardown = function(...args) {
+    _teardown.apply(watcher, args);
+    runCleanup();
+  };
+}
+
 function createWatcher(
   vm: ComponentInstance,
   source: WatcherSource<unknown> | WatcherSource<unknown>[] | SimpleEffect,
@@ -188,8 +198,13 @@ function createWatcher(
       before: runCleanup,
     });
 
+    patchWatcherTeardown(watcher, runCleanup);
+
     // enable the watcher update
     watcher.lazy = false;
+    if (vm !== fallbackVM) {
+      vm._watchers.push(watcher);
+    }
 
     const originGet = watcher.get.bind(watcher);
     if (isSync) {
@@ -201,7 +216,6 @@ function createWatcher(
 
     return () => {
       watcher.teardown();
-      runCleanup();
     };
   }
 
@@ -240,9 +254,12 @@ function createWatcher(
     sync: isSync,
   });
 
+  // Once again, we have to hack the watcher for proper teardown
+  const watcher = vm._watchers[vm._watchers.length - 1];
+  patchWatcherTeardown(watcher, runCleanup);
+
   return () => {
     stop();
-    runCleanup();
   };
 }
 
diff --git a/test/apis/watch.spec.js b/test/apis/watch.spec.js
index cbe3f439..9f74cc5d 100644
--- a/test/apis/watch.spec.js
+++ b/test/apis/watch.spec.js
@@ -13,10 +13,14 @@ describe('api/watch', () => {
   });
 
   it('should work', done => {
+    const onCleanupSpy = jest.fn();
     const vm = new Vue({
       setup() {
         const a = ref(1);
-        watch(a, spy);
+        watch(a, (n, o, _onCleanup) => {
+          spy(n, o, _onCleanup);
+          _onCleanup(onCleanupSpy);
+        });
         return {
           a,
         };
@@ -25,13 +29,21 @@ describe('api/watch', () => {
     }).$mount();
     expect(spy).toBeCalledTimes(1);
     expect(spy).toHaveBeenLastCalledWith(1, undefined, anyFn);
+    expect(onCleanupSpy).toHaveBeenCalledTimes(0);
     vm.a = 2;
     vm.a = 3;
     expect(spy).toBeCalledTimes(1);
     waitForUpdate(() => {
       expect(spy).toBeCalledTimes(2);
       expect(spy).toHaveBeenLastCalledWith(3, 1, anyFn);
-    }).then(done);
+      expect(onCleanupSpy).toHaveBeenCalledTimes(1);
+
+      vm.$destroy();
+    })
+      .then(() => {
+        expect(onCleanupSpy).toHaveBeenCalledTimes(2);
+      })
+      .then(done);
   });
 
   it('basic usage(value wrapper)', done => {
@@ -349,11 +361,13 @@ describe('api/watch', () => {
     let renderedText;
     it('should work', done => {
       let onCleanup;
+      const onCleanupSpy = jest.fn();
       const vm = new Vue({
         setup() {
           const count = ref(0);
           watchEffect(_onCleanup => {
             onCleanup = _onCleanup;
+            _onCleanup(onCleanupSpy);
             spy(count.value);
             renderedText = vm.$el.textContent;
           });
@@ -369,6 +383,7 @@ describe('api/watch', () => {
       expect(spy).not.toHaveBeenCalled();
       waitForUpdate(() => {
         expect(onCleanup).toEqual(anyFn);
+        expect(onCleanupSpy).toHaveBeenCalledTimes(0);
         expect(renderedText).toBe('0');
         expect(spy).toHaveBeenLastCalledWith(0);
         vm.count++;
@@ -376,6 +391,11 @@ describe('api/watch', () => {
         .then(() => {
           expect(renderedText).toBe('1');
           expect(spy).toHaveBeenLastCalledWith(1);
+          expect(onCleanupSpy).toHaveBeenCalledTimes(1);
+          vm.$destroy();
+        })
+        .then(() => {
+          expect(onCleanupSpy).toHaveBeenCalledTimes(2);
         })
         .then(done);
     });

From 505a164a85f8e453326b10e9ae4e9aad039aeae9 Mon Sep 17 00:00:00 2001
From: Thorsten <t.luenborg@googlemail.com>
Date: Fri, 24 Apr 2020 12:41:50 +0200
Subject: [PATCH 2/2] fix: dont push to vm._watchers manually

---
 src/apis/watch.ts | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/apis/watch.ts b/src/apis/watch.ts
index e846e9a2..2e874ba9 100644
--- a/src/apis/watch.ts
+++ b/src/apis/watch.ts
@@ -202,9 +202,6 @@ function createWatcher(
 
     // enable the watcher update
     watcher.lazy = false;
-    if (vm !== fallbackVM) {
-      vm._watchers.push(watcher);
-    }
 
     const originGet = watcher.get.bind(watcher);
     if (isSync) {