fix: queuing effects in an array

close #48

Author: johnsoncodehk

src/system.ts

@@ -16,7 +16,6 @@ export interface Link {
 	// Reused to link the previous stack in propagate
 	prevSub: Link | undefined;
 	nextSub: Link | undefined;
-	// Reused to link the notify effect in queuedEffects
 	nextDep: Link | undefined;
 }
 
@@ -60,8 +59,7 @@ export function createReactiveSystem({
 	 */
 	notifyEffect(effect: Subscriber): boolean;
 }) {
-	let queuedEffects: Subscriber | undefined;
-	let queuedEffectsTail: Subscriber | undefined;
+	const queuedEffects: Subscriber[] = [];
 
 	return {
 		/**
@@ -146,22 +144,12 @@ export function createReactiveSystem({
 						continue;
 					}
 					if (subFlags & SubscriberFlags.Effect) {
-						if (queuedEffectsTail !== undefined) {
-							queuedEffectsTail.depsTail!.nextDep = sub.deps;
-						} else {
-							queuedEffects = sub;
-						}
-						queuedEffectsTail = sub;
+						queuedEffects.push(sub);
 					}
 				} else if (!(subFlags & (SubscriberFlags.Tracking | targetFlag))) {
 					sub.flags = subFlags | targetFlag | SubscriberFlags.Notified;
 					if ((subFlags & (SubscriberFlags.Effect | SubscriberFlags.Notified)) === SubscriberFlags.Effect) {
-						if (queuedEffectsTail !== undefined) {
-							queuedEffectsTail.depsTail!.nextDep = sub.deps;
-						} else {
-							queuedEffects = sub;
-						}
-						queuedEffectsTail = sub;
+						queuedEffects.push(sub);
 					}
 				} else if (
 					!(subFlags & targetFlag)
@@ -313,17 +301,8 @@ export function createReactiveSystem({
 		 * notifications may be triggered until fully handled.
 		 */
 		processEffectNotifications(): void {
-			while (queuedEffects !== undefined) {
-				const effect = queuedEffects;
-				const depsTail = effect.depsTail!;
-				const queuedNext = depsTail.nextDep;
-				if (queuedNext !== undefined) {
-					depsTail.nextDep = undefined;
-					queuedEffects = queuedNext.sub;
-				} else {
-					queuedEffects = undefined;
-					queuedEffectsTail = undefined;
-				}
+			while (queuedEffects.length) {
+				const effect = queuedEffects.shift()!;
 				if (!notifyEffect(effect)) {
 					effect.flags &= ~SubscriberFlags.Notified;
 				}
@@ -474,12 +453,7 @@ export function createReactiveSystem({
 			if ((subFlags & (SubscriberFlags.PendingComputed | SubscriberFlags.Dirty)) === SubscriberFlags.PendingComputed) {
 				sub.flags = subFlags | SubscriberFlags.Dirty | SubscriberFlags.Notified;
 				if ((subFlags & (SubscriberFlags.Effect | SubscriberFlags.Notified)) === SubscriberFlags.Effect) {
-					if (queuedEffectsTail !== undefined) {
-						queuedEffectsTail.depsTail!.nextDep = sub.deps;
-					} else {
-						queuedEffects = sub;
-					}
-					queuedEffectsTail = sub;
+					queuedEffects.push(sub);
 				}
 			}
 			link = link.nextSub!;

tests/issue_48.spec.ts

@@ -0,0 +1,97 @@
+import { expect, test } from 'vitest';
+import { computed, effect, pauseTracking, resumeTracking, signal } from '../src';
+
+test('#43', () => {
+	const source = signal(0);
+	let disposeInner: () => void;
+
+	reaction(
+		() => source(),
+		(val) => {
+			if (val === 1) {
+				disposeInner = reaction(
+					() => source(),
+					() => { }
+				);
+			} else if (val === 2) {
+				disposeInner!();
+			}
+		}
+	);
+
+	source(1);
+	source(2);
+	source(3);
+});
+
+interface ReactionOptions<T = unknown, F extends boolean = boolean> {
+	fireImmediately?: F;
+	equals?: F extends true
+	? (a: T, b: T | undefined) => boolean
+	: (a: T, b: T) => boolean;
+	onError?: (error: unknown) => void;
+	scheduler?: (fn: () => void) => void;
+	once?: boolean;
+}
+
+function reaction<T>(
+	dataFn: () => T,
+	effectFn: (newValue: T, oldValue: T | undefined) => void,
+	options: ReactionOptions<T> = {}
+): () => void {
+	const {
+		scheduler = (fn) => fn(),
+		equals = Object.is,
+		onError,
+		once = false,
+		fireImmediately = false,
+	} = options;
+
+	let prevValue: T | undefined;
+	let version = 0;
+
+	const tracked = computed(() => {
+		try {
+			return dataFn();
+		} catch (error) {
+			untracked(() => onError?.(error));
+			return prevValue!;
+		}
+	});
+
+	const dispose = effect(() => {
+		const current = tracked();
+		if (!fireImmediately && version === 0) {
+			prevValue = current;
+		}
+		version++;
+		if (equals(current, prevValue!)) return;
+		const oldValue = prevValue;
+		prevValue = current;
+		untracked(() =>
+			scheduler(() => {
+				try {
+					effectFn(current, oldValue);
+				} catch (error) {
+					onError?.(error);
+				} finally {
+					if (once) {
+						if (fireImmediately && version > 1) dispose();
+						else if (!fireImmediately && version > 0) dispose();
+					}
+				}
+			})
+		);
+	});
+
+	return dispose;
+}
+
+function untracked<T>(callback: () => T): T {
+	try {
+		pauseTracking();
+		return callback();
+	} finally {
+		resumeTracking();
+	}
+}