From 54436ab5530e5bcedb763bc3b36f6dd5b0c798e8 Mon Sep 17 00:00:00 2001
From: Johnson Chu <johnsoncodehk@gmail.com>
Date: Sat, 12 Oct 2024 06:41:31 +0800
Subject: [PATCH] fix(effect): re-fix should trigger inner effects in sequence

---
 lib/effect.ts        | 18 +++++++++++++-----
 lib/effectScope.ts   | 15 ++++++++++++---
 lib/system.ts        | 36 ++++++++++++++++++++++++++++++------
 tests/effect.spec.ts |  6 ++++--
 4 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/lib/effect.ts b/lib/effect.ts
index e90484f..9936d27 100644
--- a/lib/effect.ts
+++ b/lib/effect.ts
@@ -30,11 +30,19 @@ export class Effect implements IEffect, Dependency, Subscriber {
 	}
 
 	notify() {
-		if (this.versionOrDirtyLevel === DirtyLevels.MaybeDirty) {
-			Subscriber.resolveMaybeDirty(this);
-		}
-		if (this.versionOrDirtyLevel === DirtyLevels.Dirty) {
-			this.run();
+		const dirtyLevel = this.versionOrDirtyLevel;
+		if (dirtyLevel === DirtyLevels.SideEffectsOnly) {
+			this.versionOrDirtyLevel = DirtyLevels.None;
+			Subscriber.runInnerEffects(this);
+		} else {
+			if (dirtyLevel === DirtyLevels.MaybeDirty) {
+				Subscriber.resolveMaybeDirty(this);
+			}
+			if (this.versionOrDirtyLevel === DirtyLevels.Dirty) {
+				this.run();
+			} else {
+				Subscriber.runInnerEffects(this);
+			}
 		}
 	}
 
diff --git a/lib/effectScope.ts b/lib/effectScope.ts
index 1e82418..1cb70c5 100644
--- a/lib/effectScope.ts
+++ b/lib/effectScope.ts
@@ -1,17 +1,26 @@
-import { DirtyLevels, Subscriber } from './system';
+import { DirtyLevels, IEffect, Subscriber } from './system';
 
 export function effectScope() {
 	return new EffectScope();
 }
 
-export class EffectScope implements Subscriber {
+export class EffectScope implements IEffect, Subscriber {
+	nextNotify = undefined;
+
 	// Subscriber
 	deps = undefined;
 	depsTail = undefined;
-	versionOrDirtyLevel = DirtyLevels.Dirty;
+	versionOrDirtyLevel = DirtyLevels.None;
 
 	notifyLostSubs() { }
 
+	notify() {
+		if (this.versionOrDirtyLevel !== DirtyLevels.None) {
+			this.versionOrDirtyLevel = DirtyLevels.None;
+			Subscriber.runInnerEffects(this);
+		}
+	}
+
 	run<T>(fn: () => T) {
 		const prevActiveSub = Subscriber.startTrack(this, true);
 		const res = fn();
diff --git a/lib/system.ts b/lib/system.ts
index 289d875..6a9782b 100644
--- a/lib/system.ts
+++ b/lib/system.ts
@@ -33,7 +33,8 @@ export interface Link {
 }
 
 export const enum DirtyLevels {
-	NotDirty,
+	None,
+	SideEffectsOnly,
 	MaybeDirty,
 	Dirty,
 }
@@ -228,6 +229,7 @@ export namespace Dependency {
 	}
 
 	export function propagate(dep: Dependency) {
+		let depIsEffect = false;
 		let link = dep.subs;
 		let dirtyLevel = DirtyLevels.Dirty;
 		let depth = 0;
@@ -242,19 +244,25 @@ export namespace Dependency {
 					sub.versionOrDirtyLevel = dirtyLevel;
 				}
 
-				if (subDirtyLevel === DirtyLevels.NotDirty) {
+				if (subDirtyLevel === DirtyLevels.None) {
+					const subIsEffect = 'notify' in sub;
 
 					if ('subs' in sub) {
 						sub.deps!.prevPropagateOrNextReleased = link;
 						dep = sub;
+						depIsEffect = subIsEffect;
 						link = sub.subs;
-						dirtyLevel = DirtyLevels.MaybeDirty;
+						if (subIsEffect) {
+							dirtyLevel = DirtyLevels.SideEffectsOnly;
+						} else {
+							dirtyLevel = DirtyLevels.MaybeDirty;
+						}
 						depth++;
 
 						continue top;
 					}
 
-					if ('notify' in sub) {
+					if (subIsEffect) {
 						const queuedEffectsTail = system.queuedEffectsTail;
 
 						if (queuedEffectsTail !== undefined) {
@@ -278,11 +286,16 @@ export namespace Dependency {
 				if (prevLink !== undefined) {
 					depDeps.prevPropagateOrNextReleased = undefined;
 					dep = prevLink.dep;
+					depIsEffect = 'notify' in dep;
 					link = prevLink.nextSub;
 					depth--;
 
 					if (depth === 0) {
 						dirtyLevel = DirtyLevels.Dirty;
+					} else if (depIsEffect) {
+						dirtyLevel = DirtyLevels.SideEffectsOnly;
+					} else {
+						dirtyLevel = DirtyLevels.MaybeDirty;
 					}
 
 					const prevSub = prevLink.sub;
@@ -312,6 +325,17 @@ export namespace Subscriber {
 
 	const system = System;
 
+	export function runInnerEffects(sub: Subscriber) {
+		let link = sub.deps as Link | undefined;
+		while (link !== undefined) {
+			const dep = link.dep as Dependency | Dependency & IEffect;
+			if ('notify' in dep) {
+				dep.notify();
+			}
+			link = link.nextDep;
+		}
+	}
+
 	export function resolveMaybeDirty(sub: Subscriber) {
 		let link = sub.deps;
 
@@ -344,7 +368,7 @@ export namespace Subscriber {
 			const dirtyLevel = sub.versionOrDirtyLevel;
 
 			if (dirtyLevel === DirtyLevels.MaybeDirty) {
-				sub.versionOrDirtyLevel = DirtyLevels.NotDirty;
+				sub.versionOrDirtyLevel = DirtyLevels.None;
 			}
 
 			const subSubs = (sub as Dependency & Subscriber).subs;
@@ -417,6 +441,6 @@ export namespace Subscriber {
 			Link.release(sub.deps);
 			sub.deps = undefined;
 		}
-		sub.versionOrDirtyLevel = DirtyLevels.NotDirty;
+		sub.versionOrDirtyLevel = DirtyLevels.None;
 	}
 }
diff --git a/tests/effect.spec.ts b/tests/effect.spec.ts
index 89cd2f3..5cd0d39 100644
--- a/tests/effect.spec.ts
+++ b/tests/effect.spec.ts
@@ -89,12 +89,14 @@ test('should not trigger inner effect when resolve maybe dirty', () => {
 	a.set(2);
 });
 
-test.skip('should trigger inner effects in sequence', () => {
+test('should trigger inner effects in sequence', () => {
 	const a = signal(0);
 	const b = signal(0);
+	const c = computed(() => a.get() - b.get());
 	const order: string[] = [];
 
 	effect(() => {
+		c.get();
 
 		effect(() => {
 			order.push('first inner');
@@ -118,7 +120,7 @@ test.skip('should trigger inner effects in sequence', () => {
 	expect(order).toEqual(['first inner', 'last inner']);
 });
 
-test.skip('should trigger inner effects in sequence in effect scope', () => {
+test('should trigger inner effects in sequence in effect scope', () => {
 	const a = signal(0);
 	const b = signal(0);
 	const scope = effectScope();