From f2b3ebd1ec8fbcf812387ba39d09b518b75a9b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 13 Nov 2019 17:11:42 +0100 Subject: [PATCH] [FIX] component: properly set currentFiber to null in all cases Whenever a rendering is completed, we need to reset the currentFiber to null to make sure all subsequent renderings will not be confused. However, the way it was done before this commit was wrong in a specific situation: we resetted the currentFiber to null in the patch method. The idea was that this method was called every time the component completes a rendering. But this is not true: it can happen that a component is unmounted and remounted without changes. Then the component will be patched, but if there is no change, it will not call recursively the patched methods of its children. So, we need to choose a better place to reset it to null. closes #454 --- src/component/component.ts | 2 +- src/component/fiber.ts | 1 + tests/component/component.test.ts | 31 +++++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/component/component.ts b/src/component/component.ts index bda72e5f..c8b350bd 100644 --- a/src/component/component.ts +++ b/src/component/component.ts @@ -451,6 +451,7 @@ export class Component { } } __owl__.isMounted = true; + __owl__.currentFiber = null; try { this.mounted(); if (__owl__.mountedCB) { @@ -526,7 +527,6 @@ export class Component { const __owl__ = this.__owl__; const target = __owl__.vnode || document.createElement(vnode.sel!); __owl__.vnode = patch(target, vnode); - __owl__.currentFiber = null; } /** diff --git a/src/component/fiber.ts b/src/component/fiber.ts index 8bf3f91c..fab98f70 100644 --- a/src/component/fiber.ts +++ b/src/component/fiber.ts @@ -163,6 +163,7 @@ export class Fiber { const fiber = patchQueue[i]; component = fiber.component; component.__patch(fiber.vnode); + component.__owl__.currentFiber = null; } try { for (let i = patchLen - 1; i >= 0; i--) { diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index 28c4f34b..ecbb32b4 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -4889,6 +4889,37 @@ describe("unmounting and remounting", () => { await widget.mount(fixture); expect(fixture.innerHTML).toBe("
3
"); }); + + test("sub component is still active after being unmounted and remounted", async () => { + class Child extends Component { + static template = xml` +

+ +

`; + + state = useState({ value: 1 }); + } + + class Parent extends Component { + static components = { Child }; + static template = xml`
`; + } + + const w = new Parent(); + await w.mount(fixture); + expect(fixture.innerHTML).toBe("

1

"); + + fixture.querySelector("p")!.click(); + await nextTick(); + expect(fixture.innerHTML).toBe("

2

"); + w.unmount(); + await nextTick(); + await w.mount(fixture); + expect(fixture.innerHTML).toBe("

2

"); + fixture.querySelector("p")!.click(); + await nextTick(); + expect(fixture.innerHTML).toBe("

3

"); + }); }); describe("dynamic root nodes", () => {