From 28ee790b3edbda98d2f0fdc2e23fb0046100017b Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Wed, 20 Nov 2019 11:09:19 +0100
Subject: [PATCH] [FIX] context: always remove subscription when destroyed

Before this rev., components destroyed before being mounted didn't
stop listening to the context changes.

Closes #476
---
 src/context.ts        |  8 +++++---
 tests/context.test.ts | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/context.ts b/src/context.ts
index 1ef049f0..b6c35368 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -2,7 +2,7 @@ import { Component } from "./component/component";
 import { scheduler } from "./component/scheduler";
 import { EventBus } from "./core/event_bus";
 import { Observer } from "./core/observer";
-import { onWillUnmount } from "./hooks";
+
 /**
  * The `Context` object provides a way to share data between an arbitrary number
  * of component. Usually, data is passed from a parent to its children component,
@@ -138,9 +138,11 @@ export function useContextWithCB(ctx: Context, component: Component<any, any>, m
       await method();
     }
   });
-  onWillUnmount(() => {
+  const __destroy = component.__destroy;
+  component.__destroy = (parent) => {
     ctx.off("update", component);
     delete mapping[id];
-  });
+    __destroy.call(component, parent);
+  }
   return ctx.state;
 }
diff --git a/tests/context.test.ts b/tests/context.test.ts
index 64e64487..a688df01 100644
--- a/tests/context.test.ts
+++ b/tests/context.test.ts
@@ -255,6 +255,38 @@ describe("Context", () => {
     expect(testContext.subscriptions.update.length).toBe(0);
   });
 
+  test("destroyed component before being mounted is inactive", async () => {
+    const testContext = new Context({ a: 123 });
+    const def = makeDeferred();
+
+    class Child extends Component<any, any> {
+      static template = xml`<span><t t-esc="contextObj.a"/></span>`;
+      contextObj = useContext(testContext);
+      willStart() {
+        return def;
+      }
+    }
+    class Parent extends Component<any, any> {
+      static template = xml`<div><Child t-if="state.flag"/></div>`;
+      static components = { Child };
+      state = useState({ flag: true });
+    }
+
+    const parent = new Parent();
+    const prom = parent.mount(fixture);
+    await nextTick(); // wait for Child to be instantiated
+    expect(testContext.subscriptions.update.length).toBe(1);
+    parent.state.flag = false;
+    await prom;
+    expect(fixture.innerHTML).toBe("<div></div>");
+    def.resolve(); // must wait for willStart promise to be resolved
+    await nextTick();
+    // kind of whitebox...
+    // we make sure we do not have any pending subscriptions to the 'update'
+    // event
+    expect(testContext.subscriptions.update.length).toBe(0);
+  });
+
   test("concurrent renderings", async () => {
     const testContext = new Context({ x: { n: 1 }, key: "x" });
     const def = makeDeferred();