From f5d019bb69a59ace71466f3f17b64bc3718c0a1d Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Mon, 2 Mar 2020 11:01:50 +0100 Subject: [PATCH] [FIX] component: concurrency issue with cancelled fiber Since ee956a197, the rendering is skipped if the currentFiber is completed. Unfortunately, cancelled fibers remain set in __owl__, so when a fiber is cancelled, subsequent calls to render are skipped. It would be nice to reset __owl__.currentFiber to null when the fiber is cancelled, but when trying to do so, a lot of tests fail. Part of issue #622 Closes #665 --- src/component/component.ts | 7 ++++--- tests/component/async.test.ts | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/component/component.ts b/src/component/component.ts index 24590bfd..57d1e91b 100644 --- a/src/component/component.ts +++ b/src/component/component.ts @@ -357,14 +357,15 @@ export class Component { */ async render(force: boolean = false): Promise { const __owl__ = this.__owl__; - if (!__owl__.isMounted && !__owl__.currentFiber) { + const currentFiber = __owl__.currentFiber; + if (!__owl__.isMounted && !currentFiber) { // if we get here, this means that the component was either never mounted, // or was unmounted and some state change triggered a render. Either way, // we do not want to actually render anything in this case. return; } - if (__owl__.currentFiber && !__owl__.currentFiber.isRendered) { - return scheduler.addFiber(__owl__.currentFiber.root); + if (currentFiber && !currentFiber.isRendered && !currentFiber.isCompleted) { + return scheduler.addFiber(currentFiber.root); } // if we aren't mounted at this point, it implies that there is a // currentFiber that is already rendered (isRendered is true), so we are diff --git a/tests/component/async.test.ts b/tests/component/async.test.ts index fb1dd593..b9223dfd 100644 --- a/tests/component/async.test.ts +++ b/tests/component/async.test.ts @@ -1366,6 +1366,27 @@ describe("async rendering", () => { await prom; }); + test("concurrent renderings scenario 17", async () => { + class Parent extends Component { + static template = xml``; + state = useState({ value: 1 }); + } + + const parent = new Parent(); + await parent.mount(fixture); + expect(fixture.innerHTML).toBe("1"); + + parent.state.value = 2; + parent.__owl__.currentFiber!.cancel(); + + parent.state.value = 3; // update value directly + await nextTick(); + expect(fixture.innerHTML).toBe("3"); + + parent.state.value = 4; // update value after a tick + expect(fixture.innerHTML).toBe("4"); + }); + test("change state and call manually render: no unnecessary rendering", async () => { class Widget extends Component { static template = xml`
`;