[IMP] portal: portal as a Directive

Before this commit, portal was a Component, now is a directive.
This commit also clean some unused code, and fix an issue on the clean
optimization when a portal is found in a condition or a loop.

src/app/template_helpers.ts

@@ -2,6 +2,7 @@ import { BDom, multi, text, toggler } from "../blockdom";
 import { validateProps } from "../component/props_validation";
 import { Markup } from "../utils";
 import { html } from "../blockdom/index";
+import { VPortal } from "../portal";
 
 /**
  * This file contains utility functions that will be injected in each template,
@@ -12,6 +13,16 @@ function withDefault(value: any, defaultValue: any): any {
   return value === undefined || value === null || value === false ? defaultValue : value;
 }
 
+function callPortal(
+  ctx: any,
+  parent: any,
+  key: string,
+  target: string,
+  content: (ctx: any, node: any, key: string) => BDom
+): BDom {
+  return new VPortal(target, content(ctx, parent, key)) as any;
+}
+
 function callSlot(
   ctx: any,
   parent: any,
@@ -186,6 +197,7 @@ export const UTILS = {
   zero: Symbol("zero"),
   isBoundary,
   callSlot,
+  callPortal,
   capture,
   withKey,
   prepareList,

src/blockdom/block_compiler.ts

@@ -47,7 +47,7 @@ const cache: { [key: string]: BlockType } = {};
  * @param str
  * @returns a new block type, that can build concrete blocks
  */
-export function createBlock(str: string): BlockType {
+export function createBlock(str: string, deepRemove: boolean = false): BlockType {
   if (str in cache) {
     return cache[str];
   }
@@ -67,7 +67,7 @@ export function createBlock(str: string): BlockType {
 
   // step 3: build the final block class
   const template = tree.el as HTMLElement;
-  const Block = buildBlock(template, context);
+  const Block = buildBlock(template, context, deepRemove);
   cache[str] = Block;
   return Block;
 }
@@ -422,7 +422,7 @@ function updateCtx(ctx: BlockCtx, tree: IntermediateTree) {
 // building the concrete block class
 // -----------------------------------------------------------------------------
 
-function buildBlock(template: HTMLElement, ctx: BlockCtx): BlockType {
+function buildBlock(template: HTMLElement, ctx: BlockCtx, deepRemove: boolean): BlockType {
   let B = createBlockClass(template, ctx);
 
   if (ctx.cbRefs.length) {
@@ -447,6 +447,14 @@ function buildBlock(template: HTMLElement, ctx: BlockCtx): BlockType {
       }
     };
     B.prototype.beforeRemove = VMulti.prototype.beforeRemove;
+    if (deepRemove) {
+      const blockRemove = B.prototype.remove;
+      const vMultiRemove = VMulti.prototype.remove;
+      B.prototype.remove = function () {
+        blockRemove.call(this);
+        vMultiRemove.call(this);
+      };
+    }
     return (data?: any[], children: (VNode | undefined)[] = []) => new B(data, children);
   }
 

src/blockdom/list.ts

@@ -17,9 +17,11 @@ class VList {
   anchor: Node | undefined;
   parentEl?: HTMLElement | undefined;
   isOnlyChild?: boolean | undefined;
+  deepRemove: boolean;
 
-  constructor(children: VNode[]) {
+  constructor(children: VNode[], deepRemove: boolean) {
     this.children = children;
+    this.deepRemove = deepRemove;
   }
 
   mount(parent: HTMLElement, afterNode: Node | null) {
@@ -75,7 +77,7 @@ class VList {
     const parent = this.parentEl!;
 
     // fast path: no new child => only remove
-    if (ch2.length === 0 && isOnlyChild) {
+    if (ch2.length === 0 && isOnlyChild && !this.deepRemove) {
       if (withBeforeRemove) {
         for (let i = 0, l = ch1.length; i < l; i++) {
           beforeRemove.call(ch1[i]);
@@ -98,7 +100,6 @@ class VList {
     let endVn2 = ch2[endIdx2];
 
     let mapping: any = undefined;
-    // let noFullRemove = this.hasNoComponent;
 
     while (startIdx1 <= endIdx1 && startIdx2 <= endIdx2) {
       // -------------------------------------------------------------------
@@ -202,7 +203,7 @@ class VList {
 
   remove() {
     const { parentEl, anchor } = this;
-    if (this.isOnlyChild) {
+    if (this.isOnlyChild && !this.deepRemove) {
       nodeSetTextContent.call(parentEl, "");
     } else {
       const children = this.children;
@@ -227,8 +228,8 @@ class VList {
   }
 }
 
-export function list(children: VNode[]): VNode<VList> {
-  return new VList(children);
+export function list(children: VNode[], deepRemove = false): VNode<VList> {
+  return new VList(children, deepRemove);
 }
 
 function createMapping(ch1: any[], startIdx1: number, endIdx2: number): { [key: string]: any } {

src/blockdom/multi.ts

@@ -15,9 +15,11 @@ export class VMulti {
   anchors?: Node[] | undefined;
   parentEl?: HTMLElement | undefined;
   isOnlyChild?: boolean | undefined;
+  deepRemove: boolean;
 
-  constructor(children: (VNode | undefined)[]) {
+  constructor(children: (VNode | undefined)[], deepRemove: boolean) {
     this.children = children;
+    this.deepRemove = deepRemove;
   }
 
   mount(parent: HTMLElement, afterNode: Node | null) {
@@ -103,7 +105,7 @@ export class VMulti {
 
   remove() {
     const parentEl = this.parentEl;
-    if (this.isOnlyChild) {
+    if (this.isOnlyChild && !this.deepRemove) {
       nodeSetTextContent.call(parentEl, "");
     } else {
       const children = this.children;
@@ -129,6 +131,6 @@ export class VMulti {
   }
 }
 
-export function multi(children: (VNode | undefined)[]): VNode<VMulti> {
-  return new VMulti(children);
+export function multi(children: (VNode | undefined)[], deepRemove = false): VNode<VMulti> {
+  return new VMulti(children, deepRemove);
 }

src/compiler/code_generator.ts

@@ -19,6 +19,7 @@ import {
   ASTTSet,
   ASTTranslation,
   ASTType,
+  ASTTPortal,
 } from "./parser";
 
 type BlockType = "block" | "text" | "multi" | "list" | "html" | "comment";
@@ -64,13 +65,15 @@ class BlockDescription {
   type: BlockType;
   parentVar: string = "";
   id: number;
+  deepRemove: boolean;
 
-  constructor(target: CodeTarget, type: BlockType) {
+  constructor(target: CodeTarget, type: BlockType, deepRemove: boolean = false) {
     this.id = BlockDescription.nextBlockId++;
     this.varName = "b" + this.id;
     this.blockName = "block" + this.id;
     this.target = target;
     this.type = type;
+    this.deepRemove = deepRemove;
   }
 
   insertData(str: string, prefix: string = "d"): number {
@@ -99,7 +102,7 @@ class BlockDescription {
       }
       return `${this.blockName}(${params})`;
     } else if (this.type === "list") {
-      return `list(c_block${this.id})`;
+      return `list(c_block${this.id}${this.deepRemove ? ", true" : ""})`;
     }
     return expr;
   }
@@ -250,6 +253,8 @@ export class CodeGenerator {
       mainCode.push(`const ${id} = getTemplate(${template});`);
     }
 
+    const deepRemove = "deepRemove" in this.ast ? this.ast.deepRemove : false;
+
     // define all blocks
     if (this.blocks.length) {
       mainCode.push(``);
@@ -259,9 +264,15 @@ export class CodeGenerator {
           if (block.dynamicTagName) {
             xmlString = xmlString.replace(/^<\w+/, `<\${tag || '${block.dom.nodeName}'}`);
             xmlString = xmlString.replace(/\w+>$/, `\${tag || '${block.dom.nodeName}'}>`);
-            mainCode.push(`let ${block.blockName} = tag => createBlock(\`${xmlString}\`);`);
+            mainCode.push(
+              `let ${block.blockName} = tag => createBlock(\`${xmlString}\`${
+                deepRemove ? ", true" : ""
+              });`
+            );
           } else {
-            mainCode.push(`let ${block.blockName} = createBlock(\`${xmlString}\`);`);
+            mainCode.push(
+              `let ${block.blockName} = createBlock(\`${xmlString}\`${deepRemove ? ", true" : ""});`
+            );
           }
         }
       }
@@ -317,10 +328,11 @@ export class CodeGenerator {
   createBlock(
     parentBlock: BlockDescription | null,
     type: BlockType,
-    ctx: Context
+    ctx: Context,
+    deepRemove: boolean = false
   ): BlockDescription {
     const hasRoot = this.target.hasRoot;
-    const block = new BlockDescription(this.target, type);
+    const block = new BlockDescription(this.target, type, deepRemove);
     if (!hasRoot && !ctx.preventRoot) {
       this.target.hasRoot = true;
       block.isRoot = true;
@@ -445,6 +457,8 @@ export class CodeGenerator {
       case ASTType.TTranslation:
         this.compileTTranslation(ast, ctx);
         break;
+      case ASTType.TPortal:
+        this.compileTPortal(ast, ctx);
     }
   }
 
@@ -704,7 +718,7 @@ export class CodeGenerator {
     if (ast.body) {
       const nextId = BlockDescription.nextBlockId;
       const subCtx: Context = createContext(ctx);
-      this.compileAST({ type: ASTType.Multi, content: ast.body }, subCtx);
+      this.compileAST({ type: ASTType.Multi, content: ast.body, deepRemove: false }, subCtx);
       this.helpers.add("withDefault");
       expr = `withDefault(${expr}, b${nextId})`;
     }
@@ -765,7 +779,7 @@ export class CodeGenerator {
 
       // note: this part is duplicated from end of compilemulti:
       const args = block!.children.map((c) => c.varName).join(", ");
-      this.insertBlock(`multi([${args}])`, block!, ctx)!;
+      this.insertBlock(`multi([${args}]${ast.deepRemove ? ", true" : ""})`, block!, ctx)!;
     }
   }
 
@@ -774,7 +788,7 @@ export class CodeGenerator {
     if (block) {
       this.insertAnchor(block);
     }
-    block = this.createBlock(block, "list", ctx);
+    block = this.createBlock(block, "list", ctx, ast.deepRemove);
     this.target.loopLevel++;
     const loopVar = `i${this.target.loopLevel}`;
     this.addLine(`ctx = Object.create(ctx);`);
@@ -909,7 +923,7 @@ export class CodeGenerator {
       }
 
       const args = block!.children.map((c) => c.varName).join(", ");
-      this.insertBlock(`multi([${args}])`, block!, ctx)!;
+      this.insertBlock(`multi([${args}]${ast.deepRemove ? ", true" : ""})`, block!, ctx)!;
     }
   }
 
@@ -921,7 +935,7 @@ export class CodeGenerator {
       this.helpers.add("isBoundary");
       const nextId = BlockDescription.nextBlockId;
       const subCtx: Context = createContext(ctx, { preventRoot: true });
-      this.compileAST({ type: ASTType.Multi, content: ast.body }, subCtx);
+      this.compileAST({ type: ASTType.Multi, content: ast.body, deepRemove: false }, subCtx);
       if (nextId !== BlockDescription.nextBlockId) {
         this.helpers.add("zero");
         this.addLine(`ctx[zero] = b${nextId};`);
@@ -976,7 +990,7 @@ export class CodeGenerator {
     const expr = ast.value ? compileExpr(ast.value || "") : "null";
     if (ast.body) {
       this.helpers.add("LazyValue");
-      const bodyAst: AST = { type: ASTType.Multi, content: ast.body };
+      const bodyAst: AST = { type: ASTType.Multi, content: ast.body, deepRemove: false };
       const name = this.compileInNewTarget("value", bodyAst, ctx);
       let value = `new LazyValue(${name}, ctx, node)`;
       value = ast.value ? (value ? `withDefault(${expr}, ${value})` : expr) : value;
@@ -1162,4 +1176,15 @@ export class CodeGenerator {
       this.compileAST(ast.content, Object.assign({}, ctx, { translate: false }));
     }
   }
+  compileTPortal(ast: ASTTPortal, ctx: Context) {
+    this.helpers.add("callPortal");
+    let { block } = ctx;
+    const name = this.compileInNewTarget("portalContent", ast.content, ctx);
+    const blockString = `callPortal(ctx, node, key, ${ast.target}, ${name})`;
+    if (block) {
+      this.insertAnchor(block);
+    }
+    block = this.createBlock(block, "multi", ctx);
+    this.insertBlock(blockString, block, { ...ctx, forceNewBlock: false });
+  }
 }

src/compiler/parser.ts

@@ -20,6 +20,7 @@ export const enum ASTType {
   TSlot,
   TCallBlock,
   TTranslation,
+  TPortal,
 }
 
 export interface ASTText {
@@ -55,6 +56,7 @@ export interface ASTDomNode {
 export interface ASTMulti {
   type: ASTType.Multi;
   content: AST[];
+  deepRemove: boolean;
 }
 
 export interface ASTTEsc {
@@ -75,6 +77,7 @@ export interface ASTTif {
   content: AST;
   tElif: { condition: string; content: AST }[] | null;
   tElse: AST | null;
+  deepRemove: boolean;
 }
 
 export interface ASTTSet {
@@ -92,8 +95,7 @@ export interface ASTTForEach {
   key: string | null;
   body: AST;
   memo: string;
-  isOnlyChild: boolean;
-  hasNoComponent: boolean;
+  deepRemove: boolean;
   hasNoFirst: boolean;
   hasNoLast: boolean;
   hasNoIndex: boolean;
@@ -149,6 +151,12 @@ export interface ASTTranslation {
   content: AST | null;
 }
 
+export interface ASTTPortal {
+  type: ASTType.TPortal;
+  target: string;
+  content: AST;
+}
+
 export type AST =
   | ASTText
   | ASTComment
@@ -166,7 +174,8 @@ export type AST =
   | ASTTCallBlock
   | ASTLog
   | ASTDebug
-  | ASTTranslation;
+  | ASTTranslation
+  | ASTTPortal;
 
 // -----------------------------------------------------------------------------
 // Parser
@@ -195,6 +204,7 @@ function parseNode(node: ChildNode, ctx: ParsingContext): AST | null {
     parseTDebugLog(node, ctx) ||
     parseTForEach(node, ctx) ||
     parseTIf(node, ctx) ||
+    parseTPortal(node, ctx) ||
     parseTCall(node, ctx) ||
     parseTCallBlock(node, ctx) ||
     parseTEscNode(node, ctx) ||
@@ -351,9 +361,6 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
       attrs[attr] = value;
     }
   }
-  if (children.length === 1 && children[0].type === ASTType.TForEach) {
-    children[0].isOnlyChild = true;
-  }
   return {
     type: ASTType.DomNode,
     tag: tagName,
@@ -477,9 +484,8 @@ function parseTForEach(node: Element, ctx: ParsingContext): AST | null {
     elem,
     body,
     memo,
+    deepRemove: needDeepRemove(body),
     key,
-    isOnlyChild: false,
-    hasNoComponent: hasNoComponent(body),
     hasNoFirst,
     hasNoLast,
     hasNoIndex,
@@ -488,54 +494,40 @@ function parseTForEach(node: Element, ctx: ParsingContext): AST | null {
 }
 
 /**
- * @returns true if we are sure the ast does not contain any component
+ * @returns true if we are sure that a deep remove (without optimisation) is needed, for exemple
+ * if there is a portal.
  */
-function hasNoComponent(ast: AST): boolean {
+function needDeepRemove(ast: AST): boolean {
   switch (ast.type) {
+    case ASTType.Multi:
+    case ASTType.TForEach:
+    case ASTType.TIf:
+      return ast.deepRemove;
+
+    case ASTType.TPortal:
+      return true;
+
     case ASTType.TComponent:
     case ASTType.TOut:
     case ASTType.TCall:
     case ASTType.TCallBlock:
     case ASTType.TSlot:
-      return false;
-    case ASTType.TSet:
     case ASTType.Text:
     case ASTType.Comment:
     case ASTType.TEsc:
-      return true;
+      return false;
+
     case ASTType.TKey:
-      return hasNoComponent(ast.content);
+      return needDeepRemove(ast.content);
     case ASTType.TDebug:
     case ASTType.TLog:
     case ASTType.TTranslation:
-      return ast.content ? hasNoComponent(ast.content) : true;
-    case ASTType.TForEach:
-      return ast.hasNoComponent;
-    case ASTType.Multi:
-    case ASTType.DomNode: {
-      for (let elem of ast.content) {
-        if (!hasNoComponent(elem)) {
-          return false;
-        }
-      }
-      return true;
-    }
-    case ASTType.TIf: {
-      if (!hasNoComponent(ast.content)) {
-        return false;
-      }
-      if (ast.tElif) {
-        for (let elem of ast.tElif) {
-          if (!hasNoComponent(elem.content)) {
-            return false;
-          }
-        }
-      }
-      if (ast.tElse && !hasNoComponent(ast.tElse)) {
-        return false;
-      }
-      return true;
-    }
+      return ast.content ? needDeepRemove(ast.content) : false;
+    case ASTType.TSet:
+      return ast.body ? ast.body.some((ast) => needDeepRemove(ast)) : false;
+
+    case ASTType.DomNode:
+      return ast.content.some((ast) => needDeepRemove(ast));
   }
 }
 
@@ -636,12 +628,21 @@ function parseTIf(node: Element, ctx: ParsingContext): AST | null {
     nextElement.remove();
   }
 
+  let deepRemove = needDeepRemove(content);
+  if (tElifs) {
+    deepRemove = deepRemove || tElifs.some((ast) => needDeepRemove(ast));
+  }
+  if (tElse) {
+    deepRemove = deepRemove || needDeepRemove(tElse);
+  }
+
   return {
     type: ASTType.TIf,
     condition,
     content,
     tElif: tElifs.length ? tElifs : null,
     tElse,
+    deepRemove,
   };
 }
 
@@ -805,6 +806,35 @@ function parseTTranslation(node: Element, ctx: ParsingContext): AST | null {
   };
 }
 
+// -----------------------------------------------------------------------------
+// Portal
+// -----------------------------------------------------------------------------
+
+function parseTPortal(node: Element, ctx: ParsingContext): AST | null {
+  if (!node.hasAttribute("t-portal")) {
+    return null;
+  }
+  if (node.tagName !== "t") {
+    throw new Error(
+      `Directive 't-portal' can only be used on <t> nodes (used on a <${node.tagName}>)`
+    );
+  }
+  const target = node.getAttribute("t-portal")!;
+  node.removeAttribute("t-portal");
+  const content = parseNode(node, ctx);
+  if (!content) {
+    return {
+      type: ASTType.Text,
+      value: "",
+    };
+  }
+  return {
+    type: ASTType.TPortal,
+    target,
+    content,
+  };
+}
+
 // -----------------------------------------------------------------------------
 // helpers
 // -----------------------------------------------------------------------------
@@ -839,7 +869,11 @@ function parseChildNodes(node: Node, ctx: ParsingContext): AST | null {
     case 1:
       return children[0];
     default:
-      return { type: ASTType.Multi, content: children };
+      return {
+        type: ASTType.Multi,
+        content: children,
+        deepRemove: children.some((ast) => needDeepRemove(ast)),
+      };
   }
 }
 

src/index.ts

@@ -36,7 +36,6 @@ export { App, mount } from "./app/app";
 export { Component } from "./component/component";
 export { useComponent } from "./component/component_node";
 export { status } from "./component/status";
-export { Portal } from "./portal";
 export { Memo } from "./memo";
 export { xml } from "./app/template_set";
 export { useState, reactive } from "./reactivity";

src/portal.ts

@@ -1,10 +1,8 @@
-import { xml } from "./app/template_set";
 import { BDom, text, VNode } from "./blockdom";
-import { Component } from "./component/component";
 
 const VText: any = text("").constructor;
 
-class VPortal extends VText implements Partial<VNode<VPortal>> {
+export class VPortal extends VText implements Partial<VNode<VPortal>> {
   //   selector: string;
   realBDom: BDom | null;
   target: HTMLElement | null = null;
@@ -49,19 +47,3 @@ class VPortal extends VText implements Partial<VNode<VPortal>> {
     }
   }
 }
-
-export class Portal extends Component {
-  static template = xml`<t t-slot="default"/>`;
-  static props = {
-    target: {
-      type: String,
-    },
-    slots: true,
-  };
-
-  setup() {
-    const node = this.__owl__;
-    const renderFn = node.renderFn;
-    node.renderFn = () => new VPortal(this.props.target, renderFn());
-  }
-}