[FIX] compiler: properly handle <t> tags in some cases

The problem was that the compiler is based on the assumption that the
multi block received by the parser only occurs in some cases
where the structure of the template require a multi block, and it does
not work when we have random multiblock elsewhere.

We could fix the issue by modifying the code generator code to support
these usecases, or by simply removing these cases in the parser. Since
this seems more efficient, this is the approach taken by this commit.

Note that it was a good opportunity to simplify the parser.

src/compiler/parser.ts

@@ -217,24 +217,7 @@ function parseTNode(node: Element, ctx: ParsingContext): AST | null {
   if (node.tagName !== "t") {
     return null;
   }
-  const children: AST[] = [];
-  for (let child of node.childNodes) {
-    const ast = parseNode(child, ctx);
-    if (ast) {
-      children.push(ast);
-    }
-  }
-  switch (children.length) {
-    case 0:
-      return null;
-    case 1:
-      return children[0];
-    default:
-      return {
-        type: ASTType.Multi,
-        content: children,
-      };
-  }
+  return parseChildNodes(node, ctx);
 }
 
 // -----------------------------------------------------------------------------
@@ -299,7 +282,6 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
     return null;
   }
   ctx = Object.assign({}, ctx);
-  const children: AST[] = [];
   if (tagName === "pre") {
     ctx.inPreTag = true;
   }
@@ -309,12 +291,7 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
   const ref = node.getAttribute("t-ref");
   node.removeAttribute("t-ref");
 
-  for (let child of node.childNodes) {
-    const ast = parseNode(child, ctx);
-    if (ast) {
-      children.push(ast);
-    }
-  }
+  const children = parseChildren(node, ctx);
 
   const nodeAttrsNames = node.getAttributeNames();
   const attrs: ASTDomNode["attrs"] = {};
@@ -600,13 +577,7 @@ function parseTCall(node: Element, ctx: ParsingContext): AST | null {
       };
     }
   }
-  const body: AST[] = [];
-  for (let child of node.childNodes) {
-    const ast = parseNode(child, ctx);
-    if (ast) {
-      body.push(ast);
-    }
-  }
+  const body = parseChildren(node, ctx);
 
   return {
     type: ASTType.TCall,
@@ -687,13 +658,7 @@ function parseTSetNode(node: Element, ctx: ParsingContext): AST | null {
   const defaultValue = node.innerHTML === node.textContent ? node.textContent || null : null;
   let body: AST[] | null = null;
   if (node.textContent !== node.innerHTML) {
-    body = [];
-    for (let child of node.childNodes) {
-      let childAst = parseNode(child, ctx);
-      if (childAst) {
-        body.push(childAst);
-      }
-    }
+    body = parseChildren(node, ctx);
   }
   return { type: ASTType.TSet, name, value, defaultValue, body };
 }
@@ -844,14 +809,30 @@ function parseTTranslation(node: Element, ctx: ParsingContext): AST | null {
 // helpers
 // -----------------------------------------------------------------------------
 
-function parseChildNodes(node: Element, ctx: ParsingContext): AST | null {
+/**
+ * Parse all the child nodes of a given node and return a list of ast elements
+ */
+function parseChildren(node: Node, ctx: ParsingContext): AST[] {
   const children: AST[] = [];
   for (let child of node.childNodes) {
     const childAst = parseNode(child, ctx);
     if (childAst) {
-      children.push(childAst);
+      if (childAst.type === ASTType.Multi) {
+        children.push(...childAst.content);
+      } else {
+        children.push(childAst);
+      }
     }
   }
+  return children;
+}
+
+/**
+ * Parse all the child nodes of a given node and return an ast if possible.
+ * In the case there are multiple children, they are wrapped in a astmulti.
+ */
+function parseChildNodes(node: Node, ctx: ParsingContext): AST | null {
+  const children = parseChildren(node, ctx);
   switch (children.length) {
     case 0:
       return null;

tests/blockdom/multi.test.ts

@@ -76,4 +76,11 @@ describe("multi blocks", () => {
     mount(text(multi([text("a"), text("b")]) as any), fixture);
     expect(fixture.innerHTML).toBe("ab");
   });
+
+  test("multi inside a block", async () => {
+    const block = createBlock("<div><block-child-0/></div>");
+    const tree = block([], [multi([text("foo"), text("bar")])]);
+    mount(tree, fixture);
+    expect(fixture.innerHTML).toBe("<div>foobar</div>");
+  });
 });

tests/compiler/__snapshots__/simple_templates.test.ts.snap

@@ -308,6 +308,39 @@ exports[`simple templates, mostly static t-esc in dom node, variations 2`] = `
 }"
 `;
 
+exports[`simple templates, mostly static template with multiple t tag with multiple content 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div><block-text-0/><block-text-1/>Loading<block-text-2/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let d1 = ctx['a'];
+    let d2 = ctx['b'];
+    let d3 = ctx['c'];
+    return block1([d1, d2, d3]);
+  }
+}"
+`;
+
+exports[`simple templates, mostly static template with t tag with multiple content 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div>Loading<block-child-0/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2;
+    if (false) {
+      b2 = text(\`\`);
+    }
+    return block1([], [b2]);
+  }
+}"
+`;
+
 exports[`simple templates, mostly static two t-escs next to each other 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/compiler/parser.test.ts

@@ -149,6 +149,93 @@ describe("qweb parser", () => {
     });
   });
 
+  test("dom node with t multi inside", async () => {
+    const template = `<div><t>Loading<t t-esc="abc"/></t></div>`;
+    expect(parse(template)).toEqual({
+      type: ASTType.DomNode,
+      tag: "div",
+      dynamicTag: null,
+      attrs: {},
+      on: {},
+      ref: null,
+      model: null,
+      ns: null,
+      content: [
+        { type: ASTType.Text, value: "Loading" },
+        { type: ASTType.TEsc, expr: "abc", defaultValue: "" },
+      ],
+    });
+  });
+
+  test("dom node with multiple t multi inside", async () => {
+    const template = `
+      <div>
+        <t t-esc="a"/>
+        <t>
+          <t t-esc="b"/>
+          <t>Loading<t t-esc="c"/></t>
+        </t>
+      </div>`;
+    expect(parse(template)).toEqual({
+      type: ASTType.DomNode,
+      tag: "div",
+      dynamicTag: null,
+      attrs: {},
+      on: {},
+      ref: null,
+      model: null,
+      ns: null,
+      content: [
+        { type: ASTType.TEsc, expr: "a", defaultValue: "" },
+        { type: ASTType.TEsc, expr: "b", defaultValue: "" },
+        { type: ASTType.Text, value: "Loading" },
+        { type: ASTType.TEsc, expr: "c", defaultValue: "" },
+      ],
+    });
+  });
+
+  test("dom node with t multi inside", async () => {
+    const template = `<div><t><t>Loading<t t-esc="abc"/></t></t></div>`;
+    expect(parse(template)).toEqual({
+      type: ASTType.DomNode,
+      tag: "div",
+      dynamicTag: null,
+      attrs: {},
+      on: {},
+      ref: null,
+      model: null,
+      ns: null,
+      content: [
+        { type: ASTType.Text, value: "Loading" },
+        { type: ASTType.TEsc, expr: "abc", defaultValue: "" },
+      ],
+    });
+  });
+
+  test("dom node with two t multi inside", async () => {
+    const template = `
+      <div>
+        <t><t t-esc="a"/><t t-esc="b"/></t>
+        <t><t t-esc="c"/><t t-esc="d"/></t>
+      </div>`;
+    expect(parse(template)).toEqual({
+      type: ASTType.DomNode,
+      tag: "div",
+      dynamicTag: null,
+      attrs: {},
+      on: {},
+      ref: null,
+      model: null,
+      ns: null,
+      content: [
+        { type: ASTType.TEsc, expr: "a", defaultValue: "" },
+        { type: ASTType.TEsc, expr: "b", defaultValue: "" },
+        { type: ASTType.TEsc, expr: "c", defaultValue: "" },
+        { type: ASTType.TEsc, expr: "d", defaultValue: "" },
+      ],
+    });
+  });
+
   test("dom node next to text node", async () => {
     expect(parse("some text<span></span>")).toEqual({
       type: ASTType.Multi,

tests/compiler/simple_templates.test.ts

@@ -137,4 +137,21 @@ describe("simple templates, mostly static", () => {
     const template = '<t><t t-esc="`text ${v}`"/></t>';
     expect(renderToString(template, { v: "from context" })).toBe("text from context");
   });
+
+  test("template with t tag with multiple content", () => {
+    const template = `<div><t>Loading<t t-if="false"/></t></div>`;
+    expect(renderToString(template)).toBe("<div>Loading</div>");
+  });
+
+  test("template with multiple t tag with multiple content", () => {
+    const template = `
+      <div>
+        <t t-esc="a"/>
+        <t>
+          <t t-esc="b"/>
+          <t>Loading<t t-esc="c"/></t>
+        </t>
+      </div>`;
+    expect(renderToString(template, { a: "a", b: "b", c: "c" })).toBe("<div>abLoadingc</div>");
+  });
 });