From 7d14db7d316179ef014e5fe40c3464e2c9e9621f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Mon, 11 Apr 2022 10:52:27 +0200
Subject: [PATCH] [FIX] component: strict check of deep argument truth value

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
---
 doc/reference/component.md                    |  3 +-
 src/component/component.ts                    |  2 +-
 src/component/component_node.ts               |  4 +-
 .../__snapshots__/rendering.test.ts.snap      | 24 ++++++++++++
 tests/components/rendering.test.ts            | 37 +++++++++++++++++++
 5 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/doc/reference/component.md b/doc/reference/component.md
index 5b01fbd0..6d8331f3 100644
--- a/doc/reference/component.md
+++ b/doc/reference/component.md
@@ -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
 
diff --git a/src/component/component.ts b/src/component/component.ts
index 220fb094..7fcfe885 100644
--- a/src/component/component.ts
+++ b/src/component/component.ts
@@ -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);
   }
 }
diff --git a/src/component/component_node.ts b/src/component/component_node.ts
index d35c1197..ba3ab568 100644
--- a/src/component/component_node.ts
+++ b/src/component/component_node.ts
@@ -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));
@@ -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();
diff --git a/tests/components/__snapshots__/rendering.test.ts.snap b/tests/components/__snapshots__/rendering.test.ts.snap
index a953fc24..8f44f53e 100644
--- a/tests/components/__snapshots__/rendering.test.ts.snap
+++ b/tests/components/__snapshots__/rendering.test.ts.snap
@@ -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
 ) {
diff --git a/tests/components/rendering.test.ts b/tests/components/rendering.test.ts
index ab785364..cb08437d 100644
--- a/tests/components/rendering.test.ts
+++ b/tests/components/rendering.test.ts
@@ -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()"/>`;