From b1d95eae114119d3803c67ecf94c9ea0de9e8198 Mon Sep 17 00:00:00 2001 From: Lucas Perais Date: Wed, 30 Nov 2022 15:22:51 +0100 Subject: [PATCH] [FIX] runtime/fibers: render a parent when self is in error Have a Component A which instantiate another one (B) which also have some Children (C). Have B be the one to handle the errors from the Children via the onError hook. This hook should call a props coming from A. The real error handler is on A then, passed to B via props. This handler naturally sets a flag on A, and triggers a render. Before this commit, the mounting of A never resolved as the fiber that was recycled in A was still part of a root one which was flagged as being in error. After this commit, the A component mounts correctly. Closes #1298 --- src/runtime/fibers.ts | 2 +- .../__snapshots__/error_handling.test.ts.snap | 125 +++++++++++++++++ tests/components/error_handling.test.ts | 126 ++++++++++++++++++ tests/components/t_call.test.ts | 2 +- 4 files changed, 253 insertions(+), 2 deletions(-) diff --git a/src/runtime/fibers.ts b/src/runtime/fibers.ts index 9b33373f..29351b7a 100644 --- a/src/runtime/fibers.ts +++ b/src/runtime/fibers.ts @@ -25,7 +25,7 @@ export function makeRootFiber(node: ComponentNode): Fiber { current.children = []; current.childrenMap = {}; current.bdom = null; - if (fibersInError.has(current)) { + if (fibersInError.has(root)) { fibersInError.delete(current); fibersInError.delete(root); current.appliedToDom = false; diff --git a/tests/components/__snapshots__/error_handling.test.ts.snap b/tests/components/__snapshots__/error_handling.test.ts.snap index ee7453ff..9b2592be 100644 --- a/tests/components/__snapshots__/error_handling.test.ts.snap +++ b/tests/components/__snapshots__/error_handling.test.ts.snap @@ -1195,6 +1195,131 @@ exports[`can catch errors onError in class inheritance is not called if no rethr }" `; +exports[`can catch errors re-render parent when self is in error - 2 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + const comp1 = app.createComponent(\`Classic\`, true, false, false, false); + const comp2 = app.createComponent(\`Classic\`, true, false, false, false); + + return function template(ctx, node, key = \\"\\") { + const b2 = comp1({hasBoom: true,state: ctx['reactive']}, key + \`__1\`, node, this, null); + const b3 = comp2({hasBoom: false,state: ctx['reactive']}, key + \`__2\`, node, this, null); + return multi([b2, b3]); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error - 2 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + const comp1 = app.createComponent(\`BoomWrapper\`, true, false, false, false); + + let block3 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let b2,b3; + if (ctx['props'].hasBoom) { + b2 = comp1({state: ctx['props'].state}, key + \`__1\`, node, this, null); + } else { + let txt1 = ctx['props'].state.safeTree; + b3 = block3([txt1]); + } + return multi([b2, b3]); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error - 2 3`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + const comp1 = app.createComponent(\`Boom\`, true, false, false, true); + + return function template(ctx, node, key = \\"\\") { + let b2,b3; + if (ctx['state'].errorTree==='error') { + b2 = comp1({}, key + \`__1\`, node, this, null); + } else { + b3 = text(ctx['state'].errorTree); + } + return multi([b2, b3]); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error - 2 4`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + const comp1 = app.createComponent(\`Classic\`, true, false, false, true); + + return function template(ctx, node, key = \\"\\") { + return comp1({}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + let { bind } = helpers; + const comp1 = app.createComponent(\`BoomWrapper\`, true, false, false, false); + + let block3 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let b2,b3; + if (!ctx['inError']) { + b2 = comp1({onError: bind(this, ctx['onErrorAsProps'])}, key + \`__1\`, node, this, null); + } else { + b3 = block3(); + } + return multi([b2, b3]); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error 3`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + const comp1 = app.createComponent(\`Boom\`, true, false, false, true); + + return function template(ctx, node, key = \\"\\") { + return comp1({}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`can catch errors re-render parent when self is in error 4`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + exports[`errors and promises a rendering error in a sub component will reject the mount promise 1`] = ` "function anonymous(app, bdom, helpers ) { diff --git a/tests/components/error_handling.test.ts b/tests/components/error_handling.test.ts index e78230e3..276a864a 100644 --- a/tests/components/error_handling.test.ts +++ b/tests/components/error_handling.test.ts @@ -9,6 +9,7 @@ import { onRendered, onWillUnmount, useState, + reactive, xml, } from "../../src/index"; import { @@ -785,6 +786,131 @@ describe("can catch errors", () => { expect(mockConsoleWarn).toBeCalledTimes(0); }); + test("re-render parent when self is in error", async () => { + class Boom extends Component { + static template = xml`
`; + setup() { + onWillStart(() => { + throw new Error("Boom Error"); + }); + } + } + + const steps: string[] = []; + class BoomWrapper extends Component { + static template = xml``; + static components = { Boom }; + setup() { + onError(() => { + steps.push("onError in child"); + this.props.onError(); + }); + } + } + + class Classic extends Component { + static template = xml`
`; + static components = { BoomWrapper }; + inError: Boolean = false; + setup() { + onMounted(() => { + steps.push("mounted"); + }); + } + onErrorAsProps() { + this.inError = true; + this.render(true); + } + } + + class App extends Component { + static template = xml``; + static components = { Classic }; + } + + await mount(App, fixture); + expect(steps).toEqual(["onError in child", "mounted"]); + }); + + test("re-render parent when self is in error - 2", async () => { + class Boom extends Component { + static template = xml`
`; + setup() { + onWillStart(() => { + throw new Error("Boom Error"); + }); + } + } + + const steps: string[] = []; + class BoomWrapper extends Component { + static template = xml``; + static components = { Boom }; + state: any; + setup() { + this.state = useState(this.props.state); + onError(() => { + steps.push("onError"); + this.state.onError(); + }); + + onWillRender(() => { + steps.push(`BoomWrapper willRender`); + }); + } + } + + let classicId = 0; + class Classic extends Component { + static template = xml` + +
+ `; + static components = { BoomWrapper }; + id: Number = 0; + state: any; + inError: Boolean = false; + setup() { + this.id = classicId++; + onMounted(() => { + steps.push(`mounted ${this.id}`); + }); + onWillRender(() => { + steps.push(`Classic willRender ${this.id}`); + }); + } + } + + class App extends Component { + reactive: any; + setup() { + this.reactive = reactive({ + errorTree: "error", + safeTree: "safe", + onError() { + this.safeTree = "safe2"; + this.errorTree = "errorHandled"; + }, + }); + } + static template = xml``; + static components = { Classic }; + } + + await mount(App, fixture); + expect(steps).toEqual([ + "Classic willRender 0", + "Classic willRender 1", + "BoomWrapper willRender", + "onError", + "Classic willRender 1", + "BoomWrapper willRender", + "mounted 1", + "mounted 0", + ]); + expect(fixture.innerHTML).toBe("errorHandled
safe2
"); + }); + test("can catch an error in the willStart call", async () => { class ErrorComponent extends Component { static template = xml`
Some text
`; diff --git a/tests/components/t_call.test.ts b/tests/components/t_call.test.ts index 83b1ec1a..b83f4677 100644 --- a/tests/components/t_call.test.ts +++ b/tests/components/t_call.test.ts @@ -316,7 +316,7 @@ describe("t-call", () => { expect(fixture.innerHTML).toBe("childaaronchildlucas"); }); - test.only("t-call-context: ComponentNode is not looked up in the context", async () => { + test("t-call-context: ComponentNode is not looked up in the context", async () => { let child: any; class Child extends Component { static template = xml``;