[IMP] component: disallow calling hooks outside of setup

(and constructor)

Doing so could cause strange and difficult bugs

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<P = any, E = any> implements VNode<ComponentNode<P, E
     this.component = new C(props, env, this) as any;
     this.renderFn = app.getTemplate(C.template).bind(this.component, this.component, this);
     this.component.setup();
+    currentNode = null;
   }
 
   mountComponent(target: any, options?: MountOptions) {

src/component/lifecycle_hooks.ts

@@ -6,42 +6,42 @@ import { nodeErrorHandlers } from "./error_handling";
 // -----------------------------------------------------------------------------
 
 export function onWillStart(fn: () => Promise<void> | void | any) {
-  const node = getCurrent()!;
+  const node = getCurrent();
   node.willStart.push(fn.bind(node.component));
 }
 
 export function onWillUpdateProps(fn: (nextProps: any) => Promise<void> | 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<void> | 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> | void | any) {
-  const node = getCurrent()!;
+  const node = getCurrent();
   node.willUnmount.unshift(fn.bind(node.component));
 }
 
 export function onWillDestroy(fn: () => Promise<void> | 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 = [];

src/hooks.ts

@@ -11,7 +11,7 @@ import { onMounted, onPatched, onWillUnmount } from "./component/lifecycle_hooks
  * html node or component.
  */
 export function useRef<T extends HTMLElement = HTMLElement>(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<T extends HTMLElement = HTMLElement>(name: string): { el:
  * need a reference to the env of the component calling them.
  */
 export function useEnv<E extends Env>(): 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));

src/reactivity.ts

@@ -239,7 +239,7 @@ const batchedRenderFunctions = new WeakMap<ComponentNode, Callback>();
  * @see reactive
  */
 export function useState<T extends object>(state: T): Reactive<T> | NonReactive<T> {
-  const node = getCurrent()!;
+  const node = getCurrent();
   if (!batchedRenderFunctions.has(node)) {
     batchedRenderFunctions.set(
       node,

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
 ) {

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`<t t-esc="state.value"/>`;
+      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`<div>hey<t t-esc="state.this.will.crash"/></div>`;