From 74ce6a671e01942a4f088510c68a10f45277483b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Tue, 9 Apr 2019 15:24:15 +0200
Subject: [PATCH] fix: properly destroy widget in some cases

when widget was (props)updated, then removed from DOM, it was not
destroyed.

closes #19
---
 src/qweb.ts                                |  4 ++-
 tests/__snapshots__/component.test.ts.snap |  6 ++--
 tests/component.test.ts                    | 41 ++++++++++++++++++++++
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/src/qweb.ts b/src/qweb.ts
index 48a6fe30..1732ffae 100644
--- a/src/qweb.ts
+++ b/src/qweb.ts
@@ -1019,7 +1019,9 @@ const widgetDirective: Directive = {
     ctx.addLine(
       `def${defID} = def${defID}.then(()=>{if (w${widgetID}.__widget__.isDestroyed) {return};let vnode;if (!w${widgetID}.__widget__.vnode){vnode=w${widgetID}.__widget__.pvnode} else { vnode=h(w${widgetID}.__widget__.vnode.sel, {key: ${templateID}});vnode.elm=w${widgetID}.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w${widgetID}.el,a.elm);a.elm=w${widgetID}.el;w${widgetID}.__mount();},remove(){w${widgetID}.${
         keepAlive ? "unmount" : "destroy"
-      }()}}}c${ctx.parentNode}[_${dummyID}_index]=vnode;});`
+      }()}, destroy() {w${widgetID}.${keepAlive ? "unmount" : "destroy"}()}}}c${
+        ctx.parentNode
+      }[_${dummyID}_index]=vnode;});`
     );
     ctx.closeIf();
 
diff --git a/tests/__snapshots__/component.test.ts.snap b/tests/__snapshots__/component.test.ts.snap
index ef05566a..ffba6253 100644
--- a/tests/__snapshots__/component.test.ts.snap
+++ b/tests/__snapshots__/component.test.ts.snap
@@ -53,7 +53,7 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = `
         if (isNew7) {
             def6 = def6.then(vnode=>{let pvnode=h(vnode.sel, {key: key8});c1[_5_index]=pvnode;pvnode.data.hook = {insert(vn){let nvn=w7._mount(vnode, vn.elm);pvnode.elm=nvn.elm},remove(){w7.destroy()},destroy(){w7.destroy()}}; w7.__widget__.pvnode = pvnode;});
         } else {
-            def6 = def6.then(()=>{if (w7.__widget__.isDestroyed) {return};let vnode;if (!w7.__widget__.vnode){vnode=w7.__widget__.pvnode} else { vnode=h(w7.__widget__.vnode.sel, {key: key8});vnode.elm=w7.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w7.el,a.elm);a.elm=w7.el;w7.__mount();},remove(){w7.destroy()}}}c1[_5_index]=vnode;});
+            def6 = def6.then(()=>{if (w7.__widget__.isDestroyed) {return};let vnode;if (!w7.__widget__.vnode){vnode=w7.__widget__.pvnode} else { vnode=h(w7.__widget__.vnode.sel, {key: key8});vnode.elm=w7.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w7.el,a.elm);a.elm=w7.el;w7.__mount();},remove(){w7.destroy()}, destroy() {w7.destroy()}}}c1[_5_index]=vnode;});
         }
         extra.promises.push(def6);
     }
@@ -101,7 +101,7 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = `
     if (isNew4) {
         def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: key5});c1[_2_index]=pvnode;pvnode.data.hook = {insert(vn){let nvn=w4._mount(vnode, vn.elm);pvnode.elm=nvn.elm},remove(){w4.destroy()},destroy(){w4.destroy()}}; w4.__widget__.pvnode = pvnode;});
     } else {
-        def3 = def3.then(()=>{if (w4.__widget__.isDestroyed) {return};let vnode;if (!w4.__widget__.vnode){vnode=w4.__widget__.pvnode} else { vnode=h(w4.__widget__.vnode.sel, {key: key5});vnode.elm=w4.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w4.el,a.elm);a.elm=w4.el;w4.__mount();},remove(){w4.destroy()}}}c1[_2_index]=vnode;});
+        def3 = def3.then(()=>{if (w4.__widget__.isDestroyed) {return};let vnode;if (!w4.__widget__.vnode){vnode=w4.__widget__.pvnode} else { vnode=h(w4.__widget__.vnode.sel, {key: key5});vnode.elm=w4.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w4.el,a.elm);a.elm=w4.el;w4.__mount();},remove(){w4.destroy()}, destroy() {w4.destroy()}}}c1[_2_index]=vnode;});
     }
     extra.promises.push(def3);
     return vn1;
@@ -147,7 +147,7 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin
     if (isNew4) {
         def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4});c1[_2_index]=pvnode;pvnode.data.hook = {insert(vn){let nvn=w4._mount(vnode, vn.elm);pvnode.elm=nvn.elm},remove(){w4.destroy()},destroy(){w4.destroy()}}; w4.__widget__.pvnode = pvnode;});
     } else {
-        def3 = def3.then(()=>{if (w4.__widget__.isDestroyed) {return};let vnode;if (!w4.__widget__.vnode){vnode=w4.__widget__.pvnode} else { vnode=h(w4.__widget__.vnode.sel, {key: 4});vnode.elm=w4.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w4.el,a.elm);a.elm=w4.el;w4.__mount();},remove(){w4.destroy()}}}c1[_2_index]=vnode;});
+        def3 = def3.then(()=>{if (w4.__widget__.isDestroyed) {return};let vnode;if (!w4.__widget__.vnode){vnode=w4.__widget__.pvnode} else { vnode=h(w4.__widget__.vnode.sel, {key: 4});vnode.elm=w4.el;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w4.el,a.elm);a.elm=w4.el;w4.__mount();},remove(){w4.destroy()}, destroy() {w4.destroy()}}}c1[_2_index]=vnode;});
     }
     extra.promises.push(def3);
     return vn1;
diff --git a/tests/component.test.ts b/tests/component.test.ts
index 86eca6cf..f0142a1a 100644
--- a/tests/component.test.ts
+++ b/tests/component.test.ts
@@ -329,6 +329,47 @@ describe("lifecycle hooks", () => {
     ]);
   });
 
+  test("widgets are unmounted and destroyed if no longer in DOM, even after updateprops", async () => {
+    let childUnmounted = false;
+    class ChildWidget extends Widget {
+      inlineTemplate = `<span><t t-esc="props.n"/></span>`;
+      willUnmount() {
+        childUnmounted = true;
+      }
+      increment() {
+        this.updateState({ n: this.state.n + 1 });
+      }
+    }
+
+    class ParentWidget extends Widget {
+      widgets = { ChildWidget };
+      inlineTemplate = `
+          <div>
+            <div t-if="state.flag">
+              <t t-widget="ChildWidget" t-props="{n: state.n}"/>
+            </div>
+          </div>`;
+      state = { n: 0, flag: true };
+      increment() {
+        this.updateState({ n: this.state.n + 1 });
+      }
+      toggleSubWidget() {
+        this.updateState({ flag: !this.state.flag });
+      }
+    }
+
+    const widget = new ParentWidget(env);
+    await widget.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><div><span>0</span></div></div>");
+    widget.increment();
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><div><span>1</span></div></div>");
+    widget.toggleSubWidget();
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div></div>");
+    expect(childUnmounted).toBe(true);
+  });
+
   test("hooks are called in proper order in widget creation/destruction", async () => {
     let steps: string[] = [];
     class ParentWidget extends Widget {