From 683993dbc4896c2b425b2aed8684d53168edac8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Mon, 7 Oct 2019 16:01:28 +0200
Subject: [PATCH] [FIX] component/hooks: onWillUnmount was not properly called

---
 src/component/component.ts |  5 ++-
 tests/hooks.test.ts        | 78 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/src/component/component.ts b/src/component/component.ts
index 3e9e1537..817d9d6e 100644
--- a/src/component/component.ts
+++ b/src/component/component.ts
@@ -437,6 +437,9 @@ export class Component<T extends Env, Props extends {}> {
     const __owl__ = this.__owl__;
     const isMounted = __owl__.isMounted;
     if (isMounted) {
+      if (__owl__.willUnmountCB) {
+        __owl__.willUnmountCB();
+      }
       this.willUnmount();
       __owl__.isMounted = false;
     }
@@ -466,7 +469,7 @@ export class Component<T extends Env, Props extends {}> {
     try {
       this.mounted();
       if (__owl__.mountedCB) {
-          __owl__.mountedCB()
+        __owl__.mountedCB();
       }
     } catch (e) {
       errorHandler(e, this);
diff --git a/tests/hooks.test.ts b/tests/hooks.test.ts
index d149a4fc..b165e0d3 100644
--- a/tests/hooks.test.ts
+++ b/tests/hooks.test.ts
@@ -78,6 +78,40 @@ describe("hooks", () => {
     expect(steps).toEqual(["mounted", "willunmount"]);
   });
 
+  test("can use onMounted, onWillUnmount, part 2", async () => {
+    const steps: string[] = [];
+    function useMyHook() {
+      onMounted(() => {
+        steps.push("mounted");
+      });
+      onWillUnmount(() => {
+        steps.push("willunmount");
+      });
+    }
+    class MyComponent extends Component<any, any> {
+      static template = xml`<div>hey</div>`;
+      constructor(env) {
+        super(env);
+        useMyHook();
+      }
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`<div><MyComponent t-if="state.flag"/></div>`;
+      static components = { MyComponent };
+      state = useState({ flag: true });
+    }
+    const parent = new Parent(env);
+    await parent.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><div>hey</div></div>");
+    expect(steps).toEqual(["mounted"]);
+
+    parent.state.flag = false;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div></div>");
+    expect(steps).toEqual(["mounted", "willunmount"]);
+  });
+
   test("mounted, willUnmount, onMounted, onWillUnmount order", async () => {
     const steps: string[] = [];
     function useMyHook() {
@@ -109,6 +143,46 @@ describe("hooks", () => {
     expect(steps).toEqual(["comp:mounted", "hook:mounted", "hook:willunmount", "comp:willunmount"]);
   });
 
+  test("mounted, willUnmount, onMounted, onWillUnmount order, part 2", async () => {
+    const steps: string[] = [];
+    function useMyHook() {
+      onMounted(() => {
+        steps.push("hook:mounted");
+      });
+      onWillUnmount(() => {
+        steps.push("hook:willunmount");
+      });
+    }
+    class MyComponent extends Component<any, any> {
+      static template = xml`<div>hey</div>`;
+      constructor(env) {
+        super(env);
+        useMyHook();
+      }
+      mounted() {
+        steps.push("comp:mounted");
+      }
+      willUnmount() {
+        steps.push("comp:willunmount");
+      }
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`<div><MyComponent t-if="state.flag"/></div>`;
+      static components = { MyComponent };
+      state = useState({ flag: true });
+    }
+
+    const parent = new Parent(env);
+    await parent.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><div>hey</div></div>");
+    parent.state.flag = false;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div></div>");
+
+    expect(steps).toEqual(["comp:mounted", "hook:mounted", "hook:willunmount", "comp:willunmount"]);
+  });
+
   test("two different call to mounted/willunmount should work", async () => {
     const steps: string[] = [];
     function useMyHook(i) {
@@ -346,7 +420,7 @@ describe("hooks", () => {
 
     class Parent extends Component<any, any> {
       static template = xml`<div><t t-esc="env.val"/><Child/></div>`;
-      static components = { Child}
+      static components = { Child };
       constructor(env) {
         super(env);
         useSubEnv({ val: 3 });
@@ -354,6 +428,6 @@ describe("hooks", () => {
     }
     const component = new Parent(env);
     await component.mount(fixture);
-    expect(fixture.innerHTML).toBe( "<div>3<div>5</div></div>");
+    expect(fixture.innerHTML).toBe("<div>3<div>5</div></div>");
   });
 });