From 7e6b1a28a0b41febfcb7d653016d881075c5e6af Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Thu, 21 Nov 2019 15:17:25 +0100
Subject: [PATCH] [IMP] qweb: throw error if t-component not used on a node t

Before this rev, using t-component on a div (for example) node
would silently ignore the div and replace it by the root node of
the component. A way to improve this was to create an extra div
node and to put the component inside it. However, it raised
questions: what do we do with other attributes set on this tag?
Do we apply them on the div, or on the component? In addition,
some of them only make sense on the component (e.g. props), so we
have to detect them. Whatever we would have decided, it wouldn't
have been obvious from the user point of view, so we chose not to
support it, and we thus now raise an error in this case.

Closes #487.
---
 doc/reference/component.md        |  2 ++
 src/qweb/qweb.ts                  |  2 ++
 tests/component/component.test.ts | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/doc/reference/component.md b/doc/reference/component.md
index 00639b6e3..85b52712f 100644
--- a/doc/reference/component.md
+++ b/doc/reference/component.md
@@ -1068,6 +1068,8 @@ class App extends Component<any, any, any> {
 In this example, the component `App` selects dynamically the concrete sub
 component class.
 
+Note that the `t-component` directive can only be used on `<t>` nodes.
+
 ### Asynchronous Rendering
 
 Working with asynchronous code always adds a lot of complexity to a system. Whenever
diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts
index 53cfe6359..250f27c49 100644
--- a/src/qweb/qweb.ts
+++ b/src/qweb/qweb.ts
@@ -460,6 +460,8 @@ export class QWeb extends EventBus {
       // this is a component, we modify in place the xml document to change
       // <SomeComponent ... /> to <t t-component="SomeComponent" ... />
       node.setAttribute("t-component", node.tagName);
+    } else if (node.tagName !== 't' && node.hasAttribute('t-component')) {
+      throw new Error(`Directive 't-component' can only be used on <t> nodes (used on a <${node.tagName}>)`);
     }
     const attributes = (<Element>node).attributes;
 
diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts
index 2b65b6dfb..3dcb7097d 100644
--- a/tests/component/component.test.ts
+++ b/tests/component/component.test.ts
@@ -1414,6 +1414,25 @@ describe("composition", () => {
     expect(env.qweb.templates.ParentWidget.fn.toString()).toMatchSnapshot();
   });
 
+  test("t-component not on a <t> node", async () => {
+    class Child extends Component<any, any> {
+      static template = xml`<span>1</span>`;
+    }
+    class Parent extends Component<any, any> {
+      static components = { Child };
+      static template = xml`<div><div t-component="Child"/></div>`;
+    }
+    const parent = new Parent();
+    let error;
+    try {
+      await parent.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Directive 't-component' can only be used on <t> nodes (used on a <div>)`);
+  });
+
   test("sub components, loops, and shouldUpdate", async () => {
     class ChildWidget extends Component<any, any> {
       static template = xml`<span><t t-esc="props.val"/></span>`;