[FIX] compiler: does not modify xml doc in place

src/app/template_set.ts

@@ -5,7 +5,7 @@ import { UTILS } from "./template_helpers";
 
 const bdom = { text, createBlock, list, multi, html, toggler, component, comment };
 
-export const globalTemplates: { [key: string]: string | Node } = {};
+export const globalTemplates: { [key: string]: string | Element } = {};
 
 function parseXML(xml: string): Document {
   const parser = new DOMParser();
@@ -67,7 +67,11 @@ export class TemplateSet {
     }
   }
 
-  addTemplate(name: string, template: string | Node, options: { allowDuplicate?: boolean } = {}) {
+  addTemplate(
+    name: string,
+    template: string | Element,
+    options: { allowDuplicate?: boolean } = {}
+  ) {
     if (name in this.rawTemplates && !options.allowDuplicate) {
       throw new Error(`Template ${name} already defined`);
     }
@@ -82,7 +86,6 @@ export class TemplateSet {
     xml = xml instanceof Document ? xml : parseXML(xml);
     for (const template of xml.querySelectorAll("[t-name]")) {
       const name = template.getAttribute("t-name")!;
-      template.removeAttribute("t-name");
       this.addTemplate(name, template, options);
     }
   }
@@ -106,7 +109,7 @@ export class TemplateSet {
     return this.templates[name];
   }
 
-  _compileTemplate(name: string, template: string | Node) {
+  _compileTemplate(name: string, template: string | Element) {
     return compile(template, {
       name,
       dev: this.dev,

src/compiler/index.ts

@@ -9,7 +9,10 @@ export type TemplateFunction = (blocks: any, utils: any) => Template;
 interface CompileOptions extends Config {
   name?: string;
 }
-export function compile(template: string | Node, options: CompileOptions = {}): TemplateFunction {
+export function compile(
+  template: string | Element,
+  options: CompileOptions = {}
+): TemplateFunction {
   // parsing
   const ast = parse(template);
 

src/compiler/parser.ts

@@ -180,24 +180,35 @@ export type AST =
 // -----------------------------------------------------------------------------
 // Parser
 // -----------------------------------------------------------------------------
+const cache: WeakMap<Element, AST> = new WeakMap();
+
+export function parse(xml: string | Element): AST {
+  if (typeof xml === "string") {
+    const elem = parseXML(`<t>${xml}</t>`).firstChild as Element;
+    return _parse(elem);
+  }
+  let ast = cache.get(xml);
+  if (!ast) {
+    // we clone here the xml to prevent modifying it in place
+    ast = _parse(xml.cloneNode(true) as Element);
+    cache.set(xml, ast);
+  }
+  return ast;
+}
+
+function _parse(xml: Element): AST {
+  normalizeXML(xml);
+  const ctx = { inPreTag: false, inSVG: false };
+  return parseNode(xml, ctx) || { type: ASTType.Text, value: "" };
+}
+
 interface ParsingContext {
   tModelInfo?: TModelInfo | null;
   inPreTag: boolean;
   inSVG: boolean;
 }
 
-export function parse(xml: string | Node): AST {
-  const node = xml instanceof Element ? xml : (parseXML(`<t>${xml}</t>`).firstChild! as Element);
-  normalizeXML(node);
-  const ctx = { inPreTag: false, inSVG: false };
-  const ast = parseNode(node, ctx);
-  if (!ast) {
-    return { type: ASTType.Text, value: "" };
-  }
-  return ast;
-}
-
-function parseNode(node: ChildNode, ctx: ParsingContext): AST | null {
+function parseNode(node: Node, ctx: ParsingContext): AST | null {
   if (!(node instanceof Element)) {
     return parseTextCommentNode(node, ctx);
   }
@@ -237,7 +248,7 @@ function parseTNode(node: Element, ctx: ParsingContext): AST | null {
 const lineBreakRE = /[\r\n]/;
 const whitespaceRE = /\s+/g;
 
-function parseTextCommentNode(node: ChildNode, ctx: ParsingContext): AST | null {
+function parseTextCommentNode(node: Node, ctx: ParsingContext): AST | null {
   if (node.nodeType === Node.TEXT_NODE) {
     let value = node.textContent || "";
     if (!ctx.inPreTag) {
@@ -360,7 +371,7 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
         ctx = Object.assign({}, ctx);
         ctx.tModelInfo = model;
       }
-    } else {
+    } else if (attr !== "t-name") {
       if (attr.startsWith("t-") && !attr.startsWith("t-att")) {
         throw new Error(`Unknown QWeb directive: '${attr}'`);
       }
@@ -801,7 +812,7 @@ function parseTPortal(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[] {
+function parseChildren(node: Element, ctx: ParsingContext): AST[] {
   const children: AST[] = [];
   for (let child of node.childNodes) {
     const childAst = parseNode(child, ctx);
@@ -820,7 +831,7 @@ function parseChildren(node: Node, ctx: ParsingContext): AST[] {
  * 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 {
+function parseChildNodes(node: Element, ctx: ParsingContext): AST | null {
   const children = parseChildren(node, ctx);
   switch (children.length) {
     case 0:

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

@@ -1,5 +1,19 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`loading templates addTemplates does not modify its xml document in place 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  
+  let block1 = createBlock(\`<div><block-text-0/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let txt1 = ctx['value'];
+    return block1([txt1]);
+  }
+}"
+`;
+
 exports[`loading templates can initialize qweb with a string 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/compiler/template_set.test.ts

@@ -24,6 +24,17 @@ describe("loading templates", () => {
     expect(context.renderToString("hey")).toBe("<div>jupiler</div>");
   });
 
+  test("addTemplates does not modify its xml document in place", () => {
+    const data = `<?xml version="1.0" encoding="UTF-8"?>
+      <templates id="template" xml:space="preserve"><div t-name="hey"><t t-esc="value"/></div></templates>`;
+    const xml = new DOMParser().parseFromString(data, "text/xml");
+    const context = new TestContext();
+    expect(xml.firstElementChild!.innerHTML).toBe(`<div t-name="hey"><t t-esc="value"/></div>`);
+    context.addTemplates(xml);
+    expect(context.renderToString("hey", { value: 123 })).toBe("<div>123</div>");
+    expect(xml.firstElementChild!.innerHTML).toBe(`<div t-name="hey"><t t-esc="value"/></div>`);
+  });
+
   test("can load a few templates from a xml string", () => {
     const data = `<?xml version="1.0" encoding="UTF-8"?>
       <templates id="template" xml:space="preserve">

tests/helpers.ts

@@ -123,7 +123,7 @@ export function snapshotEverything() {
   });
 
   const originalCompileTemplate = TemplateSet.prototype._compileTemplate;
-  TemplateSet.prototype._compileTemplate = function (name: string, template: string | Node) {
+  TemplateSet.prototype._compileTemplate = function (name: string, template: string | Element) {
     const fn = originalCompileTemplate.call(this, "", template);
     if (!globalTemplateNames.has(name)) {
       expect(fn.toString()).toMatchSnapshot();