From eab0caa6cbd3f93f3e6e4f7a19e24dd507b3c4f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Wed, 19 Jan 2022 13:53:07 +0100
Subject: [PATCH] [IMP] portal: ensure that destroy is synchronous

---
 src/portal.ts                                | 15 ++++----
 tests/misc/__snapshots__/portal.test.ts.snap | 30 ++++++++++++++++
 tests/misc/portal.test.ts                    | 36 ++++++++++++++++++++
 3 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/src/portal.ts b/src/portal.ts
index 4b81b067..c9461a85 100644
--- a/src/portal.ts
+++ b/src/portal.ts
@@ -35,9 +35,11 @@ class VPortal extends VText implements Partial<VNode<VPortal>> {
     this.realBDom!.beforeRemove();
   }
   remove() {
-    super.remove();
-    this.realBDom!.remove();
-    this.realBDom = null;
+    if (this.realBDom) {
+      super.remove();
+      this.realBDom!.remove();
+      this.realBDom = null;
+    }
   }
 
   patch(other: VPortal) {
@@ -64,10 +66,9 @@ export class Portal extends Component {
     const node = this.__owl__;
     const renderFn = node.renderFn;
     node.renderFn = () => new VPortal(this.props.target, renderFn());
-    onWillUnmount(async () => {
-      await Promise.resolve();
-      if (node.bdom && (node.bdom as any).realBDom) {
-        (node.bdom as any).realBDom.remove();
+    onWillUnmount(() => {
+      if (node.bdom) {
+        node.bdom.remove();
       }
     });
   }
diff --git a/tests/misc/__snapshots__/portal.test.ts.snap b/tests/misc/__snapshots__/portal.test.ts.snap
index 53932b53..bceeed15 100644
--- a/tests/misc/__snapshots__/portal.test.ts.snap
+++ b/tests/misc/__snapshots__/portal.test.ts.snap
@@ -83,6 +83,36 @@ exports[`Portal Add and remove portals with t-foreach 1`] = `
 }"
 `;
 
+exports[`Portal Add and remove portals with t-foreach and destroy 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { prepareList, Portal, capture, withKey } = helpers;
+  
+  let block2 = createBlock(\`<div><block-text-0/><block-child-0/></div>\`);
+  
+  function slot1(ctx, node, key = \\"\\") {
+    let b4 = text(\` Portal\`);
+    let b5 = text(ctx['portalId']);
+    return multi([b4, b5]);
+  }
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    const [k_block1, v_block1, l_block1, c_block1] = prepareList(ctx['portalIds']);
+    for (let i1 = 0; i1 < l_block1; i1++) {
+      ctx[\`portalId\`] = v_block1[i1];
+      let key1 = ctx['portalId'];
+      let txt1 = ctx['portalId'];
+      const ctx1 = capture(ctx);
+      let b6 = component(Portal, {target: '#outside',slots: {'default': {__render: slot1, __ctx: ctx1}}}, key + \`__1__\${key1}\`, node, ctx);
+      c_block1[i1] = withKey(block2([txt1], [b6]), key1);
+    }
+    return list(c_block1);
+  }
+}"
+`;
+
 exports[`Portal Add and remove portals with t-foreach inside div 1`] = `
 "function anonymous(bdom, helpers
 ) {
diff --git a/tests/misc/portal.test.ts b/tests/misc/portal.test.ts
index 98f3d24e..1f698351 100644
--- a/tests/misc/portal.test.ts
+++ b/tests/misc/portal.test.ts
@@ -1,4 +1,5 @@
 import {
+  App,
   Component,
   mount,
   onError,
@@ -653,6 +654,41 @@ describe("Portal", () => {
     expect(fixture.innerHTML).toBe('<div id="outside"></div>');
   });
 
+  test("Add and remove portals with t-foreach and destroy", async () => {
+    class Parent extends Component {
+      static template = xml`
+          <t t-foreach="portalIds" t-as="portalId" t-key="portalId">
+            <div>
+              <t t-esc="portalId"/>
+              <t t-portal="'#outside'">
+                Portal<t t-esc="portalId"/>
+              </t>
+            </div>
+          </t>`;
+      portalIds = useState([] as any);
+    }
+
+    addOutsideDiv(fixture);
+    const app = new App(Parent);
+    const parent = await app.mount(fixture);
+
+    expect(fixture.innerHTML).toBe('<div id="outside"></div>');
+
+    parent.portalIds.push(1);
+    await nextTick();
+    expect(fixture.innerHTML).toBe('<div id="outside"> Portal1</div><div>1</div>');
+
+    parent.portalIds.push(2);
+    await nextTick();
+    expect(fixture.innerHTML).toBe(
+      '<div id="outside"> Portal1 Portal2</div><div>1</div><div>2</div>'
+    );
+
+    app.destroy();
+    //This will test explicitly that we don't use an await nextTick(); after the destroy.
+    expect(fixture.innerHTML).toBe('<div id="outside"></div>');
+  });
+
   test("conditional use of Portal with div", async () => {
     class Parent extends Component {
       static template = xml`