diff --git a/src/app/app.ts b/src/app/app.ts index 36773e98..c5ee215a 100644 --- a/src/app/app.ts +++ b/src/app/app.ts @@ -1,3 +1,4 @@ +import { onError, onMounted } from "../component/lifecycle_hooks"; import { Component } from "../component/component"; import { ComponentNode } from "../component/component_node"; import { MountOptions } from "../component/fibers"; @@ -64,8 +65,16 @@ export class App extends TemplateSet { throw new Error("Cannot mount a component on a detached dom node"); } const node = new ComponentNode(this.Root, this.props, this); + const promise: any = new Promise((resolve, reject) => { + onMounted(() => resolve(node.component)); + onError((e) => { + reject(e); + throw e; + }); + }); this.root = node; - return node.mountComponent(target, options); + node.mountComponent(target, options); + return promise; } destroy() { diff --git a/src/component/component_node.ts b/src/component/component_node.ts index ae9522e0..abb3f404 100644 --- a/src/component/component_node.ts +++ b/src/component/component_node.ts @@ -112,11 +112,10 @@ export class ComponentNode this.component.setup(); } - mountComponent(target: any, options?: MountOptions): Promise> { + mountComponent(target: any, options?: MountOptions) { const fiber = new MountFiber(this, target, options); this.app.scheduler.addFiber(fiber); this.initiateRender(fiber); - return fiber.promise.then(() => this.component); } async initiateRender(fiber: Fiber | MountFiber) { @@ -227,6 +226,9 @@ export class ComponentNode * a mounted hook failed and was handled. */ updateDom() { + if (!this.fiber) { + return; + } if (this.bdom === this.fiber!.bdom) { // If the error was handled by some child component, we need to find it to // apply its change diff --git a/src/component/error_handling.ts b/src/component/error_handling.ts index 534d63ba..810af9b4 100644 --- a/src/component/error_handling.ts +++ b/src/component/error_handling.ts @@ -20,23 +20,22 @@ function _handleError(node: ComponentNode | null, error: any, isFirstRound = fal fiber.root.counter--; } - let propagate = true; + let stopped = false; for (const h of errorHandlers) { try { h(error); - propagate = false; + stopped = true; + break; } catch (e) { error = e; } } - if (propagate) { - return _handleError(node.parent, error); + if (stopped) { + return true; } - return true; - } else { - return _handleError(node.parent, error); } + return _handleError(node.parent, error); } type ErrorParams = { error: any } & ({ node: ComponentNode } | { fiber: Fiber }); diff --git a/src/component/fibers.ts b/src/component/fibers.ts index e1942155..7911d84e 100644 --- a/src/component/fibers.ts +++ b/src/component/fibers.ts @@ -151,18 +151,11 @@ export interface MountOptions { export class MountFiber extends RootFiber { target: HTMLElement; position: Position; - resolve: any; - promise: Promise; - reject: any; constructor(node: ComponentNode, target: HTMLElement, options: MountOptions = {}) { super(node, null); this.target = target; this.position = options.position || "last-child"; - this.promise = new Promise((resolve, reject) => { - this.resolve = resolve; - this.reject = reject; - }); } complete() { let current: Fiber | undefined = this; @@ -195,10 +188,7 @@ export class MountFiber extends RootFiber { } node.fiber = null; } catch (e) { - if (!handleError({ fiber: current as Fiber, error: e })) { - this.reject(e); - } + handleError({ fiber: current as Fiber, error: e }); } - this.resolve(); } } diff --git a/src/component/scheduler.ts b/src/component/scheduler.ts index cb893aa9..c1a9a274 100644 --- a/src/component/scheduler.ts +++ b/src/component/scheduler.ts @@ -1,5 +1,5 @@ -import { Fiber, MountFiber, RootFiber } from "./fibers"; import { fibersInError } from "./error_handling"; +import { Fiber, RootFiber } from "./fibers"; import { STATUS } from "./status"; // ----------------------------------------------------------------------------- @@ -44,9 +44,6 @@ export class Scheduler { const hasError = fibersInError.has(fiber); if (hasError && fiber.counter !== 0) { this.tasks.delete(fiber); - if (fiber instanceof MountFiber) { - fiber.reject(fibersInError.get(fiber)); - } return; } if (fiber.node.status === STATUS.DESTROYED) { diff --git a/tests/components/__snapshots__/error_handling.test.ts.snap b/tests/components/__snapshots__/error_handling.test.ts.snap index 74749c63..a7b28036 100644 --- a/tests/components/__snapshots__/error_handling.test.ts.snap +++ b/tests/components/__snapshots__/error_handling.test.ts.snap @@ -739,6 +739,72 @@ exports[`can catch errors catchError in catchError 3`] = ` }" `; +exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers; + + let block1 = createBlock(\`
Some text
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + +exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 2`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let b2,b3; + if (ctx['state'].error) { + b2 = text(\`Error handled\`); + } else { + b3 = callSlot(ctx, node, key, 'default', false, {}); + } + return block1([], [b2, b3]); + } +}" +`; + +exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 3`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers; + + return function template(ctx, node, key = \\"\\") { + return text(\`OK\`); + } +}" +`; + +exports[`can catch errors error in mounted on a component with a sibling (properly mounted) 4`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers; + + let block1 = createBlock(\`
\`); + + function slot2(ctx, node, key = \\"\\") { + return component(\`ErrorComponent\`, {}, key + \`__3\`, node, ctx); + } + + return function template(ctx, node, key = \\"\\") { + let b2 = component(\`OK\`, {}, key + \`__1\`, node, ctx); + let b4 = component(\`ErrorBoundary\`, {slots: {'default': {__render: slot2, __ctx: ctx}}}, key + \`__4\`, node, ctx); + return block1([], [b2, b4]); + } +}" +`; + exports[`errors and promises a rendering error in a sub component will reject the mount promise 1`] = ` "function anonymous(bdom, helpers ) { diff --git a/tests/components/error_handling.test.ts b/tests/components/error_handling.test.ts index 82016f96..87d9f441 100644 --- a/tests/components/error_handling.test.ts +++ b/tests/components/error_handling.test.ts @@ -585,7 +585,6 @@ describe("can catch errors", () => { } } await mount(Root, fixture); - await nextTick(); expect(fixture.innerHTML).toBe("
Error handled
"); expect([ "Root:setup", @@ -634,7 +633,6 @@ describe("can catch errors", () => { } } await mount(Root, fixture); - await nextTick(); expect(fixture.innerHTML).toBe("
Error handled
"); expect([ "Root:setup", @@ -694,7 +692,6 @@ describe("can catch errors", () => { } } await mount(A, fixture); - await nextTick(); expect(fixture.innerHTML).toBe("
Error handled
"); expect([ "A:setup", @@ -723,6 +720,75 @@ describe("can catch errors", () => { ]).toBeLogged(); }); + test("error in mounted on a component with a sibling (properly mounted)", async () => { + class ErrorComponent extends Component { + static template = xml`
Some text
`; + setup() { + useLogLifecycle(); + onMounted(() => { + logStep("boom"); + throw new Error("NOOOOO"); + }); + } + } + class ErrorBoundary extends Component { + static template = xml`
+ Error handled + +
`; + state = useState({ error: false }); + + setup() { + useLogLifecycle(); + onError(() => (this.state.error = true)); + } + } + class OK extends Component { + static template = xml`OK`; + setup() { + useLogLifecycle(); + } + } + + class Root extends Component { + static template = xml`
+ + +
`; + static components = { ErrorBoundary, ErrorComponent, OK }; + setup() { + useLogLifecycle(); + } + } + await mount(Root, fixture); + expect(fixture.innerHTML).toBe("
OK
Error handled
"); + expect([ + "Root:setup", + "Root:willStart", + "Root:willRender", + "OK:setup", + "OK:willStart", + "ErrorBoundary:setup", + "ErrorBoundary:willStart", + "Root:rendered", + "OK:willRender", + "OK:rendered", + "ErrorBoundary:willRender", + "ErrorComponent:setup", + "ErrorComponent:willStart", + "ErrorBoundary:rendered", + "ErrorComponent:willRender", + "ErrorComponent:rendered", + "ErrorComponent:mounted", + "boom", + "ErrorBoundary:willRender", + "ErrorBoundary:rendered", + "ErrorBoundary:mounted", + "OK:mounted", + "Root:mounted", + ]).toBeLogged(); + }); + test("can catch an error in the willPatch call", async () => { class ErrorComponent extends Component { static template = xml`
`;