From 5e24abb89c1f27422c7ecd511400c02cc52ff078 Mon Sep 17 00:00:00 2001
From: Seven Du <5564821+medz@users.noreply.github.com>
Date: Fri, 15 May 2026 22:17:21 +0800
Subject: [PATCH] fix: dispose failed effect setup

---
 src/index.ts                  | 18 +++++++---
 tests/effect-teardown.spec.ts | 67 +++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 4 deletions(-)
 create mode 100644 tests/effect-teardown.spec.ts

diff --git a/src/index.ts b/src/index.ts
index 800fe53..ee39d59 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -99,6 +99,10 @@ export function setActiveSub(sub?: ReactiveNode) {
 	return prevSub;
 }
 
+function shouldTrack(sub: ReactiveNode): boolean {
+	return !!(sub.flags & (ReactiveFlags.Mutable | ReactiveFlags.Watching));
+}
+
 export function getBatchDepth(): number {
 	return batchDepth;
 }
@@ -173,13 +177,16 @@ export function effect(fn: () => void | (() => void)): () => void {
 		flags: ReactiveFlags.Watching | ReactiveFlags.RecursedCheck,
 	};
 	const prevSub = setActiveSub(e);
-	if (prevSub !== undefined) {
+	if (prevSub !== undefined && shouldTrack(prevSub)) {
 		link(e, prevSub, 0);
 		prevSub.flags |= HasChildEffect;
 	}
 	try {
 		++runDepth;
 		e.cleanup = e.fn();
+	} catch (error) {
+		effectOper.call(e);
+		throw error;
 	} finally {
 		--runDepth;
 		activeSub = prevSub;
@@ -197,12 +204,15 @@ export function effectScope(fn: () => void): () => void {
 		flags: ReactiveFlags.Mutable,
 	};
 	const prevSub = setActiveSub(e);
-	if (prevSub !== undefined) {
+	if (prevSub !== undefined && shouldTrack(prevSub)) {
 		link(e, prevSub, 0);
 		prevSub.flags |= HasChildEffect;
 	}
 	try {
 		fn();
+	} catch (error) {
+		effectScopeOper.call(e);
+		throw error;
 	} finally {
 		activeSub = prevSub;
 	}
@@ -359,7 +369,7 @@ function computedOper<T>(this: ComputedNode<T>): T {
 		}
 	}
 	const sub = activeSub;
-	if (sub !== undefined) {
+	if (sub !== undefined && shouldTrack(sub)) {
 		link(this, sub, cycle);
 	}
 	return this.value!;
@@ -387,7 +397,7 @@ function signalOper<T>(this: SignalNode<T>, ...value: [T]): T | void {
 			}
 		}
 		const sub = activeSub;
-		if (sub !== undefined) {
+		if (sub !== undefined && shouldTrack(sub)) {
 			link(this, sub, cycle);
 		}
 		return this.currentValue;
diff --git a/tests/effect-teardown.spec.ts b/tests/effect-teardown.spec.ts
new file mode 100644
index 0000000..2820d84
--- /dev/null
+++ b/tests/effect-teardown.spec.ts
@@ -0,0 +1,67 @@
+import { expect, test } from 'vitest';
+import { effect, effectScope, getActiveSub, signal } from '../src';
+import { ReactiveFlags, type ReactiveNode } from '../src/system';
+
+test('stopped effect does not subscribe to signals read later in the same run', () => {
+	const rerun = signal(0);
+	const readAfterStop = signal(0);
+	let stop!: () => void;
+	let node: ReactiveNode | undefined;
+	let stopDuringRun = false;
+	let runs = 0;
+
+	stop = effect(() => {
+		node ??= getActiveSub();
+		runs++;
+		rerun();
+		if (stopDuringRun) {
+			stop();
+			readAfterStop();
+		}
+	});
+
+	expect(runs).toBe(1);
+
+	stopDuringRun = true;
+	rerun(1);
+
+	expect(runs).toBe(2);
+	expect(node!.flags).toBe(ReactiveFlags.None);
+	expect(node!.deps).toBeUndefined();
+});
+
+test('failed effect setup does not leave a live subscription behind', () => {
+	const source = signal(0);
+	let runs = 0;
+
+	expect(() =>
+		effect(() => {
+			runs++;
+			source();
+			throw new Error('setup failed');
+		})
+	).toThrow('setup failed');
+
+	expect(runs).toBe(1);
+	expect(() => source(1)).not.toThrow();
+	expect(runs).toBe(1);
+});
+
+test('failed effect scope setup disposes child effects created before throw', () => {
+	const source = signal(0);
+	let childRuns = 0;
+
+	expect(() =>
+		effectScope(() => {
+			effect(() => {
+				childRuns++;
+				source();
+			});
+			throw new Error('scope setup failed');
+		})
+	).toThrow('scope setup failed');
+
+	expect(childRuns).toBe(1);
+	source(1);
+	expect(childRuns).toBe(1);
+});