From 6c753bb49f29898810a3b05ce4fea7ec8a0711d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 27 Nov 2019 07:24:33 +0100 Subject: [PATCH] [FIX] qweb: scope t-key directive to node and subnodes not to siblings closes #503 --- src/qweb/compilation_context.ts | 5 +- src/qweb/extensions.ts | 2 +- src/qweb/qweb.ts | 7 +- .../__snapshots__/component.test.ts.snap | 72 +++++++++++++++++++ tests/component/component.test.ts | 20 ++++++ tests/qweb/__snapshots__/qweb.test.ts.snap | 12 ++-- 6 files changed, 106 insertions(+), 12 deletions(-) diff --git a/src/qweb/compilation_context.ts b/src/qweb/compilation_context.ts index 16eb1e17..080ca417 100644 --- a/src/qweb/compilation_context.ts +++ b/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) { - const k = this.lastNodeKey || this.currentKey; + if (this.currentKey) { + const k = this.currentKey; this.addLine(`let k${id} = ${locationExpr}\` + ${k};`); } else { locationExpr += this.loopNumber ? `\${i${this.loopNumber}}__\`` : "`"; diff --git a/src/qweb/extensions.ts b/src/qweb/extensions.ts index 7a51c0ba..b7cfe5b0 100644 --- a/src/qweb/extensions.ts +++ b/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}`; } }); diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts index b14e58a0..741ac52a 100644 --- a/src/qweb/qweb.ts +++ b/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(",")}}`); diff --git a/tests/component/__snapshots__/component.test.ts.snap b/tests/component/__snapshots__/component.test.ts.snap index ef8b7ecc..ca35d828 100644 --- a/tests/component/__snapshots__/component.test.ts.snap +++ b/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 ) { diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index 4a814f13..c937e03a 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -336,6 +336,26 @@ describe("basic widget properties", () => { await widget.mount(fixture); expect(fixture.innerHTML).toBe("
1
2
"); }); + + test("t-key on a component with t-if, and a sibling component", async () => { + class Child extends Component { + static template = xml`child`; + } + + class Parent extends Component { + static template = xml` +
+ + +
`; + static components = { Child }; + } + + const widget = new Parent(); + await widget.mount(fixture); + expect(fixture.innerHTML).toBe("
child
"); + expect(env.qweb.templates[Parent.template].fn.toString()).toMatchSnapshot(); + }); }); describe("lifecycle hooks", () => { diff --git a/tests/qweb/__snapshots__/qweb.test.ts.snap b/tests/qweb/__snapshots__/qweb.test.ts.snap index ee914c4a..003f3fe5 100644 --- a/tests/qweb/__snapshots__/qweb.test.ts.snap +++ b/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) { - let c5 = [], p5 = {key:5}; - var vn5 = h('span', p5, c5); - c1.push(vn5); - c5.push({text: \`BBB\`}); - } + else if (!_3) { + let c5 = [], p5 = {key:5}; + var vn5 = h('span', p5, c5); + c1.push(vn5); + c5.push({text: \`BBB\`}); + } return vn1; }" `;