From 145bcb3f06c186ae801a259f61b8a0f235656f86 Mon Sep 17 00:00:00 2001
From: Andre Wiggins <459878+andrewiggins@users.noreply.github.com>
Date: Thu, 15 Jun 2023 15:45:33 -0700
Subject: [PATCH] Add some tests for nested effects (#371)

---
 packages/core/test/signal.test.tsx           | 26 ++++++++
 packages/preact/test/index.test.tsx          | 64 ++++++++++++++++++++
 packages/react/test/browser/updates.test.tsx | 64 ++++++++++++++++++++
 3 files changed, 154 insertions(+)

diff --git a/packages/core/test/signal.test.tsx b/packages/core/test/signal.test.tsx
index 00b5a543f..04aa12715 100644
--- a/packages/core/test/signal.test.tsx
+++ b/packages/core/test/signal.test.tsx
@@ -642,6 +642,32 @@ describe("effect()", () => {
 		expect(spy).not.to.be.called;
 	});
 
+	it("should not rerun parent effect if a nested child effect's signal's value changes", () => {
+		const parentSignal = signal(0);
+		const childSignal = signal(0);
+
+		const parentEffect = sinon.spy(() => parentSignal.value);
+		const childEffect = sinon.spy(() => childSignal.value);
+
+		effect(() => {
+			parentEffect();
+			effect(childEffect);
+		});
+
+		expect(parentEffect).to.be.calledOnce;
+		expect(childEffect).to.be.calledOnce;
+
+		childSignal.value = 1;
+
+		expect(parentEffect).to.be.calledOnce;
+		expect(childEffect).to.be.calledTwice;
+
+		parentSignal.value = 1;
+
+		expect(parentEffect).to.be.calledTwice;
+		expect(childEffect).to.be.calledThrice;
+	});
+
 	// Test internal behavior depended on by Preact & React integrations
 	describe("internals", () => {
 		it("should pass in the effect instance in callback's `this`", () => {
diff --git a/packages/preact/test/index.test.tsx b/packages/preact/test/index.test.tsx
index dd98e3a74..212375da1 100644
--- a/packages/preact/test/index.test.tsx
+++ b/packages/preact/test/index.test.tsx
@@ -189,6 +189,70 @@ describe("@preact/signals", () => {
 			rerender();
 			expect(spy).to.be.calledOnce;
 		});
+
+		it("should not subscribe to computed signals only created and not used", () => {
+			const sig = signal(0);
+			const childSpy = sinon.spy();
+			const parentSpy = sinon.spy();
+
+			function Child({ num }: { num: Signal<number> }) {
+				childSpy();
+				return <p>{num.value}</p>;
+			}
+
+			function Parent({ num }: { num: Signal<number> }) {
+				parentSpy();
+				const sig2 = useComputed(() => num.value + 1);
+				return <Child num={sig2} />;
+			}
+
+			render(<Parent num={sig} />, scratch);
+			expect(scratch.innerHTML).to.equal("<p>1</p>");
+			expect(parentSpy).to.be.calledOnce;
+			expect(childSpy).to.be.calledOnce;
+
+			sig.value += 1;
+			rerender();
+			expect(scratch.innerHTML).to.equal("<p>2</p>");
+			expect(parentSpy).to.be.calledOnce;
+			expect(childSpy).to.be.calledTwice;
+		});
+
+		it("should properly subscribe and unsubscribe to conditionally rendered computed signals ", () => {
+			const computedDep = signal(0);
+			const renderComputed = signal(true);
+			const renderSpy = sinon.spy();
+			const computer = sinon.spy(() => computedDep.value + 1);
+
+			function App() {
+				renderSpy();
+				const computed = useComputed(computer);
+				return renderComputed.value ? <p>{computed.value}</p> : null;
+			}
+
+			render(<App />, scratch);
+			expect(scratch.innerHTML).to.equal("<p>1</p>");
+			expect(renderSpy).to.be.calledOnce;
+			expect(computer).to.be.calledOnce;
+
+			computedDep.value += 1;
+			rerender();
+			expect(scratch.innerHTML).to.equal("<p>2</p>");
+			expect(renderSpy).to.be.calledTwice;
+			expect(computer).to.be.calledTwice;
+
+			renderComputed.value = false;
+			rerender();
+			expect(scratch.innerHTML).to.equal("");
+			expect(renderSpy).to.be.calledThrice;
+			expect(computer).to.be.calledTwice;
+
+			computedDep.value += 1;
+			rerender();
+			expect(scratch.innerHTML).to.equal("");
+			expect(renderSpy).to.be.calledThrice; // Should not be called again
+			expect(computer).to.be.calledTwice; // Should not be called again
+		});
 	});
 
 	describe("prop bindings", () => {
diff --git a/packages/react/test/browser/updates.test.tsx b/packages/react/test/browser/updates.test.tsx
index 0e9a0d2cb..23a47f50d 100644
--- a/packages/react/test/browser/updates.test.tsx
+++ b/packages/react/test/browser/updates.test.tsx
@@ -7,6 +7,7 @@ import {
 	useComputed,
 	useSignalEffect,
 	useSignal,
+	Signal,
 } from "@preact/signals-react";
 import {
 	createElement,
@@ -452,6 +453,69 @@ describe("@preact/signals-react updating", () => {
 			});
 			expect(scratch.innerHTML).to.equal("<div>1 1</div>");
 		});
+
+		it("should not subscribe to computed signals only created and not used", async () => {
+			const sig = signal(0);
+			const childSpy = sinon.spy();
+			const parentSpy = sinon.spy();
+
+			function Child({ num }: { num: Signal<number> }) {
+				childSpy();
+				return <p>{num.value}</p>;
+			}
+
+			function Parent({ num }: { num: Signal<number> }) {
+				parentSpy();
+				const sig2 = useComputed(() => num.value + 1);
+				return <Child num={sig2} />;
+			}
+
+			await render(<Parent num={sig} />);
+			expect(scratch.innerHTML).to.equal("<p>1</p>");
+			expect(parentSpy).to.be.calledOnce;
+			expect(childSpy).to.be.calledOnce;
+
+			await act(() => {
+				sig.value += 1;
+			});
+			expect(scratch.innerHTML).to.equal("<p>2</p>");
+			expect(parentSpy).to.be.calledOnce;
+			expect(childSpy).to.be.calledTwice;
+		});
+
+		it("should properly subscribe and unsubscribe to conditionally rendered computed signals ", async () => {
+			const computedDep = signal(0);
+			const renderComputed = signal(true);
+			const renderSpy = sinon.spy();
+
+			function App() {
+				renderSpy();
+				const computed = useComputed(() => computedDep.value + 1);
+				return renderComputed.value ? <p>{computed.value}</p> : null;
+			}
+
+			await render(<App />);
+			expect(scratch.innerHTML).to.equal("<p>1</p>");
+			expect(renderSpy).to.be.calledOnce;
+
+			await act(() => {
+				computedDep.value += 1;
+			});
+			expect(scratch.innerHTML).to.equal("<p>2</p>");
+			expect(renderSpy).to.be.calledTwice;
+
+			await act(() => {
+				renderComputed.value = false;
+			});
+			expect(scratch.innerHTML).to.equal("");
+			expect(renderSpy).to.be.calledThrice;
+
+			await act(() => {
+				computedDep.value += 1;
+			});
+			expect(scratch.innerHTML).to.equal("");
+			expect(renderSpy).to.be.calledThrice; // Should not be called again
+		});
 	});
 
 	describe("useSignal()", () => {