[FIX] qweb: scope t-key directive to node and subnodes

not to siblings

closes #503

src/qweb/compilation_context.ts

@@ -32,7 +32,6 @@ export class CompilationContext {
   hasParentWidget: boolean = false;
   scopeVars: any[] = [];
   currentKey: string = "";
-  lastNodeKey: string = ""; // temp variable to communicate to previous caller
   templates: { [key: string]: boolean } = {};
 
   constructor(name?: string) {
@@ -61,8 +60,8 @@ export class CompilationContext {
     for (let i = 0; i < this.loopNumber - 1; i++) {
       locationExpr += `\${i${i + 1}}__`;
     }
-    if (this.lastNodeKey || this.currentKey) {
+    if (this.currentKey) {
-      const k = this.lastNodeKey || this.currentKey;
+      const k = this.currentKey;
       this.addLine(`let k${id} = ${locationExpr}\` + ${k};`);
     } else {
       locationExpr += this.loopNumber ? `\${i${this.loopNumber}}__\`` : "`";

src/qweb/extensions.ts

@@ -279,6 +279,6 @@ QWeb.addDirective({
   atNodeEncounter({ ctx, value }) {
     let id = ctx.generateID();
     ctx.addLine(`const nodeKey${id} = ${ctx.formatExpression(value)};`);
-    ctx.lastNodeKey = `nodeKey${id}`;
+    ctx.currentKey = `nodeKey${id}`;
   }
 });

src/qweb/qweb.ts

@@ -455,6 +455,10 @@ export class QWeb extends EventBus {
       return;
     }
 
+    if (ctx !== ctx.rootContext) {
+      ctx = ctx.subContext("currentKey", ctx.currentKey);
+    }
+
     const firstLetter = node.tagName[0];
     if (firstLetter === firstLetter.toUpperCase()) {
       // this is a component, we modify in place the xml document to change
@@ -544,7 +548,6 @@ export class QWeb extends EventBus {
     if (node.nodeName !== "t") {
       let nodeID = this._compileGenericNode(node, ctx, withHandlers);
       ctx = ctx.withParent(nodeID);
-      ctx = ctx.subContext("currentKey", ctx.lastNodeKey);
       let nodeHooks = {};
       let addNodeHook = function(hook, handler) {
         nodeHooks[hook] = nodeHooks[hook] || [];
@@ -728,7 +731,7 @@ export class QWeb extends EventBus {
       }
     }
     let nodeID = ctx.generateID();
-    let nodeKey = ctx.lastNodeKey || nodeID;
+    let nodeKey = ctx.currentKey || nodeID;
     const parts = [`key:${nodeKey}`];
     if (attrs.length + tattrs.length > 0) {
       parts.push(`attrs:{${attrs.join(",")}}`);

tests/component/__snapshots__/component.test.ts.snap

@@ -73,6 +73,78 @@ exports[`basic widget properties reconciliation alg works for t-foreach in t-for
 }"
 `;
 
+exports[`basic widget properties t-key on a component with t-if, and a sibling component 1`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.constructor.utils;
+    let QWeb = this.constructor;
+    let parent = context;
+    let owner = context;
+    let sibling = null;
+    var h = this.h;
+    let c1 = [], p1 = {key:1};
+    var vn1 = h('div', p1, c1);
+    if (false) {
+        const nodeKey2 = 'str';
+        //COMPONENT
+        let k5 = \`__6__\` + nodeKey2;
+        let w4 = k5 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[k5]] : false;
+        let props4 = {};
+        if (w4 && w4.__owl__.currentFiber && !w4.__owl__.vnode) {
+            w4.destroy();
+            w4 = false;
+        }
+        if (w4) {
+            w4.__updateProps(props4, extra.fiber, undefined, undefined, sibling);
+            let pvnode = w4.__owl__.pvnode;
+            c1.push(pvnode);
+        } else {
+            let componentKey4 = \`Child\`;
+            let W4 = context.constructor.components[componentKey4] || QWeb.components[componentKey4]|| context['Child'];
+            if (!W4) {throw new Error('Cannot find the definition of component \\"' + componentKey4 + '\\"')}
+            w4 = new W4(parent, props4);
+            parent.__owl__.cmap[k5] = w4.__owl__.id;
+            let def3 = w4.__prepare(extra.fiber, undefined, undefined, sibling);
+            let pvnode = h('dummy', {key: k5, hook: {insert(vn) { let nvn=w4.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});
+            const fiber = w4.__owl__.currentFiber;
+            def3.then(function () { if (fiber.isCompleted) { return; } const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
+            c1.push(pvnode);
+            w4.__owl__.pvnode = pvnode;
+        }
+        w4.__owl__.parentLastFiberId = extra.fiber.id;
+        sibling = w4.__owl__.currentFiber || sibling;
+    }
+    //COMPONENT
+    let k9 = \`__10__\`;
+    let w8 = k9 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[k9]] : false;
+    let props8 = {};
+    if (w8 && w8.__owl__.currentFiber && !w8.__owl__.vnode) {
+        w8.destroy();
+        w8 = false;
+    }
+    if (w8) {
+        w8.__updateProps(props8, extra.fiber, undefined, undefined, sibling);
+        let pvnode = w8.__owl__.pvnode;
+        c1.push(pvnode);
+    } else {
+        let componentKey8 = \`Child\`;
+        let W8 = context.constructor.components[componentKey8] || QWeb.components[componentKey8]|| context['Child'];
+        if (!W8) {throw new Error('Cannot find the definition of component \\"' + componentKey8 + '\\"')}
+        w8 = new W8(parent, props8);
+        parent.__owl__.cmap[k9] = w8.__owl__.id;
+        let def7 = w8.__prepare(extra.fiber, undefined, undefined, sibling);
+        let pvnode = h('dummy', {key: k9, hook: {insert(vn) { let nvn=w8.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w8.destroy();}}});
+        const fiber = w8.__owl__.currentFiber;
+        def7.then(function () { if (fiber.isCompleted) { return; } const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
+        c1.push(pvnode);
+        w8.__owl__.pvnode = pvnode;
+    }
+    w8.__owl__.parentLastFiberId = extra.fiber.id;
+    sibling = w8.__owl__.currentFiber || sibling;
+    return vn1;
+}"
+`;
+
 exports[`class and style attributes with t-component dynamic t-att-style is properly added and updated on widget root el 1`] = `
 "function anonymous(context,extra
 ) {

tests/component/component.test.ts

@@ -336,6 +336,26 @@ describe("basic widget properties", () => {
     await widget.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><div><span>1</span></div><div><span>2</span></div></div>");
   });
+
+  test("t-key on a component with t-if, and a sibling component", async () => {
+    class Child extends Component<any, any> {
+      static template = xml`<span>child</span>`;
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`
+        <div>
+          <Child t-if="false" t-key="'str'"/>
+          <Child/>
+        </div>`;
+      static components = { Child };
+    }
+
+    const widget = new Parent();
+    await widget.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><span>child</span></div>");
+    expect(env.qweb.templates[Parent.template].fn.toString()).toMatchSnapshot();
+  });
 });
 
 describe("lifecycle hooks", () => {

tests/qweb/__snapshots__/qweb.test.ts.snap

@@ -1592,12 +1592,12 @@ exports[`t-if t-set, then t-elif, part 3 1`] = `
         c1.push(vn4);
         c4.push({text: \`AAA\`});
     }
-        else if (!_3) {
+    else if (!_3) {
-            let c5 = [], p5 = {key:5};
+        let c5 = [], p5 = {key:5};
-            var vn5 = h('span', p5, c5);
+        var vn5 = h('span', p5, c5);
-            c1.push(vn5);
+        c1.push(vn5);
-            c5.push({text: \`BBB\`});
+        c5.push({text: \`BBB\`});
-        }
+    }
     return vn1;
 }"
 `;