diff --git a/src/hooks.ts b/src/hooks.ts index e24c7566..231f3ff5 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -57,7 +57,6 @@ export function useChildSubEnv(envExtension: Env) { // useEffect // ----------------------------------------------------------------------------- -const NO_OP = () => {}; /** * @param {...any} dependencies the dependencies computed by computeDependencies * @returns {void|(()=>void)} a cleanup function that reverses the side @@ -78,11 +77,11 @@ type Effect = (...dependencies: any[]) => void | (() => void); * NaN !== NaN, which will cause the effect to rerun on every patch. */ export function useEffect(effect: Effect, computeDependencies: () => any[] = () => [NaN]) { - let cleanup: () => void; + let cleanup: (() => void) | void; let dependencies: any[]; onMounted(() => { dependencies = computeDependencies(); - cleanup = effect(...dependencies) || NO_OP; + cleanup = effect(...dependencies); }); onPatched(() => { @@ -90,12 +89,14 @@ export function useEffect(effect: Effect, computeDependencies: () => any[] = () const shouldReapply = newDeps.some((val, i) => val !== dependencies[i]); if (shouldReapply) { dependencies = newDeps; - cleanup(); - cleanup = effect(...dependencies) || NO_OP; + if (cleanup) { + cleanup(); + } + cleanup = effect(...dependencies); } }); - onWillUnmount(() => cleanup()); + onWillUnmount(() => cleanup && cleanup()); } // ----------------------------------------------------------------------------- diff --git a/tests/components/__snapshots__/hooks.test.ts.snap b/tests/components/__snapshots__/hooks.test.ts.snap index 7c98c2d7..2a8c4763 100644 --- a/tests/components/__snapshots__/hooks.test.ts.snap +++ b/tests/components/__snapshots__/hooks.test.ts.snap @@ -301,6 +301,19 @@ exports[`hooks useEffect hook effect with empty dependency list never reruns 1`] }" `; +exports[`hooks useEffect hook properly behaves when the effect function throws 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + exports[`hooks useExternalListener 1`] = ` "function anonymous(bdom, helpers ) { diff --git a/tests/components/hooks.test.ts b/tests/components/hooks.test.ts index 9c6cbd56..56a0ad58 100644 --- a/tests/components/hooks.test.ts +++ b/tests/components/hooks.test.ts @@ -632,5 +632,35 @@ describe("hooks", () => { "cleaning up for 1", ]); }); + + test("properly behaves when the effect function throws", async () => { + let originalconsoleError = console.error; + let originalconsoleWarn = console.warn; + console.error = jest.fn(() => {}); + console.warn = jest.fn(() => {}); + class MyComponent extends Component { + static template = xml``; + setup() { + useEffect( + () => { + throw new Error("Intentional error"); + }, + () => [] + ); + } + } + + try { + await mount(MyComponent, fixture); + } catch (e: any) { + expect(e.message).toBe("Intentional error"); + } + // no console.error because the error has been caught in this test + expect(console.error).toHaveBeenCalledTimes(0); + console.error = originalconsoleError; + // 1 console.warn because app is destroyed + expect(console.warn).toHaveBeenCalledTimes(1); + console.warn = originalconsoleWarn; + }); }); });