Merge pull request #526 from jviide/inline-cycle-errors

refactor: inline cycle errors & use a shared .peek() implementation for signals and computeds

packages/core/src/index.ts

@@ -1,11 +1,10 @@
-function cycleDetected(): never {
-	throw new Error("Cycle detected");
-}
 function mutationDetected(): never {
 	throw new Error("Computed cannot have side-effects");
 }
 
-const identifier = Symbol.for("preact-signals");
+// An named symbol/brand for detecting Signal instances even when they weren't
+// created using the same signals library version.
+const BRAND_SYMBOL = Symbol.for("preact-signals");
 
 // Flags for Computed and Effect.
 const RUNNING = 1 << 0;
@@ -237,7 +236,7 @@ declare class Signal<T = any> {
 
 	peek(): T;
 
-	brand: typeof identifier;
+	brand: typeof BRAND_SYMBOL;
 
 	get value(): T;
 	set value(value: T);
@@ -252,7 +251,7 @@ function Signal(this: Signal, value?: unknown) {
 	this._targets = undefined;
 }
 
-Signal.prototype.brand = identifier;
+Signal.prototype.brand = BRAND_SYMBOL;
 
 Signal.prototype._refresh = function () {
 	return true;
@@ -307,7 +306,13 @@ Signal.prototype.toJSON = function () {
 };
 
 Signal.prototype.peek = function () {
-	return this._value;
+	const prevContext = evalContext;
+	evalContext = undefined;
+	try {
+		return this.value;
+	} finally {
+		evalContext = prevContext;
+	}
 };
 
 Object.defineProperty(Signal.prototype, "value", {
@@ -325,7 +330,7 @@ Object.defineProperty(Signal.prototype, "value", {
 
 		if (value !== this._value) {
 			if (batchIteration > 100) {
-				cycleDetected();
+				throw new Error("Cycle detected");
 			}
 
 			this._value = value;
@@ -591,20 +596,10 @@ Computed.prototype._notify = function () {
 	}
 };
 
-Computed.prototype.peek = function () {
-	if (!this._refresh()) {
-		cycleDetected();
-	}
-	if (this._flags & HAS_ERROR) {
-		throw this._value;
-	}
-	return this._value;
-};
-
 Object.defineProperty(Computed.prototype, "value", {
 	get() {
 		if (this._flags & RUNNING) {
-			cycleDetected();
+			throw new Error("Cycle detected");
 		}
 		const node = addDependency(this);
 		this._refresh();
@@ -719,7 +714,7 @@ Effect.prototype._callback = function () {
 
 Effect.prototype._start = function () {
 	if (this._flags & RUNNING) {
-		cycleDetected();
+		throw new Error("Cycle detected");
 	}
 	this._flags |= RUNNING;
 	this._flags &= ~DISPOSED;

packages/core/test/signal.test.tsx

@@ -1175,6 +1175,21 @@ describe("computed()", () => {
 			expect(c.peek()).equal(1);
 		});
 
+		it("should throw when evaluation throws", () => {
+			const c = computed(() => {
+				throw Error("test");
+			});
+			expect(() => c.peek()).to.throw("test");
+		});
+
+		it("should throw when previous evaluation threw and dependencies haven't changed", () => {
+			const c = computed(() => {
+				throw Error("test");
+			});
+			expect(() => c.value).to.throw("test");
+			expect(() => c.peek()).to.throw("test");
+		});
+
 		it("should refresh value if stale", () => {
 			const a = signal(1);
 			const b = computed(() => a.value);