From bfa76e9de5288a53f851559c2ef4adc8dcac089e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Sat, 26 Jan 2019 08:40:09 +0100
Subject: [PATCH] fix tricky issue with sub widgets modifying el

---
 web/static/src/ts/core/Widget.ts     | 27 +++++++++++++++++++--------
 web/static/src/ts/core/qweb_vdom.ts  |  5 ++++-
 web/static/src/ts/widgets/Discuss.ts | 15 +++++++--------
 web/static/tests/widget.test.ts      | 13 +++++++++++++
 4 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/web/static/src/ts/core/Widget.ts b/web/static/src/ts/core/Widget.ts
index 29462f47..4c90164b 100644
--- a/web/static/src/ts/core/Widget.ts
+++ b/web/static/src/ts/core/Widget.ts
@@ -30,8 +30,11 @@ export class Widget<T extends WEnv> {
   name: string = "widget";
   template: string = "<div></div>";
 
+  get el(): HTMLElement | null {
+    return this._.vnode ? (<any>this)._.vnode.elm : null;
+  }
+
   env: T;
-  el: HTMLElement | null = null;
   state: Object = {};
   refs: { [key: string]: Widget<T> | HTMLElement | undefined } = {}; // either HTMLElement or Widget
 
@@ -110,12 +113,11 @@ export class Widget<T extends WEnv> {
 
   async render(): Promise<VNode> {
     const vnode = await this._render();
-    if (!this.el) {
-      this.el = document.createElement(vnode.sel!);
-    }
-    patch(this._.vnode || this.el, vnode);
-    this._.vnode = vnode;
-    return vnode;
+    this._.vnode = patch(
+      this._.vnode || document.createElement(vnode.sel!),
+      vnode
+    );
+    return this._.vnode;
   }
 
   private async _start(): Promise<void> {
@@ -130,11 +132,20 @@ export class Widget<T extends WEnv> {
     }
     const promises: Promise<void>[] = [];
     let vnode = this.env.qweb.render(this.name, this, { promises });
+
+    // this part is critical for the patching process to be done correctly. The
+    // tricky part is that a child widget can be rerendered on its own, which
+    // will update its own vnode representation without the knowledge of the
+    // parent widget.  With this, we make sure that the parent widget will be
+    // able to patch itself properly after
+    vnode.key = this._.id;
     return Promise.all(promises).then(() => vnode);
   }
 
+  /**
+   * Only called by qweb t-widget directive
+   */
   _mount(vnode: VNode) {
-    this.el = <HTMLElement>vnode.elm;
     this._.vnode = vnode;
     if (this._.parent) {
       if (this._.parent._.isMounted) {
diff --git a/web/static/src/ts/core/qweb_vdom.ts b/web/static/src/ts/core/qweb_vdom.ts
index a3aa3488..b8a439f0 100644
--- a/web/static/src/ts/core/qweb_vdom.ts
+++ b/web/static/src/ts/core/qweb_vdom.ts
@@ -678,6 +678,7 @@ const widgetDirective: Directive = {
     let dummyID = ctx.generateID();
     let defID = ctx.generateID();
     ctx.addLine(`let _${dummyID} = {} // DUMMY`);
+    ctx.addLine(`let _${dummyID}_index = c${ctx.parentNode}.length`);
     ctx.addLine(`c${ctx.parentNode}.push(_${dummyID})`);
     let props = node.getAttribute("t-props");
     let widgetID = ctx.generateID();
@@ -685,7 +686,9 @@ const widgetDirective: Directive = {
       `let _${widgetID} = new context.widgets['${value}'](owner, ${props})`
     );
     ctx.addLine(
-      `let def${defID} = _${widgetID}._start().then(() => _${widgetID}._render()).then(vnode=>{Object.assign(_${dummyID}, vnode);_${dummyID}.key="${dummyID}";_${dummyID}.data.hook = {create(_,vn){_${widgetID}._mount(vn)}}})`
+      `let def${defID} = _${widgetID}._start().then(() => _${widgetID}._render()).then(vnode=>{c${
+        ctx.parentNode
+      }[_${dummyID}_index]=vnode;vnode.data.hook = {create(_,vn){_${widgetID}._mount(vn)}}})`
     );
     ctx.addLine(`extra.promises.push(def${defID})`);
 
diff --git a/web/static/src/ts/widgets/Discuss.ts b/web/static/src/ts/widgets/Discuss.ts
index a729295f..747e748e 100644
--- a/web/static/src/ts/widgets/Discuss.ts
+++ b/web/static/src/ts/widgets/Discuss.ts
@@ -6,12 +6,13 @@ import { Counter } from "./counter";
 const template = `
     <div class="o_discuss">
         <span>DISCUSS!!</span>
-        <button t-on-click="resetCounter">Reset</button>
-        <button t-on-click="resetCounterAsync">Reset in 3s</button>
-        <button t-on-click="toggle">Toggle  Counter</button>
+        <button t-on-click="resetCounter">Reset first counter</button>
+        <button t-on-click="resetCounterAsync">Reset counter 2 in 3s</button>
+        <button t-on-click="toggle">Toggle  Clock/counters</button>
         <input/>
         <t t-if="state.validcounter">
             <t t-widget="Counter" t-ref="counter" t-props="{initialState:4}"/>
+            <t t-widget="Counter" t-ref="counter2" t-props="{initialState:400}"/>
         </t>
         <t t-else="1">
             <t t-widget="Clock"/>
@@ -35,15 +36,13 @@ export class Discuss extends Widget<Env> {
 
   resetCounterAsync(ev: MouseEvent) {
     setTimeout(() => {
-      if (this.refs.counter instanceof Counter) {
-        this.refs.counter.updateState({ counter: 3 });
+      if (this.refs.counter2 instanceof Counter) {
+        this.refs.counter2.updateState({ counter: 300 });
       }
     }, 3000);
   }
 
   toggle() {
-    if (this.refs.counter instanceof Counter) {
-      this.updateState({ validcounter: !this.state.validcounter });
-    }
+    this.updateState({ validcounter: !this.state.validcounter });
   }
 }
diff --git a/web/static/tests/widget.test.ts b/web/static/tests/widget.test.ts
index b27a4c65..74e92376 100644
--- a/web/static/tests/widget.test.ts
+++ b/web/static/tests/widget.test.ts
@@ -296,4 +296,17 @@ describe("composition", () => {
       "<div><div>1<button>Inc</button></div></div>"
     );
   });
+
+  test("parent's elm for a children === children's elm, even after rerender", async () => {
+    const widget = new WidgetA(env);
+    await widget.mount(fixture);
+
+    expect((<any>widget._.vnode!.children![1]).elm).toBe(
+      (<any>widget._.children[0]._.vnode).elm
+    );
+    await widget._.children[0].render();
+    expect((<any>widget._.vnode!.children![1]).elm).toBe(
+      (<any>widget._.children[0]._.vnode).elm
+    );
+  });
 });