Skip to content

Commit

Permalink
[IMP] qweb: throw error if t-component not used on a node t
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aab-odoo authored and ged-odoo committed Nov 21, 2019
1 parent 58f6724 commit 7e6b1a2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
2 changes: 2 additions & 0 deletions doc/reference/component.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/qweb/qweb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions tests/component/component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>`;
Expand Down

0 comments on commit 7e6b1a2

Please sign in to comment.