From 7933328d0a95e463fa762be1c3b9a2d6e8a3dd5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Sat, 30 Nov 2019 21:59:18 +0100 Subject: [PATCH] wip --- src/component/fiber.ts | 83 ++++++++++++++++++------------- src/component/scheduler.ts | 7 ++- tests/component/component.test.ts | 38 ++++++-------- 3 files changed, 67 insertions(+), 61 deletions(-) diff --git a/src/component/fiber.ts b/src/component/fiber.ts index 1e4c967f..09cb86e7 100644 --- a/src/component/fiber.ts +++ b/src/component/fiber.ts @@ -180,6 +180,7 @@ export class Fiber { */ complete() { let component = this.component; + let fiber: Fiber = this; this.isCompleted = true; if (!this.target && !component.__owl__.isMounted) { return; @@ -194,48 +195,58 @@ export class Fiber { this._walk(doWork); const patchLen = patchQueue.length; - // call willPatch hook on each fiber of patchQueue - for (let i = 0; i < patchLen; i++) { - const fiber = patchQueue[i]; - if (fiber.shouldPatch) { + try { + // call willPatch hook on each fiber of patchQueue + for (let i = 0; i < patchLen; i++) { + fiber = patchQueue[i]; + if (fiber.shouldPatch) { + component = fiber.component; + if (component.__owl__.willPatchCB) { + component.__owl__.willPatchCB(); + } + component.willPatch(); + } + } + + // call __patch on each fiber of (reversed) patchQueue + for (let i = patchLen - 1; i >= 0; i--) { + fiber = patchQueue[i]; component = fiber.component; - if (component.__owl__.willPatchCB) { - component.__owl__.willPatchCB(); + component.__patch(fiber.vnode!); + if (!fiber.shouldPatch && (!fiber.target || i !== 0)) { + component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm; } - component.willPatch(); + component.__owl__.currentFiber = null; } - } - // call __patch on each fiber of (reversed) patchQueue - for (let i = patchLen - 1; i >= 0; i--) { - const fiber = patchQueue[i]; - component = fiber.component; - component.__patch(fiber.vnode!); - if (!fiber.shouldPatch && (!fiber.target || i !== 0)) { - component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm; + // insert into the DOM (mount case) + let inDOM = false; + if (this.target) { + this.target.appendChild(this.component.el!); + inDOM = document.body.contains(this.target); } - component.__owl__.currentFiber = null; - } - // insert into the DOM (mount case) - let inDOM = false; - if (this.target) { - this.target.appendChild(this.component.el!); - inDOM = document.body.contains(this.target); - } - - // call patched/mounted hook on each fiber of (reversed) patchQueue - for (let i = patchLen - 1; i >= 0; i--) { - const fiber = patchQueue[i]; - component = fiber.component; - if (fiber.shouldPatch && !this.target) { - component.patched(); - if (component.__owl__.patchedCB) { - component.__owl__.patchedCB(); + // call patched/mounted hook on each fiber of (reversed) patchQueue + for (let i = patchLen - 1; i >= 0; i--) { + fiber = patchQueue[i]; + component = fiber.component; + if (fiber.shouldPatch && !this.target) { + component.patched(); + if (component.__owl__.patchedCB) { + component.__owl__.patchedCB(); + } + } else if (this.target ? inDOM : true) { + component.__callMounted(); } - } else if (this.target ? inDOM : true) { - component.__callMounted(); } + } catch (e) { + // if there is no current fiber on component, we are in the situation where + // components were patched to the DOM, but a mounted/patched hook threw an + // error. In that case, we cannot manage the error at a lower level than + // the root fiber, since some components may not have been properly mounted + // patched yet. + const errorFiber = component.__owl__.currentFiber ? fiber : this; + errorFiber.handleError(e); } } @@ -274,7 +285,11 @@ export class Fiber { qweb.trigger("error", error); if (canCatch) { + // this.root.isCompleted = false + this.root.isCompleted = false; + // component.__owl__.currentFiber!.root.isCompleted = false; component.catchError!(error); + } else { // the 3 next lines aim to mark the root fiber as being in error, and // to force it to end, without waiting for its children diff --git a/src/component/scheduler.ts b/src/component/scheduler.ts index 5013702b..7c311980 100644 --- a/src/component/scheduler.ts +++ b/src/component/scheduler.ts @@ -62,10 +62,9 @@ export class Scheduler { } if (task.fiber.counter === 0) { if (!task.fiber.error) { - try { - task.fiber.complete(); - } catch (e) { - task.fiber.handleError(e); + task.fiber.complete(); + if (!task.fiber.isCompleted) { + return true; } } task.callback(); diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index 44d19eb0..0de9e6b2 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -4992,41 +4992,35 @@ describe("component error handling (catchError)", () => { test.skip("can catch an error in the mounted call", async () => { // we do not catch error in mounted anymore console.error = jest.fn(); - env.qweb.addTemplates(` - -
- Error handled - -
-
Some text
-
- -
- `); - class ErrorComponent extends Widget { + + class ErrorComponent extends Component { + static template = xml`
Some text
`; mounted() { throw new Error("NOOOOO"); } } - class ErrorBoundary extends Widget { + class ErrorBoundary extends Component { + static template = xml` +
+ Error handled + +
`; state = useState({ error: false }); catchError() { this.state.error = true; } } - class App extends Widget { + class App extends Component { + static template = xml`
`; static components = { ErrorBoundary, ErrorComponent }; } const app = new App(); await app.mount(fixture); - await nextTick(); - await nextTick(); - await nextTick(); expect(fixture.innerHTML).toBe("
Error handled
"); }); - test.skip("can catch an error in the willPatch call", async () => { + test("can catch an error in the willPatch call", async () => { // we do not catch error in willPatch anymore const consoleError = console.error; console.error = jest.fn(); @@ -5054,18 +5048,16 @@ describe("component error handling (catchError)", () => { `; - state = useState({ message: "abc" }); + state = { message: "abc" }; static components = { ErrorBoundary, ErrorComponent }; } const app = new App(); await app.mount(fixture); expect(fixture.innerHTML).toBe("
abc
abc
"); app.state.message = "def"; - await nextTick(); - await nextTick(); - await nextTick(); + await app.render(); expect(fixture.innerHTML).toBe("
def
Error handled
"); - expect(console.error).toHaveBeenCalledTimes(1); + expect(console.error).toHaveBeenCalledTimes(0); console.error = consoleError; });