[FIX] portal: correctly mount portal content in target created by mount

Previously, blah blah blah

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@odoo/owl",
-  "version": "2.0.1",
+  "version": "2.0.0",
   "description": "Odoo Web Library (OWL)",
   "main": "dist/owl.cjs.js",
   "module": "dist/owl.es.js",

src/runtime/blockdom/block_compiler.ts

@@ -517,9 +517,8 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
       return this.el!;
     }
 
-    moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    moveBeforeDOMNode(node: Node | null) {
-      this.parentEl = parent;
+      nodeInsertBefore.call(this.parentEl, this.el!, node);
-      nodeInsertBefore.call(parent, this.el!, node);
     }
 
     moveBeforeVNode(other: Block | null, afterNode: Node | null) {

src/runtime/blockdom/event_catcher.ts

@@ -46,7 +46,7 @@ export function createCatcher(eventsSpec: EventsSpec): Catcher {
           const target = ev.target;
           let currentNode: any = self.child.firstNode();
           const afterNode = self.afterNode;
-          while (currentNode && currentNode !== afterNode) {
+          while (currentNode !== afterNode) {
             if (currentNode.contains(target)) {
               return origFn.call(this, ev);
             }
@@ -56,10 +56,9 @@ export function createCatcher(eventsSpec: EventsSpec): Catcher {
       }
     }
 
-    moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    moveBeforeDOMNode(node: Node | null) {
-      this.parentEl = parent;
+      this.child.moveBeforeDOMNode(node);
-      this.child.moveBeforeDOMNode(node, parent);
+      this.parentEl!.insertBefore(this.afterNode!, node);
-      parent!.insertBefore(this.afterNode!, node);
     }
 
     moveBeforeVNode(other: VCatcher | null, afterNode: Node | null) {

src/runtime/blockdom/html.ts

@@ -29,8 +29,8 @@ class VHtml {
     }
   }
 
-  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+  moveBeforeDOMNode(node: Node | null) {
-    this.parentEl = parent;
+    const parent = this.parentEl;
     for (let elem of this.content) {
       nodeInsertBefore.call(parent, elem, node);
     }

src/runtime/blockdom/index.ts

@@ -10,7 +10,7 @@ export { createCatcher } from "./event_catcher";
 
 export interface VNode<T = any> {
   mount(parent: HTMLElement, afterNode: Node | null): void;
-  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement): void;
+  moveBeforeDOMNode(node: Node | null): void;
   moveBeforeVNode(other: T | null, afterNode: Node | null): void;
   patch(other: T, withBeforeRemove: boolean): void;
   beforeRemove(): void;

src/runtime/blockdom/list.ts

@@ -38,13 +38,12 @@ class VList {
     this.parentEl = parent;
   }
 
-  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+  moveBeforeDOMNode(node: Node | null) {
-    this.parentEl = parent;
     const children = this.children;
     for (let i = 0, l = children.length; i < l; i++) {
-      children[i].moveBeforeDOMNode(node, parent);
+      children[i].moveBeforeDOMNode(node);
     }
-    parent!.insertBefore(this.anchor!, node);
+    this.parentEl!.insertBefore(this.anchor!, node);
   }
 
   moveBeforeVNode(other: VList | null, afterNode: Node | null) {

src/runtime/blockdom/multi.ts

@@ -38,14 +38,14 @@ export class VMulti {
     this.parentEl = parent;
   }
 
-  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+  moveBeforeDOMNode(node: Node | null) {
-    this.parentEl = parent;
     const children = this.children;
+    const parent = this.parentEl;
     const anchors = this.anchors;
     for (let i = 0, l = children.length; i < l; i++) {
       let child = children[i];
       if (child) {
-        child.moveBeforeDOMNode(node, parent);
+        child.moveBeforeDOMNode(node);
       } else {
         const anchor = anchors![i];
         nodeInsertBefore.call(parent, anchor, node);

src/runtime/blockdom/text.ts

@@ -23,9 +23,8 @@ abstract class VSimpleNode {
     this.el = node;
   }
 
-  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+  moveBeforeDOMNode(node: Node | null) {
-    this.parentEl = parent;
+    nodeInsertBefore.call(this.parentEl, this.el!, node);
-    nodeInsertBefore.call(parent, this.el!, node);
   }
 
   moveBeforeVNode(other: VText | null, afterNode: Node | null) {

src/runtime/blockdom/toggler.ts

@@ -20,8 +20,8 @@ class VToggler {
     this.child.mount(parent, afterNode);
   }
 
-  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement) {
+  moveBeforeDOMNode(node: Node | null) {
-    this.child.moveBeforeDOMNode(node, parent);
+    this.child.moveBeforeDOMNode(node);
   }
 
   moveBeforeVNode(other: VToggler | null, afterNode: Node | null) {

src/runtime/component_node.ts

@@ -306,8 +306,8 @@ export class ComponentNode<P extends Props = any, E = any> implements VNode<Comp
     this.fiber = null;
   }
 
-  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement): void {
+  moveBeforeDOMNode(node: Node | null): void {
-    this.bdom!.moveBeforeDOMNode(node, parent);
+    this.bdom!.moveBeforeDOMNode(node);
   }
 
   moveBeforeVNode(other: ComponentNode<P, E> | null, afterNode: Node | null) {

src/runtime/portal.ts

@@ -21,20 +21,16 @@ class VPortal extends VText implements Partial<VNode<VPortal>> {
     this.target = document.querySelector(this.selector) as any;
     if (this.target) {
       this.content!.mount(this.target!, null);
-    } else {
-      this.content!.mount(parent, anchor);
     }
   }
 
   beforeRemove() {
+    // this.target not being null means content is mounted
+    if (this.target) {
       this.content!.beforeRemove();
-  }
-  remove() {
-    if (this.content) {
-      super.remove();
       this.content!.remove();
-      this.content = null;
     }
+    this.content = null;
   }
 
   patch(other: VPortal) {
@@ -73,9 +69,9 @@ export class Portal extends Component {
     onMounted(() => {
       const portal: VPortal = node.bdom;
       if (!portal.target) {
-        const target: HTMLElement = document.querySelector(this.props.target);
+        portal.target = document.querySelector(this.props.target);
-        if (target) {
+        if (portal.target) {
-          portal.content!.moveBeforeDOMNode(target.firstChild, target);
+          portal.content!.mount(portal.target, null);
         } else {
           throw new OwlError("invalid portal target");
         }
@@ -84,7 +80,7 @@ export class Portal extends Component {
 
     onWillUnmount(() => {
       const portal: VPortal = node.bdom;
-      portal.remove();
+      portal.beforeRemove();
     });
   }
 }

tests/components/__snapshots__/t_on.test.ts.snap

@@ -456,45 +456,3 @@ exports[`t-on t-on on t-slots 2`] = `
   }
 }"
 `;
-
-exports[`t-on t-on when first component child is an empty component 1`] = `
-"function anonymous(app, bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
-  let { createCatcher } = helpers;
-  const comp1 = app.createComponent(\`Child\`, true, false, false, false);
-  const catcher1 = createCatcher({\\"click\\":0});
-  
-  let block1 = createBlock(\`<div block-handler-0=\\"click\\"><block-child-0/></div>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    let hdlr1 = [ctx['push'], ctx];
-    const hdlr2 = [()=>{}, ctx];
-    const b2 = catcher1(comp1({list: ctx['list']}, key + \`__1\`, node, this, null), [hdlr2]);
-    return block1([hdlr1], [b2]);
-  }
-}"
-`;
-
-exports[`t-on t-on when first component child is an empty component 2`] = `
-"function anonymous(app, bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
-  let { prepareList, withKey } = helpers;
-  
-  let block2 = createBlock(\`<span><block-text-0/></span>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    ctx = Object.create(ctx);
-    const [k_block1, v_block1, l_block1, c_block1] = prepareList(ctx['props'].list);;
-    for (let i1 = 0; i1 < l_block1; i1++) {
-      ctx[\`c\`] = v_block1[i1];
-      ctx[\`c_index\`] = i1;
-      const key1 = ctx['c_index'];
-      let txt1 = ctx['c'];
-      c_block1[i1] = withKey(block2([txt1]), key1);
-    }
-    return list(c_block1);
-  }
-}"
-`;

tests/components/t_on.test.ts

@@ -41,30 +41,6 @@ describe("t-on", () => {
     expect(steps).toEqual(["click"]);
   });
 
-  test("t-on when first component child is an empty component", async () => {
-    class Child extends Component {
-      static template = xml`
-        <span t-foreach="props.list" t-as="c" t-key="c_index" t-esc="c"/>
-      `;
-    }
-    class Parent extends Component {
-      static template = xml`
-        <div t-on-click="push"><Child list="list" t-on-click="() => {}"/></div>
-      `;
-      static components = { Child };
-      list = useState([] as string[]);
-      push() {
-        this.list.push("foo");
-      }
-    }
-    const parent = await mount(Parent, fixture);
-    const el = elem(parent);
-    expect(el.innerHTML).toBe("");
-    el.click();
-    await nextTick();
-    expect(el.innerHTML).toBe("<span>foo</span>");
-  });
-
   test("t-on expression in t-foreach", async () => {
     class Comp extends Component {
       static template = xml`