fix: general error handling improvements

README.md

@@ -632,6 +632,13 @@ found [here](./bench/layers.js).
 Each column represents how deep computations were layered. The average time taken to update the
 computation out of a 100 runs is used for each library.
 
+> ❗ Do note that only Maverick and Solid JS are feature complete below which includes nested
+> effects, arbritrary node disposal, context, and error handling.
+
+> ❗ Nearly all computations in a real world app are going to be less than 10 layers deep, so
+> only the first column really matters. What this benchmark is really showing is how notification
+> propagation scales with computation depth.
+
 #### Sync
 
 <img src="./bench/layers.png" alt="Layers sync benchmark table" width="350px" />

src/observables.ts

@@ -22,6 +22,7 @@ export type ObservableOptions<T> = {
   id?: string;
   dirty?: (prev: T, next: T) => boolean;
 };
+export type ComputedOptions<T> = ObservableOptions<T>;
 
 export type ObservableValue<T> = T extends Observable<infer R> ? R : T;
 
@@ -156,9 +157,10 @@ export function isObservable<T>(fn: MaybeObservable<T>): fn is Observable<T> {
  *
  * @see {@link https://github.com/maverick-js/observables#computed}
  */
-export function computed<T>(fn: () => T, options?: ObservableOptions<T>): Observable<T> {
+export function computed<T>(fn: () => T, options?: ComputedOptions<T>): Observable<T> {
   let currentValue,
-    init = false;
+    init = false,
+    effect = (options as any)?._e;
 
   const isDirty = options?.dirty ?? notEqual;
 
@@ -186,20 +188,19 @@ export function computed<T>(fn: () => T, options?: ObservableOptions<T>): Observ
         }
 
         const nextValue = compute($computed, fn);
-        $computed[DIRTY] = false;
-
-        if (!init) {
-          currentValue = nextValue;
-          init = true;
-        } else if (isDirty(currentValue, nextValue)) {
+        if (isDirty(currentValue, nextValue)) {
           currentValue = nextValue;
           dirtyNode($computed);
         }
-
-        if (!$computed[OBSERVING]?.size) dispose($computed);
       } catch (error) {
+        if (!init && !effect) throw error;
         handleError($computed, error);
+        return currentValue;
       }
+
+      init = true;
+      $computed[DIRTY] = false;
+      if (!$computed[OBSERVING]?.size) dispose($computed);
     }
 
     return currentValue;
@@ -231,10 +232,15 @@ export function isObserved(): boolean {
  */
 export function effect(fn: Effect, options?: { id?: string }): StopEffect {
   const $effect = computed(
-    () => onDispose(fn()),
-    __DEV__ ? { id: options?.id ?? 'effect' } : undefined,
+    () => {
+      const result = fn();
+      result && onDispose(result);
+    },
+    {
+      id: __DEV__ ? options?.id ?? 'effect' : undefined,
+      _e: true,
+    } as ComputedOptions<unknown>,
   );
-
   $effect();
   return () => dispose($effect);
 }
@@ -295,22 +301,22 @@ export function getScheduler(): Scheduler {
 }
 
 /**
- * Scopes the given function to the given parent scope so context and error handling continue to
+ * Scopes the given function to the current parent scope so context and error handling continue to
  * work as expected. Generally this should be called on non-observable functions. A scoped
  * function will return `undefined` if an error is thrown.
  *
  * This is more compute and memory efficient than the alternative `effect(() => peek(callback))`
  * because it doesn't require creating and tracking a `computed` observable.
  */
-export function scope<T>(fn: () => T, scope = getScope()!): () => T | undefined {
-  adopt(fn, scope);
+export function scope<T>(fn: () => T): () => T | undefined {
+  adopt(fn);
   return () => {
     try {
-      return compute(scope, fn, currentObserver);
+      return compute(fn[SCOPE], fn, currentObserver);
     } catch (error) {
       handleError(fn, error);
-      return; // make TS happy
     }
+    return; // make TS happy -_-
   };
 }
 
@@ -405,21 +411,25 @@ export function dispose(fn: () => void) {
   fn[DISPOSED] = true;
 }
 
-function compute<T>(scope: () => void, node: () => T, observer: () => void = scope): T {
+function compute<T>(
+  scope: (() => void) | undefined,
+  node: () => T,
+  observer: (() => void) | undefined = scope,
+): T {
   const prevScope = currentScope;
   const prevObserver = currentObserver;
 
   currentScope = scope;
   currentObserver = observer;
-  if (__DEV__) computeStack.push(scope);
+  if (__DEV__ && scope) computeStack.push(scope);
 
-  const nextValue = node();
-
-  currentScope = prevScope;
-  currentObserver = prevObserver;
-  if (__DEV__) computeStack.pop();
-
-  return nextValue;
+  try {
+    return node();
+  } finally {
+    currentScope = prevScope;
+    currentObserver = prevObserver;
+    if (__DEV__ && scope) computeStack.pop();
+  }
 }
 
 function lookup(node: Node | undefined, key: string | symbol): any {
@@ -433,10 +443,10 @@ function lookup(node: Node | undefined, key: string | symbol): any {
   }
 }
 
-function adopt(node: Node, scope = currentScope) {
-  if (!scope) return;
-  node[SCOPE] = scope;
-  (scope[CHILDREN] ??= new Set()).add(node);
+function adopt(node: Node) {
+  if (!currentScope) return;
+  node[SCOPE] = currentScope;
+  (currentScope[CHILDREN] ??= new Set()).add(node);
 }
 
 function observe(observable: Node, observer: Node) {
@@ -468,7 +478,8 @@ function handleError(node: () => void, error: unknown) {
   const handlers = lookup(node, ERROR);
   if (!handlers) throw error;
   try {
-    for (const handler of handlers) handler(error);
+    const coercedError = error instanceof Error ? error : Error(JSON.stringify(error));
+    for (const handler of handlers) handler(coercedError);
   } catch (error) {
     handleError(node[SCOPE], error);
   }

tests/dispose.test.ts

@@ -1,4 +1,4 @@
-import { computed, observable, tick, dispose, effect, root, getScope } from '../src';
+import { computed, observable, tick, dispose, effect, root, getScope, onError } from '../src';
 import { CHILDREN, DISPOSED, OBSERVED_BY } from '../src/symbols';
 
 afterEach(() => tick());
@@ -35,16 +35,32 @@ it('shoud remove observable from parent children set', () => {
 });
 
 it('should auto-dispose computed if not observing anything', () => {
-  const $a = computed(() => null, { id: '$a' });
+  const $a = computed(() => null);
   $a();
 
-  const $b = computed(() => $a(), { id: '$b' });
+  const $b = computed(() => $a());
   $b();
 
   expect($a[DISPOSED]).toBeTruthy();
   expect($b[DISPOSED]).toBeTruthy();
 });
 
+it('should _not_ auto-dispose effect if error is thrown', async () => {
+  const $a = observable(0);
+
+  let shouldThrow = true,
+    effectScope;
+
+  effect(() => {
+    onError(() => {});
+    effectScope = getScope();
+    if (shouldThrow) throw Error();
+    $a();
+  });
+
+  expect(effectScope[DISPOSED]).toBeFalsy();
+});
+
 it('should stop observing effect', async () => {
   const $a = observable(0);
 

tests/scope.test.ts

@@ -1,14 +1,4 @@
-import {
-  getContext,
-  root,
-  setContext,
-  scope,
-  onError,
-  getScope,
-  observable,
-  effect,
-  tick,
-} from '../src';
+import { getContext, root, setContext, scope, onError, observable, effect, tick } from '../src';
 
 it('should scope function to current scope', () => {
   let callback!: () => void;
@@ -22,21 +12,6 @@ it('should scope function to current scope', () => {
   callback();
 });
 
-it('should scope function to given scope', () => {
-  let callback!: () => void;
-
-  let $root;
-  root(() => {
-    setContext('id', 10);
-    $root = getScope();
-  });
-
-  const fn = () => expect(getContext('id')).toBe(10);
-  callback = scope(fn, $root);
-
-  callback();
-});
-
 it('should return value', () => {
   let callback!: () => void;