From b08e8761367211046140f152dc90c5165caed114 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 26 Apr 2019 17:32:02 +0200
Subject: [PATCH] [FIX] component: call mounted hooks in subcomponent properly

order was not correct in some cases.  Also, this commit is a pretty
significant optimization.
---
 src/component.ts        | 52 ++++++++++++-----------------------------
 tests/component.test.ts | 48 +++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 37 deletions(-)

diff --git a/src/component.ts b/src/component.ts
index 6a3cad6d..8e6000ff 100644
--- a/src/component.ts
+++ b/src/component.ts
@@ -191,24 +191,6 @@ export class Component<
   // Public
   //--------------------------------------------------------------------------
 
-  /**
-   * Attach a child widget to a given html element
-   *
-   * This is most of the time not necessary, since widgets should primarily be
-   * created/managed with the t-widget directive in a qweb template.  However,
-   * for the cases where we need more control, this method will do what is
-   * necessary to make sure all the proper hooks are called (for example,
-   * mounted/willUnmount)
-   *
-   * Note that this method makes a few assumptions:
-   * - the child widget is indeed a child of the current widget
-   * - the target is inside the dom of the current widget (typically a ref)
-   */
-  attachChild(child: Component<T, any, any>, target: HTMLElement) {
-    target.appendChild(child.el!);
-    child.__mount();
-  }
-
   async mount(target: HTMLElement): Promise<void> {
     const vnode = await this._prepare();
     if (this.__owl__.isDestroyed) {
@@ -271,14 +253,14 @@ export class Component<
   destroy() {
     if (!this.__owl__.isDestroyed) {
       const el = this.el;
-      this._destroy();
+      this._destroy(this.__owl__.parent);
       if (el) {
         el.remove();
       }
     }
   }
 
-  _destroy() {
+  _destroy(parent) {
     const isMounted = this.__owl__.isMounted;
     if (isMounted) {
       this.willUnmount();
@@ -286,11 +268,11 @@ export class Component<
     }
     const children = Object.values(this.__owl__.children);
     for (let child of children) {
-      child._destroy();
+      child._destroy(this);
     }
-    if (this.__owl__.parent) {
+    if (parent) {
       let id = this.__owl__.id;
-      delete this.__owl__.parent.__owl__.children[id];
+      delete parent.__owl__.children[id];
       this.__owl__.parent = null;
     }
     this.clear();
@@ -408,24 +390,20 @@ export class Component<
    */
   _mount(vnode: VNode, elm: HTMLElement): VNode {
     this.__owl__.vnode = patch(elm, vnode);
-    this.__mount();
+    if (
+      this.__owl__.parent &&
+      this.__owl__.parent.__owl__.isMounted &&
+      !this.__owl__.isMounted
+    ) {
+      this._callMounted();
+    }
     return this.__owl__.vnode;
   }
 
   __mount() {
-    if (this.__owl__.isMounted) {
-      return;
-    }
-    this._observeState();
-    if (this.__owl__.parent) {
-      if (this.__owl__.parent!.__owl__.isMounted) {
-        this.__owl__.isMounted = true;
-        this.mounted();
-        const children = this.__owl__.children;
-        for (let id in children) {
-          children[id].__mount();
-        }
-      }
+    if (!this.__owl__.isMounted) {
+      this.__owl__.isMounted = true;
+      this.mounted();
     }
   }
 
diff --git a/tests/component.test.ts b/tests/component.test.ts
index 55e268af..6b8692dd 100644
--- a/tests/component.test.ts
+++ b/tests/component.test.ts
@@ -216,6 +216,54 @@ describe("lifecycle hooks", () => {
     expect(steps).toEqual(["child:mounted", "parent:mounted"]);
   });
 
+  test("mounted hook is called on subsubwidgets, in proper order", async () => {
+    const steps: any[] = [];
+
+    class ParentWidget extends Widget {
+      inlineTemplate = `<div><t t-if="state.flag"><t t-widget="child"/></t></div>`;
+      widgets = { child: ChildWidget };
+      state = { flag: false };
+      mounted() {
+        steps.push("parent:mounted");
+      }
+      willUnmount() {
+        steps.push("parent:willUnmount");
+      }
+    }
+    class ChildWidget extends Widget {
+      inlineTemplate = `<div><t t-widget="childchild"/></div>`;
+      widgets = { childchild: ChildChildWidget };
+      mounted() {
+        steps.push("child:mounted");
+      }
+      willUnmount() {
+        steps.push("child:willUnmount");
+      }
+    }
+    class ChildChildWidget extends Widget {
+      mounted() {
+        steps.push("childchild:mounted");
+      }
+      willUnmount() {
+        steps.push("childchild:willUnmount");
+      }
+    }
+    const widget = new ParentWidget(env);
+    await widget.mount(fixture);
+    expect(steps).toEqual(["parent:mounted"]);
+    widget.state.flag = true;
+    await nextTick();
+    widget.destroy();
+    expect(steps).toEqual([
+      "parent:mounted",
+      "childchild:mounted",
+      "child:mounted",
+      "parent:willUnmount",
+      "child:willUnmount",
+      "childchild:willUnmount"
+    ]);
+  });
+
   test("willStart, mounted on subwidget rendered after main is mounted in some other position", async () => {
     const steps: string[] = [];