From 58f6724194a156c792b102a02436904a978c7cf4 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Thu, 21 Nov 2019 15:28:22 +0100
Subject: [PATCH] [FIX] component: reset isRendered when reusing fiber

It is important to reset the isRendered flag to false to ensure
that other (subsequent) simultaneous calls to render will be
skipped, as the currentFiber is actually no yet (re-)rendered.

Closes #483
---
 src/component/fiber.ts            |  1 +
 tests/component/component.test.ts | 45 +++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/src/component/fiber.ts b/src/component/fiber.ts
index a761cc48..e40ed43f 100644
--- a/src/component/fiber.ts
+++ b/src/component/fiber.ts
@@ -98,6 +98,7 @@ export class Fiber {
   _reuseFiber(oldFiber: Fiber) {
     oldFiber.cancel(); // cancel children fibers
     oldFiber.isCompleted = false; // keep the root fiber alive
+    oldFiber.isRendered = false; // the fiber has to be re-rendered
     if (oldFiber.child) {
       // remove relation to children
       oldFiber.child.parent = null;
diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts
index 9cd94647..2b65b6df 100644
--- a/tests/component/component.test.ts
+++ b/tests/component/component.test.ts
@@ -3463,6 +3463,51 @@ describe("async rendering", () => {
     expect(fixture.innerHTML).toBe("<div><span>2|5</span></div>");
   });
 
+  test("concurrent renderings scenario 12", async () => {
+    // In this scenario, we have a parent component that will be re-rendered
+    // several times simultaneously:
+    //    - once in a tick: it will create a new fiber, render it, but will have
+    //    to wait for its child (blocking) to be completed
+    //    - twice in the next tick: it will twice reuse the same fiber (as it is
+    //    rendered but not completed yet)
+    const def = makeDeferred();
+
+    class Child extends Component<any, any> {
+      static template = xml`<span><t t-esc="props.val"/></span>`;
+      willUpdateProps() {
+        return def;
+      }
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`<div><Child val="state.val"/></div>`;
+      static components = { Child };
+      state = useState({ val: 1 });
+    }
+    Parent.prototype.__render = jest.fn(Parent.prototype.__render);
+
+    const parent = new Parent();
+    await parent.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><span>1</span></div>");
+    expect(Parent.prototype.__render).toHaveBeenCalledTimes(1);
+
+    parent.state.val = 2;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><span>1</span></div>");
+    expect(Parent.prototype.__render).toHaveBeenCalledTimes(2);
+
+    parent.state.val = 3;
+    parent.state.val = 4;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><span>1</span></div>");
+    expect(Parent.prototype.__render).toHaveBeenCalledTimes(3);
+
+    def.resolve();
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><span>4</span></div>");
+    expect(Parent.prototype.__render).toHaveBeenCalledTimes(3);
+  });
+
   test("change state and call manually render: no unnecessary rendering", async () => {
     class Widget extends Component<any, any> {
       static template = xml`<div><t t-esc="state.val"/></div>`;