fix: defer effects behind dirty computed recompute

Author: brnrdog

packages/rescript-signals/src/signals/Scheduler.res

@@ -448,7 +448,9 @@ let flush = (): unit => {
 }
 
 // Notify all subscribers of a signal (traverse linked list)
-// Marks computeds dirty transitively (lazy), queues effects for execution
+// Marks computeds dirty transitively.
+// Direct effects are queued immediately.
+// Effects reached through dirty computeds are deferred until parent computed recompute.
 let notifySubs = (subs: Core.subs): unit => {
   dirtyQueue->Array.push(subs)->ignore
 
@@ -472,9 +474,14 @@ let notifySubs = (subs: Core.subs): unit => {
               dirtyQueue->Array.push(linkedSubs)->ignore
             }
           } else {
-            // It's an effect - queue for execution
-            let observer = l.observer
-            addEffectToPending(observer)
+            // It's an effect.
+            // If reached via a dirty computed, defer effect until computed recompute.
+            // This lets computed equality short-circuit downstream effect runs.
+            if Core.isComputed(s) {
+              addComputedToPending(s)
+            } else {
+              addEffectToPending(l.observer)
+            }
           }
           link := l.nextSub
         | None => ()
@@ -484,7 +491,7 @@ let notifySubs = (subs: Core.subs): unit => {
   }
   clearArray(dirtyQueue)
 
-  if pendingEffects->Array.length > 0 && !flushing.contents {
+  if (pendingEffects->Array.length > 0 || pendingComputedSubs->Array.length > 0) && !flushing.contents {
     flush()
   }
 }

packages/rescript-signals/tests/ComputedTests.res

@@ -178,7 +178,7 @@ let tests = Suite.make(
 
       Assert.combineResults([result1, result2, result3])
     }),
-    Test.make("custom equals avoids duplicate downstream effect runs", () => {
+    Test.make("custom equals suppresses downstream effect when value is unchanged", () => {
       let profile = Signal.make({"id": 1, "name": "Alice"})
       let userId = Computed.make(
         () => Signal.get(profile)["id"],
@@ -197,21 +197,46 @@ let tests = Suite.make(
       Signal.set(profile, {"id": 1, "name": "Bob"})
       let result2 = Assert.equal(
         effectRuns.contents,
-        2,
-        ~message="Effect should run once for the update",
+        1,
+        ~message="Effect should not run when derived value is unchanged",
       )
 
       // Derived id changes.
       Signal.set(profile, {"id": 2, "name": "Bob"})
       let result3 = Assert.equal(
         effectRuns.contents,
-        3,
-        ~message="Effect should still run once per update when derived value changes",
+        2,
+        ~message="Effect should run once when derived value changes",
       )
 
       disposer.dispose()
       Assert.combineResults([result1, result2, result3])
     }),
+    Test.make("repro: custom equals should suppress effect when derived value is unchanged", () => {
+      let profile = Signal.make({"id": 1, "name": "Alice"})
+      let userId = Computed.make(
+        () => Signal.get(profile)["id"],
+        ~equals=(a, b) => a == b,
+      )
+      let effectRuns = ref(0)
+      let disposer = Effect.runWithDisposer(() => {
+        effectRuns := effectRuns.contents + 1
+        ignore(Signal.get(userId))
+        None
+      })
+
+      // Source update does not change computed output.
+      Signal.set(profile, {"id": 1, "name": "Bob"})
+
+      let result = Assert.equal(
+        effectRuns.contents,
+        1,
+        ~message="Expected no effect re-run when custom equals says computed output is unchanged",
+      )
+
+      disposer.dispose()
+      result
+    }),
     Test.make("computed equality with array length", () => {
       let items = Signal.make([1, 2, 3])
       let recomputeCount = ref(0)