[FIX] qweb: evaluate body of t-set immediately

src/qweb/base_directives.ts

@@ -28,17 +28,14 @@ function compileValueNode(value: any, node: Element, qweb: QWeb, ctx: Compilatio
       // the 'zero' magical symbol is where we can find the result of the rendering
       // of  the body of the t-call.
       ctx.rootContext.shouldDefineUtils = true;
-      ctx.addLine(`c${ctx.parentNode}.push(...scope[utils.zero]);`);
+      const zeroArgs = ctx.escaping
+        ? `{text: utils.vDomToString(scope[utils.zero])}`
+        : `...scope[utils.zero]`;
+      ctx.addLine(`c${ctx.parentNode}.push(${zeroArgs});`);
     }
     return;
   }
 
-  if (value.xml instanceof NodeList && !value.id) {
-    for (let node of Array.from(value.xml)) {
-      qweb._compileNode(<ChildNode>node, ctx);
-    }
-    return;
-  }
   let exprID: string;
   if (typeof value === "string") {
     exprID = `_${ctx.generateID()}`;
@@ -47,7 +44,15 @@ function compileValueNode(value: any, node: Element, qweb: QWeb, ctx: Compilatio
     exprID = `scope.${value.id}`;
   }
   ctx.addIf(`${exprID} || ${exprID} === 0`);
+
   if (ctx.escaping) {
+    let protectID;
+    if (value.hasBody) {
+      protectID = ctx.startProtectScope();
+      ctx.addLine(
+        `${exprID} = ${exprID} instanceof utils.VDomArray ? utils.vDomToString(${exprID}) : ${exprID};`
+      );
+    }
     if (ctx.parentTextNode) {
       ctx.addLine(`vn${ctx.parentTextNode}.text += ${exprID};`);
     } else if (ctx.parentNode) {
@@ -61,21 +66,25 @@ function compileValueNode(value: any, node: Element, qweb: QWeb, ctx: Compilatio
         ctx.addLine(`result = vn${nodeID}`);
       }
     }
+    if (value.hasBody) {
+      ctx.stopProtectScope(protectID);
+    }
   } else {
     ctx.rootContext.shouldDefineUtils = true;
-    ctx.addLine(`c${ctx.parentNode}.push(...utils.htmlToVDOM(${exprID}));`);
+    if (value.hasBody) {
+      ctx.addLine(
+        `const vnodeArray = ${exprID} instanceof utils.VDomArray ? ${exprID} : utils.htmlToVDOM(${exprID});`
+      );
+      ctx.addLine(`c${ctx.parentNode}.push(...vnodeArray);`);
+    } else {
+      ctx.addLine(`c${ctx.parentNode}.push(...utils.htmlToVDOM(${exprID}));`);
+    }
   }
   if (node.childNodes.length) {
     ctx.addElse();
     qweb._compileChildren(node, ctx);
   }
 
-  if (value.xml instanceof NodeList && value.id) {
-    ctx.addElse();
-    for (let node of Array.from(value.xml)) {
-      qweb._compileNode(<ChildNode>node, ctx);
-    }
-  }
   ctx.closeIf();
 }
 
@@ -106,25 +115,46 @@ QWeb.addDirective({
   name: "set",
   extraNames: ["value"],
   priority: 60,
-  atNodeEncounter({ node, ctx }): boolean {
+  atNodeEncounter({ node, qweb, ctx }): boolean {
     ctx.rootContext.shouldDefineScope = true;
     const variable = node.getAttribute("t-set")!;
     let value = node.getAttribute("t-value")!;
     ctx.variables[variable] = ctx.variables[variable] || {};
     let qwebvar = ctx.variables[variable];
+    const hasBody = node.hasChildNodes();
 
+    qwebvar.id = variable;
+    qwebvar.expr = `scope.${variable}`;
     if (value) {
       const formattedValue = ctx.formatExpression(value);
-      if (ctx.variables.hasOwnProperty(variable) && qwebvar.id) {
-        ctx.addLine(`${qwebvar.expr} = ${formattedValue}`);
-      } else {
-        ctx.addLine(`scope.${variable} = ${formattedValue};`);
-        qwebvar.id = variable;
-        qwebvar.expr = `scope.${variable}`;
-        qwebvar.value = formattedValue;
+      ctx.addLine(`${qwebvar.expr} = ${formattedValue};`);
+      qwebvar.value = formattedValue;
+    }
+
+    if (hasBody) {
+      ctx.rootContext.shouldDefineUtils = true;
+      if (value) {
+        ctx.addIf(`!(${qwebvar.expr})`);
+      }
+      const tempParentNodeID = ctx.generateID();
+      const _parentNode = ctx.parentNode;
+      ctx.parentNode = tempParentNodeID;
+
+      ctx.addLine(`const c${tempParentNodeID} = new utils.VDomArray();`);
+      const nodeCopy = node.cloneNode(true) as Element;
+      for (let attr of ["t-set", "t-value", "t-if", "t-else", "t-elif"]) {
+        nodeCopy.removeAttribute(attr);
+      }
+      qweb._compileNode(nodeCopy, ctx);
+
+      ctx.addLine(`${qwebvar.expr} = c${tempParentNodeID}`);
+      qwebvar.value = `c${tempParentNodeID}`;
+      qwebvar.hasBody = true;
+
+      ctx.parentNode = _parentNode;
+      if (value) {
+        ctx.closeIf();
       }
-    } else {
-      qwebvar.xml = node.childNodes;
     }
     return true;
   }
@@ -205,12 +235,12 @@ QWeb.addDirective({
     // Step 3: compile t-call body if necessary
     // ------------------------------------------------
     let hasBody = node.hasChildNodes();
+    let protectID;
     if (hasBody) {
       // we add a sub scope to protect the ambient scope
       ctx.addLine(`{`);
       ctx.indent();
-      ctx.addLine(`let origScope = scope;`);
-      ctx.addLine(`scope = Object.assign(Object.create(context), scope);`);
+      protectID = ctx.startProtectScope();
       const nodeCopy = node.cloneNode(true) as Element;
       nodeCopy.removeAttribute("t-call");
       const parentNode = ctx.parentNode;
@@ -229,7 +259,7 @@ QWeb.addDirective({
 
     // Step 4: add the appropriate function call to current component
     // ------------------------------------------------
-    const callingScope = hasBody ? 'scope' : 'Object.assign(Object.create(context), scope)';
+    const callingScope = hasBody ? "scope" : "Object.assign(Object.create(context), scope)";
     if (ctx.parentNode) {
       ctx.addLine(
         `this.subTemplates['${subTemplate}'].call(this, ${callingScope}, Object.assign({}, extra, {parentNode: c${ctx.parentNode}}));`
@@ -247,7 +277,7 @@ QWeb.addDirective({
     // Step 5: restore previous scope
     // ------------------------------------------------
     if (hasBody) {
-      ctx.addLine(`scope = origScope;`);
+      ctx.stopProtectScope(protectID);
       ctx.dedent();
       ctx.addLine(`}`);
     }
@@ -279,9 +309,7 @@ QWeb.addDirective({
     ctx.addLine(`_${valuesID} = Object.values(_${arrayID});`);
     ctx.closeIf();
     ctx.addLine(`var _length${keysID} = _${keysID}.length;`);
-    let varsID = ctx.generateID();
-    ctx.addLine(`let _scope${varsID} = scope;`);
-    ctx.addLine(`scope = Object.assign(Object.create(context), _scope${varsID});`);
+    let varsID = ctx.startProtectScope();
     const loopVar = `i${ctx.loopNumber}`;
     ctx.addLine(`for (let ${loopVar} = 0; ${loopVar} < _length${keysID}; ${loopVar}++) {`);
     ctx.indent();
@@ -307,7 +335,7 @@ QWeb.addDirective({
     qweb._compileNode(nodeCopy, ctx);
     ctx.dedent();
     ctx.addLine("}");
-    ctx.addLine(`scope = _scope${varsID};`);
+    ctx.stopProtectScope(varsID);
     return true;
   }
 });

src/qweb/compilation_context.ts

@@ -174,4 +174,14 @@ export class CompilationContext {
     let r = s.replace(/\{\{.*?\}\}/g, s => "${" + this.formatExpression(s.slice(2, -2)) + "}");
     return "`" + r + "`";
   }
+  startProtectScope(): number {
+    const protectID = this.generateID();
+    this.rootContext.shouldDefineScope = true;
+    this.addLine(`const _origScope${protectID} = scope;`);
+    this.addLine(`scope = Object.assign(Object.create(context), scope);`);
+    return protectID;
+  }
+  stopProtectScope(protectID: number) {
+    this.addLine(`scope = _origScope${protectID};`);
+  }
 }

src/qweb/expression_parser.ts

@@ -42,6 +42,7 @@ export interface QWebVar {
   id: string; // foo
   expr: string; // scope.foo (local variables => only foo)
   value?: string; // 1 + 3
+  hasBody?: boolean;
 }
 
 //------------------------------------------------------------------------------

src/qweb/qweb.ts

@@ -87,7 +87,7 @@ interface Utils {
 }
 
 const UTILS: Utils = {
-  zero: Symbol('zero'),
+  zero: Symbol("zero"),
   toObj(expr) {
     if (typeof expr === "string") {
       expr = expr.trim();
@@ -106,6 +106,20 @@ const UTILS: Utils = {
   shallowEqual,
   addNameSpace(vnode) {
     addNS(vnode.data, vnode.children, vnode.sel);
+  },
+  VDomArray: class VDomArray extends Array {},
+  vDomToString: function(vdom: VNode[]): string {
+    return vdom
+      .map(vnode => {
+        if (vnode.sel) {
+          const node = document.createElement(vnode.sel);
+          const result = patch(node, vnode);
+          return (<HTMLElement>result.elm).outerHTML;
+        } else {
+          return vnode.text;
+        }
+      })
+      .join();
   }
 };
 
@@ -383,7 +397,7 @@ export class QWeb extends EventBus {
 
     let code = ctx.generateCode();
     const templateName = ctx.templateName.replace(/`/g, "'").slice(0, 200);
-    code.unshift(`    // Template name: "${templateName}"`)
+    code.unshift(`    // Template name: "${templateName}"`);
 
     let template;
     try {