Skip to content

Commit

Permalink
[FIX] component: strict check of deep argument truth value
Browse files Browse the repository at this point in the history
With this commit, we make sure that the `render` method was explicitely
called with the deep === true argument, instead of assuming that it is a
boolean.  This prevents errors when some unrelated value is given to the
render method. This could happen in some cases, such as

useBus(someBus, 'someevent', this.render)

In that case, the `useBus` code would simply call the this.render with a
customevent, which would be considered truthy before, and not anymore
  • Loading branch information
ged-odoo authored and sdegueldre committed Apr 11, 2022
1 parent 1179e84 commit 7d14db7
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 4 deletions.
3 changes: 2 additions & 1 deletion doc/reference/component.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ The `Component` class has a very small API.

By default, the render initiated by this method will stop at each child
component if their props are (shallow) equal. To force a render to update
all child components, one can use the optional `deep` argument.
all child components, one can use the optional `deep` argument. Note that the
value of the `deep` argument needs to be a boolean, not a truthy value.

## Static Properties

Expand Down
2 changes: 1 addition & 1 deletion src/component/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,6 @@ export class Component<Props = any, Env = any> {
setup() {}

render(deep: boolean = false) {
this.__owl__.render(deep);
this.__owl__.render(deep === true);
}
}
4 changes: 2 additions & 2 deletions src/component/component_node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function useState<T extends object>(state: T): Reactive<T> | NonReactive<
const node = getCurrent();
let render = batchedRenderFunctions.get(node)!;
if (!render) {
render = batched(node.render.bind(node));
render = batched(node.render.bind(node, false));
batchedRenderFunctions.set(node, render);
// manual implementation of onWillDestroy to break cyclic dependency
node.willDestroy.push(clearReactivesForCallback.bind(null, render));
Expand Down Expand Up @@ -193,7 +193,7 @@ export class ComponentNode<P extends object = any, E = any> implements VNode<Com
}
}

async render(deep: boolean = false) {
async render(deep: boolean) {
let current = this.fiber;
if (current && (current.root!.locked || (current as any).bdom === true)) {
await Promise.resolve();
Expand Down
24 changes: 24 additions & 0 deletions tests/components/__snapshots__/rendering.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,30 @@ exports[`rendering semantics props are reactive 2`] = `
}"
`;
exports[`rendering semantics render need a boolean = true to be 'deep' 1`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
return function template(ctx, node, key = \\"\\") {
const b2 = text(ctx['state'].value);
const b3 = component(\`Child\`, {}, key + \`__1\`, node, ctx);
return multi([b2, b3]);
}
}"
`;
exports[`rendering semantics render need a boolean = true to be 'deep' 2`] = `
"function anonymous(bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
return function template(ctx, node, key = \\"\\") {
return text(\`child\`);
}
}"
`;
exports[`rendering semantics render with deep=true followed by render with deep=false work as expected 1`] = `
"function anonymous(bdom, helpers
) {
Expand Down
37 changes: 37 additions & 0 deletions tests/components/rendering.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,43 @@ describe("rendering semantics", () => {
expect(childN).toBe(2);
});

test("render need a boolean = true to be 'deep'", async () => {
let childN = 0;
let parentN = 0;
class Child extends Component {
static template = xml`child`;
setup() {
onRendered(() => childN++);
}
}

class Parent extends Component {
static template = xml`
<t t-esc="state.value"/>
<Child/>
`;
static components = { Child };

state = { value: "A" };
setup() {
onRendered(() => parentN++);
}
}

const parent = await mount(Parent, fixture);

expect(fixture.innerHTML).toBe("Achild");
expect(parentN).toBe(1);
expect(childN).toBe(1);

parent.state.value = "B";
parent.render("true" as any as boolean);
await nextTick();
expect(fixture.innerHTML).toBe("Bchild");
expect(parentN).toBe(2);
expect(childN).toBe(1);
});

test("render with deep=true followed by render with deep=false work as expected", async () => {
class Child extends Component {
static template = xml`child<t t-esc="env.getValue()"/>`;
Expand Down

0 comments on commit 7d14db7

Please sign in to comment.