From cccb379377ff95e52226921446b3da5da70a25d3 Mon Sep 17 00:00:00 2001
From: "Michael (mcm)" <mcm@odoo.com>
Date: Tue, 18 Jan 2022 14:54:58 +0100
Subject: [PATCH] [FIX] bind lifecycle callbacks to component

Before this commit, some of the callbacks were bound to the component
and some were not.
This commit makes all the callbacks bind to the component.
---
 src/component/lifecycle_hooks.ts              | 20 +++----
 .../__snapshots__/lifecycle.test.ts.snap      | 22 +++++++
 tests/components/lifecycle.test.ts            | 57 +++++++++++++++++++
 3 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/src/component/lifecycle_hooks.ts b/src/component/lifecycle_hooks.ts
index b525f9ee..5516e103 100644
--- a/src/component/lifecycle_hooks.ts
+++ b/src/component/lifecycle_hooks.ts
@@ -7,44 +7,44 @@ import { nodeErrorHandlers } from "./error_handling";
 
 export function onWillStart(fn: () => Promise<void> | void | any) {
   const node = getCurrent()!;
-  node.willStart.push(fn);
+  node.willStart.push(fn.bind(node.component));
 }
 
 export function onWillUpdateProps(fn: (nextProps: any) => Promise<void> | void | any) {
   const node = getCurrent()!;
-  node.willUpdateProps.push(fn);
+  node.willUpdateProps.push(fn.bind(node.component));
 }
 
 export function onMounted(fn: () => void | any) {
   const node = getCurrent()!;
-  node.mounted.push(fn);
+  node.mounted.push(fn.bind(node.component));
 }
 
 export function onWillPatch(fn: () => Promise<void> | any | void) {
   const node = getCurrent()!;
-  node.willPatch.unshift(fn);
+  node.willPatch.unshift(fn.bind(node.component));
 }
 
 export function onPatched(fn: () => void | any) {
   const node = getCurrent()!;
-  node.patched.push(fn);
+  node.patched.push(fn.bind(node.component));
 }
 
 export function onWillUnmount(fn: () => Promise<void> | void | any) {
   const node = getCurrent()!;
-  node.willUnmount.unshift(fn);
+  node.willUnmount.unshift(fn.bind(node.component));
 }
 
 export function onWillDestroy(fn: () => Promise<void> | void | any) {
   const node = getCurrent()!;
-  node.willDestroy.push(fn);
+  node.willDestroy.push(fn.bind(node.component));
 }
 
 export function onWillRender(fn: () => void | any) {
   const node = getCurrent()!;
   const renderFn = node.renderFn;
   node.renderFn = () => {
-    fn();
+    fn.call(node.component);
     return renderFn();
   };
 }
@@ -54,7 +54,7 @@ export function onRendered(fn: () => void | any) {
   const renderFn = node.renderFn;
   node.renderFn = () => {
     const result = renderFn();
-    fn();
+    fn.call(node.component);
     return result;
   };
 }
@@ -67,5 +67,5 @@ export function onError(callback: OnErrorCallback) {
     handlers = [];
     nodeErrorHandlers.set(node, handlers);
   }
-  handlers.push(callback);
+  handlers.push(callback.bind(node.component));
 }
diff --git a/tests/components/__snapshots__/lifecycle.test.ts.snap b/tests/components/__snapshots__/lifecycle.test.ts.snap
index a3b81860..fcd8e9f6 100644
--- a/tests/components/__snapshots__/lifecycle.test.ts.snap
+++ b/tests/components/__snapshots__/lifecycle.test.ts.snap
@@ -187,6 +187,28 @@ exports[`lifecycle hooks hooks are called in proper order in widget creation/des
 }"
 `;
 
+exports[`lifecycle hooks lifecycle callbacks are bound to component 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return component(\`Test\`, {rev: ctx['rev']}, key + \`__1\`, node, ctx);
+  }
+}"
+`;
+
+exports[`lifecycle hooks lifecycle callbacks are bound to component 2`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['props'].rev);
+  }
+}"
+`;
+
 exports[`lifecycle hooks lifecycle semantics 1`] = `
 "function anonymous(bdom, helpers
 ) {
diff --git a/tests/components/lifecycle.test.ts b/tests/components/lifecycle.test.ts
index 9d49aa7e..befbccda 100644
--- a/tests/components/lifecycle.test.ts
+++ b/tests/components/lifecycle.test.ts
@@ -5,10 +5,13 @@ import {
   onPatched,
   onWillUpdateProps,
   onWillRender,
+  onWillDestroy,
+  onRendered,
 } from "../../src/component/lifecycle_hooks";
 import { status } from "../../src/component/status";
 import {
   elem,
+  logStep,
   makeDeferred,
   makeTestFixture,
   nextTick,
@@ -1196,4 +1199,58 @@ describe("lifecycle hooks", () => {
     expect(["Parent:willPatch", "Parent:patched"]).toBeLogged();
     expect(fixture.innerHTML).toBe("<span>Patched</span>");
   });
+
+  test("lifecycle callbacks are bound to component", async () => {
+    expect.assertions(14);
+    let instance: any;
+
+    class Test extends Component {
+      static template = xml`<t t-esc="props.rev" />`;
+      setup() {
+        instance = this;
+        onWillStart(this.logger("onWillStart"));
+        onMounted(this.logger("onMounted"));
+        onWillUpdateProps(this.logger("onWillUpdateProps"));
+        onWillPatch(this.logger("onWillPatch"));
+        onPatched(this.logger("onPatched"));
+        onWillUnmount(this.logger("onWillUnmount"));
+        onWillDestroy(this.logger("onWillDestroy"));
+        onWillRender(this.logger("onWillRender"));
+        onRendered(this.logger("onRendered"));
+      }
+      logger(hookName: string) {
+        return function (this: Test) {
+          logStep(hookName);
+          expect(this === instance).toBe(true);
+        };
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`<Test rev="rev" />`;
+      static components = { Test };
+      rev = 0;
+    }
+
+    const app = new App(Parent);
+    const comp = await app.mount(fixture);
+    comp.rev++;
+    comp.render();
+    await nextTick();
+    app.destroy();
+
+    expect([
+      "onWillStart",
+      "onWillRender",
+      "onRendered",
+      "onMounted",
+      "onWillUpdateProps",
+      "onWillRender",
+      "onRendered",
+      "onWillPatch",
+      "onPatched",
+      "onWillUnmount",
+      "onWillDestroy",
+    ]).toBeLogged();
+  });
 });