From af6aca83a26436e7eea8ca8f449eecf64259c7d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Sun, 23 Jun 2019 09:03:18 +0200 Subject: [PATCH] [IMP] components: allow multiple roots in slots closes #199 --- doc/component.md | 19 ----- src/qweb_core.ts | 24 ++++-- src/qweb_extensions.ts | 33 +++++++- tests/__snapshots__/component.test.ts.snap | 4 +- tests/component.test.ts | 95 +++++++++++++++++----- tools/playground/samples.js | 4 +- 6 files changed, 125 insertions(+), 54 deletions(-) diff --git a/doc/component.md b/doc/component.md index 192763c4..308d2b0e 100644 --- a/doc/component.md +++ b/doc/component.md @@ -932,25 +932,6 @@ In this example, the component `Dialog` will render the slots `content` and `foo with its parent as rendering context. This means that clicking on the button will execute the `doSomething` method on the parent, not on the dialog. -Warning! Slots have a technical constraint: the result of the slot rendering -should have exactly one root node. So, - -```xml - -
A
-
B
- -``` - -is not allowed. A workaround could be to wrap the content in a div: - -```xml -
-
A
-
B
-
-``` - Default slot: the first element inside the component which is not a named slot will be considered the `default` slot. For example: diff --git a/src/qweb_core.ts b/src/qweb_core.ts index 5d0b536b..db8935e7 100644 --- a/src/qweb_core.ts +++ b/src/qweb_core.ts @@ -268,9 +268,15 @@ export class QWeb extends EventBus { return template.fn.call(this, context, extra); } - _compile(name: string, elem: Element): CompiledTemplate { + _compile(name: string, elem: Element, parentNode?: number): CompiledTemplate { const isDebug = elem.attributes.hasOwnProperty("t-debug"); const ctx = new Context(name); + if (parentNode) { + ctx.nextID = parentNode + 1; + ctx.parentNode = parentNode; + ctx.allowMultipleRoots = true; + ctx.addLine(`let c${parentNode} = extra.parentNode;`); + } this._compileNode(elem, ctx); if (ctx.shouldProtectContext) { @@ -288,10 +294,12 @@ export class QWeb extends EventBus { ctx.code.unshift(" let utils = this.utils;"); } - if (!ctx.rootNode) { - throw new Error("A template should have one root node"); + if (!parentNode) { + if (!ctx.rootNode) { + throw new Error("A template should have one root node"); + } + ctx.addLine(`return vn${ctx.rootNode};`); } - ctx.addLine(`return vn${ctx.rootNode};`); let template; try { template = new Function( @@ -353,7 +361,6 @@ export class QWeb extends EventBus { // this is a component, we modify in place the xml document to change // to node.setAttribute("t-component", node.tagName); - node.nodeValue = "t"; } const attributes = (node).attributes; @@ -648,6 +655,7 @@ export class Context { inLoop: boolean = false; inPreTag: boolean = false; templateName: string; + allowMultipleRoots: boolean = false; constructor(name?: string) { this.rootContext = this; @@ -661,7 +669,11 @@ export class Context { } withParent(node: number): Context { - if (this === this.rootContext && (this.parentNode || this.parentTextNode)) { + if ( + !this.allowMultipleRoots && + this === this.rootContext && + (this.parentNode || this.parentTextNode) + ) { throw new Error("A template should not have more than one root node"); } if (!this.rootContext.rootNode) { diff --git a/src/qweb_extensions.ts b/src/qweb_extensions.ts index 1bccf35a..f9de2b0a 100644 --- a/src/qweb_extensions.ts +++ b/src/qweb_extensions.ts @@ -421,6 +421,18 @@ QWeb.addDirective({ : ctx.inLoop ? `String(-${componentID} - i)` : String(componentID); + if (ctx.allowMultipleRoots) { + // necessary to prevent collisions + if (!key && ctx.inLoop) { + let id = ctx.generateID(); + ctx.addLine( + `let template${id} = "_slot_" + String(-${componentID} - i)` + ); + templateID = `template${id}`; + } else { + templateID = `"_slot_${templateID}"`; + } + } let ref = node.getAttribute("t-ref"); let refExpr = ""; @@ -572,13 +584,24 @@ QWeb.addDirective({ slotNode.parentElement!.removeChild(slotNode); const key = slotNode.getAttribute("t-set")!; slotNode.removeAttribute("t-set"); - const slotFn = qweb._compile(`slot_${key}_template`, slotNode); + const slotFn = qweb._compile( + `slot_${key}_template`, + slotNode, + ctx.parentNode! + ); qweb.slots[`${slotId}_${key}`] = slotFn.bind(qweb); } } if (clone.childElementCount) { - const content = clone.children[0]; - const slotFn = qweb._compile(`slot_default_template`, content); + const t = clone.ownerDocument!.createElement("t"); + for (let child of Object.values(clone.children)) { + t.appendChild(child); + } + const slotFn = qweb._compile( + `slot_default_template`, + t, + ctx.parentNode! + ); qweb.slots[`${slotId}_default`] = slotFn.bind(qweb); } } @@ -683,7 +706,9 @@ QWeb.addDirective({ `const slot${slotKey} = this.slots[context.__owl__.slotId + '_' + '${value}'];` ); ctx.addLine( - `c${ctx.parentNode}.push(slot${slotKey}(context.__owl__.parent, extra));` + `slot${slotKey}(context.__owl__.parent, Object.assign({}, extra, {parentNode: c${ + ctx.parentNode + }}));` ); return true; } diff --git a/tests/__snapshots__/component.test.ts.snap b/tests/__snapshots__/component.test.ts.snap index ec0d302d..ab361803 100644 --- a/tests/__snapshots__/component.test.ts.snap +++ b/tests/__snapshots__/component.test.ts.snap @@ -1083,12 +1083,12 @@ exports[`t-slot directive can define and call slots 2`] = ` var vn2 = h('div', p2, c2); c1.push(vn2); const slot3 = this.slots[context.__owl__.slotId + '_' + 'header']; - c2.push(slot3(context.__owl__.parent, extra)); + slot3(context.__owl__.parent, Object.assign({}, extra, {parentNode: c2})); let c4 = [], p4 = {key:4}; var vn4 = h('div', p4, c4); c1.push(vn4); const slot5 = this.slots[context.__owl__.slotId + '_' + 'footer']; - c4.push(slot5(context.__owl__.parent, extra)); + slot5(context.__owl__.parent, Object.assign({}, extra, {parentNode: c4})); return vn1; }" `; diff --git a/tests/component.test.ts b/tests/component.test.ts index 590b113d..a52b95fc 100644 --- a/tests/component.test.ts +++ b/tests/component.test.ts @@ -2957,6 +2957,80 @@ describe("t-slot directive", () => { '
1
' ); }); + + test("content is the default slot", async () => { + env.qweb.addTemplates(` + +
+ + sts rocks + +
+
+ + `); + class Dialog extends Widget {} + class Parent extends Widget { + components = { Dialog }; + } + const parent = new Parent(env); + await parent.mount(fixture); + + expect(fixture.innerHTML).toBe( + "
sts rocks
" + ); + }); + + test("multiple roots are allowed in a named slot", async () => { + env.qweb.addTemplates(` + +
+ + + sts + rocks + + +
+
+ + `); + class Dialog extends Widget {} + class Parent extends Widget { + components = { Dialog }; + } + const parent = new Parent(env); + await parent.mount(fixture); + + expect(fixture.innerHTML).toBe( + "
stsrocks
" + ); + }); + + test("multiple roots are allowed in a default slot", async () => { + env.qweb.addTemplates(` + +
+ + sts + rocks + +
+
+ + `); + class Dialog extends Widget {} + class Parent extends Widget { + components = { Dialog }; + } + const parent = new Parent(env); + await parent.mount(fixture); + + expect(fixture.innerHTML).toBe( + "
stsrocks
" + ); + }); + }); describe("t-model directive", () => { @@ -3190,28 +3264,7 @@ describe("t-model directive", () => { expect(fixture.innerHTML).toBe("
invalid
"); }); - test("content is the default slot", async () => { - env.qweb.addTemplates(` - -
- - sts rocks - -
-
- - `); - class Dialog extends Widget {} - class Parent extends Widget { - components = { Dialog }; - } - const parent = new Parent(env); - await parent.mount(fixture); - expect(fixture.innerHTML).toBe( - "
sts rocks
" - ); - }); }); describe("environment and plugins", () => { diff --git a/tools/playground/samples.js b/tools/playground/samples.js index 2ff0a373..1bc55045 100644 --- a/tools/playground/samples.js +++ b/tools/playground/samples.js @@ -1132,10 +1132,10 @@ const SLOTS_XML = ` -
+
Card 2... []
-
+