Disallow side-effects in computed (#231)

* add test case for computed with side-effects

* disallow side-effects within computed

* add changeset

* use instanceof instead of bit flags

* update test in accessing computed within effects

---------

Co-authored-by: Jovi De Croock <[email protected]>

Author: eddyw

.changeset/happy-pants-rhyme.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-core": minor
+---
+
+Disallow side-effects in computed

packages/core/src/index.ts

@@ -1,6 +1,9 @@
 function cycleDetected(): never {
 	throw new Error("Cycle detected");
 }
+function mutationDetected(): never {
+	throw new Error("Computed cannot have side-effects");
+}
 
 // Flags for Computed and Effect.
 const RUNNING = 1 << 0;
@@ -300,7 +303,11 @@ Object.defineProperty(Signal.prototype, "value", {
 		}
 		return this._value;
 	},
-	set(value) {
+	set(this: Signal, value) {
+		if (evalContext instanceof Computed) {
+			mutationDetected();
+		}
+
 		if (value !== this._value) {
 			if (batchIteration > 100) {
 				cycleDetected();
@@ -738,4 +745,4 @@ function effect(compute: () => void | (() => void)): () => void {
 	return effect._dispose.bind(effect);
 }
 
-export { signal, computed, effect, batch, Signal, ReadonlySignal };
+export { signal, computed, effect, batch, Signal, type ReadonlySignal };

packages/core/test/signal.test.tsx

@@ -559,7 +559,7 @@ describe("effect()", () => {
 		expect(fn).to.throw(/Cycle detected/);
 	});
 
-	it("should throw on indirect cycles", () => {
+	it("should throw if a computed tries to set a signal's value", () => {
 		const a = signal(0);
 		let i = 0;
 
@@ -578,7 +578,7 @@ describe("effect()", () => {
 				c.value;
 			});
 
-		expect(fn).to.throw(/Cycle detected/);
+		expect(fn).to.throw(/Computed cannot have side-effects/);
 	});
 
 	it("should allow disposing the effect multiple times", () => {
@@ -862,16 +862,13 @@ describe("computed()", () => {
 		expect(spy).to.be.calledOnce;
 	});
 
-	it("should recompute if a dependency changes during computation after becoming a dependency", () => {
-		const a = signal(0);
-		const spy = sinon.spy(() => {
-			a.value++;
-		});
-		const c = computed(spy);
-		c.value;
-		expect(spy).to.be.calledOnce;
-		c.value;
-		expect(spy).to.be.calledTwice;
+	it("should disallow setting signal's value", () => {
+		const v: number = 123;
+		const a: Signal = signal(v);
+		const c: Signal = computed(() => a.value++);
+
+		expect(() => c.value).to.throw(/Computed cannot have side-effects/);
+		expect(a.value).to.equal(v);
 	});
 
 	it("should detect simple dependency cycles", () => {