From 37175df9ad5121521462c2a1bf33380cc79febb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Sun, 27 Jan 2019 13:50:53 +0100
Subject: [PATCH] change children implementation, remove parented link on
 destroy

---
 web/static/src/ts/core/Widget.ts     | 21 ++++++++----
 web/static/tests/core/widget.test.ts | 48 +++++++++++++++++++---------
 2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/web/static/src/ts/core/Widget.ts b/web/static/src/ts/core/Widget.ts
index 7f4d4485..e0bd77ff 100644
--- a/web/static/src/ts/core/Widget.ts
+++ b/web/static/src/ts/core/Widget.ts
@@ -23,7 +23,7 @@ interface Meta<T extends WEnv> {
   isMounted: boolean;
   isDestroyed: boolean;
   parent: Widget<T> | null;
-  children: Widget<T>[];
+  children: { [key: number]: Widget<T> };
 }
 
 export class Widget<T extends WEnv> {
@@ -45,22 +45,25 @@ export class Widget<T extends WEnv> {
 
   constructor(parent: Widget<T> | T, props?: any) {
     wl.push(this);
+    let id: number;
     let p: Widget<T> | null = null;
     if (parent instanceof Widget) {
       p = parent;
-      parent.__widget__.children.push(this);
       this.env = parent.env;
+      id = this.env.getID();
+      parent.__widget__.children[id] = this;
     } else {
       this.env = parent;
+      id = this.env.getID();
     }
     this.__widget__ = {
-      id: this.env.getID(),
+      id: id,
       vnode: null,
       isStarted: false,
       isMounted: false,
       isDestroyed: false,
       parent: p,
-      children: []
+      children: {}
     };
   }
 
@@ -97,6 +100,11 @@ export class Widget<T extends WEnv> {
         this.el.remove();
         delete this.__widget__.vnode;
       }
+      if (this.__widget__.parent) {
+        let id = this.__widget__.id;
+        delete this.__widget__.parent.__widget__.children[id];
+        this.__widget__.parent = null;
+      }
       this.__widget__.isDestroyed = true;
       this.destroyed();
     }
@@ -163,8 +171,9 @@ export class Widget<T extends WEnv> {
 
   private visitSubTree(callback: (w: Widget<T>) => void) {
     callback(this);
-    for (let child of this.__widget__.children) {
-      child.visitSubTree(callback);
+    const children = this.__widget__.children;
+    for (let id in children) {
+      children[id].visitSubTree(callback);
     }
   }
 }
diff --git a/web/static/tests/core/widget.test.ts b/web/static/tests/core/widget.test.ts
index 89898257..fda21f58 100644
--- a/web/static/tests/core/widget.test.ts
+++ b/web/static/tests/core/widget.test.ts
@@ -31,7 +31,12 @@ function nextTick(): Promise<void> {
   return Promise.resolve();
 }
 
-// Test widget
+function children(w: Widget<WEnv>): Widget<WEnv>[] {
+  const childrenMap = w.__widget__.children;
+  return Object.keys(childrenMap).map(id => childrenMap[id]);
+}
+
+// Test widgets
 class Counter extends Widget<WEnv> {
   name = "counter";
   template = `<div><t t-esc="state.counter"/><button t-on-click="inc">Inc</button></div>`;
@@ -44,6 +49,16 @@ class Counter extends Widget<WEnv> {
   }
 }
 
+class WidgetA extends Widget<WEnv> {
+  name = "a";
+  template = `<div>Hello<t t-widget="b"/></div>`;
+  widgets = { b: WidgetB };
+}
+
+class WidgetB extends Widget<WEnv> {
+  template = `<div>world</div>`;
+}
+
 //------------------------------------------------------------------------------
 // Tests
 //------------------------------------------------------------------------------
@@ -269,23 +284,26 @@ describe("destroy method", () => {
     widget.destroy();
     expect(count).toBe(1);
   });
+
+  test("destroy remove the parent/children link", async () => {
+    const parent = new WidgetA(env);
+    await parent.mount(fixture);
+
+    const child = children(parent)[0];
+    expect(child.__widget__.parent).toBe(parent);
+    expect(children(parent).length).toBe(1);
+    child.destroy();
+    expect(child.__widget__.parent).toBe(null);
+    expect(children(parent).length).toBe(0);
+  });
 });
 
 describe("composition", () => {
-  class WidgetA extends Widget<WEnv> {
-    name = "a";
-    template = `<div>Hello<t t-widget="b"/></div>`;
-    widgets = { b: WidgetB };
-  }
-  class WidgetB extends Widget<WEnv> {
-    template = `<div>world</div>`;
-  }
-
   test("a widget with a sub widget", async () => {
     const widget = new WidgetA(env);
     await widget.mount(fixture);
     expect(fixture.innerHTML).toBe("<div>Hello<div>world</div></div>");
-    expect(widget.__widget__.children[0].__widget__.parent).toBe(widget);
+    expect(children(widget)[0].__widget__.parent).toBe(widget);
   });
 
   test("t-refs on widget are widgets", async () => {
@@ -322,11 +340,11 @@ describe("composition", () => {
     await widget.mount(fixture);
 
     expect((<any>widget.__widget__.vnode!.children![1]).elm).toBe(
-      (<any>widget.__widget__.children[0].__widget__.vnode).elm
+      (<any>children(widget)[0].__widget__.vnode).elm
     );
-    await widget.__widget__.children[0].render();
+    await children(widget)[0].render();
     expect((<any>widget.__widget__.vnode!.children![1]).elm).toBe(
-      (<any>widget.__widget__.children[0].__widget__.vnode).elm
+      (<any>children(widget)[0].__widget__.vnode).elm
     );
   });
 
@@ -334,6 +352,6 @@ describe("composition", () => {
     const widget = new WidgetA(env);
     await widget.mount(fixture);
 
-    expect(widget.__widget__.children[0].env).toBe(env);
+    expect(children(widget)[0].env).toBe(env);
   });
 });