[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
This commit is contained in:
Aaron Bohy
2020-03-02 11:01:50 +01:00
committed by Géry Debongnie
parent afa36f52a0
commit f5d019bb69
2 changed files with 25 additions and 3 deletions
+4 -3
View File
@@ -357,14 +357,15 @@ export class Component<Props extends {} = any, T extends Env = Env> {
*/ */
async render(force: boolean = false): Promise<void> { async render(force: boolean = false): Promise<void> {
const __owl__ = this.__owl__; 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, // 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, // or was unmounted and some state change triggered a render. Either way,
// we do not want to actually render anything in this case. // we do not want to actually render anything in this case.
return; return;
} }
if (__owl__.currentFiber && !__owl__.currentFiber.isRendered) { if (currentFiber && !currentFiber.isRendered && !currentFiber.isCompleted) {
return scheduler.addFiber(__owl__.currentFiber.root); return scheduler.addFiber(currentFiber.root);
} }
// if we aren't mounted at this point, it implies that there is a // 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 // currentFiber that is already rendered (isRendered is true), so we are
+21
View File
@@ -1366,6 +1366,27 @@ describe("async rendering", () => {
await prom; await prom;
}); });
test("concurrent renderings scenario 17", async () => {
class Parent extends Component {
static template = xml`<span><t t-esc="state.value"/></span>`;
state = useState({ value: 1 });
}
const parent = new Parent();
await parent.mount(fixture);
expect(fixture.innerHTML).toBe("<span>1</span>");
parent.state.value = 2;
parent.__owl__.currentFiber!.cancel();
parent.state.value = 3; // update value directly
await nextTick();
expect(fixture.innerHTML).toBe("<span>3</span>");
parent.state.value = 4; // update value after a tick
expect(fixture.innerHTML).toBe("<span>4</span>");
});
test("change state and call manually render: no unnecessary rendering", async () => { test("change state and call manually render: no unnecessary rendering", async () => {
class Widget extends Component { class Widget extends Component {
static template = xml`<div><t t-esc="state.val"/></div>`; static template = xml`<div><t t-esc="state.val"/></div>`;