From bd98d4d0d06e19a42006c1ee1e4882a0cccafc32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 31 Jan 2022 15:03:52 +0100 Subject: [PATCH] [IMP] component: disallow calling hooks outside of setup (and constructor) Doing so could cause strange and difficult bugs --- src/component/component_node.ts | 6 ++++- src/component/lifecycle_hooks.ts | 20 ++++++++--------- src/hooks.ts | 10 ++++----- src/reactivity.ts | 2 +- .../__snapshots__/error_handling.test.ts.snap | 11 ++++++++++ tests/components/error_handling.test.ts | 22 +++++++++++++++++++ 6 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/component/component_node.ts b/src/component/component_node.ts index 6e1da229..d1a188b2 100644 --- a/src/component/component_node.ts +++ b/src/component/component_node.ts @@ -15,7 +15,10 @@ import { STATUS } from "./status"; let currentNode: ComponentNode | null = null; -export function getCurrent(): ComponentNode | null { +export function getCurrent(): ComponentNode { + if (!currentNode) { + throw new Error("No active component (a hook function should only be called in 'setup')"); + } return currentNode; } @@ -108,6 +111,7 @@ export class ComponentNode

implements VNode Promise | void | any) { - const node = getCurrent()!; + const node = getCurrent(); node.willStart.push(fn.bind(node.component)); } export function onWillUpdateProps(fn: (nextProps: any) => Promise | void | any) { - const node = getCurrent()!; + const node = getCurrent(); node.willUpdateProps.push(fn.bind(node.component)); } export function onMounted(fn: () => void | any) { - const node = getCurrent()!; + const node = getCurrent(); node.mounted.push(fn.bind(node.component)); } export function onWillPatch(fn: () => Promise | any | void) { - const node = getCurrent()!; + const node = getCurrent(); node.willPatch.unshift(fn.bind(node.component)); } export function onPatched(fn: () => void | any) { - const node = getCurrent()!; + const node = getCurrent(); node.patched.push(fn.bind(node.component)); } export function onWillUnmount(fn: () => Promise | void | any) { - const node = getCurrent()!; + const node = getCurrent(); node.willUnmount.unshift(fn.bind(node.component)); } export function onWillDestroy(fn: () => Promise | void | any) { - const node = getCurrent()!; + const node = getCurrent(); node.willDestroy.push(fn.bind(node.component)); } export function onWillRender(fn: () => void | any) { - const node = getCurrent()!; + const node = getCurrent(); const renderFn = node.renderFn; node.renderFn = () => { fn.call(node.component); @@ -50,7 +50,7 @@ export function onWillRender(fn: () => void | any) { } export function onRendered(fn: () => void | any) { - const node = getCurrent()!; + const node = getCurrent(); const renderFn = node.renderFn; node.renderFn = () => { const result = renderFn(); @@ -61,7 +61,7 @@ export function onRendered(fn: () => void | any) { type OnErrorCallback = (error: any) => void | any; export function onError(callback: OnErrorCallback) { - const node = getCurrent()!; + const node = getCurrent(); let handlers = nodeErrorHandlers.get(node); if (!handlers) { handlers = []; diff --git a/src/hooks.ts b/src/hooks.ts index 3eb865b7..e24c7566 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -11,7 +11,7 @@ import { onMounted, onPatched, onWillUnmount } from "./component/lifecycle_hooks * html node or component. */ export function useRef(name: string): { el: T | null } { - const node = getCurrent()!; + const node = getCurrent(); const refs = node.refs; return { get el(): T | null { @@ -29,7 +29,7 @@ export function useRef(name: string): { el: * need a reference to the env of the component calling them. */ export function useEnv(): E { - return getCurrent()!.component.env as any; + return getCurrent().component.env as any; } function extendEnv(currentEnv: Object, extension: Object): Object { @@ -44,13 +44,13 @@ function extendEnv(currentEnv: Object, extension: Object): Object { * constructor method. */ export function useSubEnv(envExtension: Env) { - const node = getCurrent()!; + const node = getCurrent(); node.component.env = extendEnv(node.component.env as any, envExtension); useChildSubEnv(envExtension); } export function useChildSubEnv(envExtension: Env) { - const node = getCurrent()!; + const node = getCurrent(); node.childEnv = extendEnv(node.childEnv, envExtension); } // ----------------------------------------------------------------------------- @@ -121,7 +121,7 @@ export function useExternalListener( handler: EventListener, eventParams?: AddEventListenerOptions ) { - const node = getCurrent()!; + const node = getCurrent(); const boundHandler = handler.bind(node.component); onMounted(() => target.addEventListener(eventName, boundHandler, eventParams)); onWillUnmount(() => target.removeEventListener(eventName, boundHandler, eventParams)); diff --git a/src/reactivity.ts b/src/reactivity.ts index ba937c89..40053ca4 100644 --- a/src/reactivity.ts +++ b/src/reactivity.ts @@ -239,7 +239,7 @@ const batchedRenderFunctions = new WeakMap(); * @see reactive */ export function useState(state: T): Reactive | NonReactive { - const node = getCurrent()!; + const node = getCurrent(); if (!batchedRenderFunctions.has(node)) { batchedRenderFunctions.set( node, diff --git a/tests/components/__snapshots__/error_handling.test.ts.snap b/tests/components/__snapshots__/error_handling.test.ts.snap index 75f4f7f1..cc50f71b 100644 --- a/tests/components/__snapshots__/error_handling.test.ts.snap +++ b/tests/components/__snapshots__/error_handling.test.ts.snap @@ -85,6 +85,17 @@ exports[`basics simple catchError 2`] = ` }" `; +exports[`can catch errors calling a hook outside setup should crash 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component, comment } = bdom; + + return function template(ctx, node, key = \\"\\") { + return text(ctx['state'].value); + } +}" +`; + exports[`can catch errors can catch an error in a component render function 1`] = ` "function anonymous(bdom, helpers ) { diff --git a/tests/components/error_handling.test.ts b/tests/components/error_handling.test.ts index 3033cd6e..078a96a0 100644 --- a/tests/components/error_handling.test.ts +++ b/tests/components/error_handling.test.ts @@ -374,6 +374,28 @@ describe("can catch errors", () => { expect(mockConsoleWarn).toBeCalledTimes(0); }); + test("calling a hook outside setup should crash", async () => { + class Root extends Component { + static template = xml``; + state = useState({ value: 1 }); + + setup() { + onWillStart(() => { + this.state = useState({ value: 2 }); + }); + } + } + let e: any = null; + try { + await mount(Root, fixture); + } catch (error) { + e = error; + } + expect(e.message).toBe( + "No active component (a hook function should only be called in 'setup')" + ); + }); + test("can catch an error in the initial call of a component render function (parent mounted)", async () => { class ErrorComponent extends Component { static template = xml`

hey
`;