From 283fee9de1672f9b303a82aaa06221e952e574b5 Mon Sep 17 00:00:00 2001
From: Doug Parker <dgp1130@users.noreply.github.com>
Date: Sun, 1 Dec 2024 16:56:27 -0800
Subject: [PATCH] Stops notifying consumers added during the notification
 process.

While trying out a synchronous scheduler, I found a case where notifying a `Consumer` caused a new `Consumer` to be added. This happened recursively and caused and infinite loop, repeatedly adding and then notifying a new `Consumer` endlessly.

`Producer` objects should only notify existing `Consumer` objects, because those are the objects which are potentially dirty. Any `Consumer` added during the notification already observes the new value of the `Producer` and does not need to be notified. This avoids an infinite loop in the sync scheduler case and also reduces unnecessary notifications for a `Consumer` added at the wrong time.
---
 src/signals/graph.test.ts | 20 ++++++++++++++++++++
 src/signals/graph.ts      |  6 +++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/signals/graph.test.ts b/src/signals/graph.test.ts
index b4e7918..3f31896 100644
--- a/src/signals/graph.test.ts
+++ b/src/signals/graph.test.ts
@@ -159,6 +159,26 @@ describe('graph', () => {
       expect(consumer1.notifyListeners).toHaveBeenCalledOnceWith();
       expect(consumer2.notifyListeners).not.toHaveBeenCalled();
     });
+
+    it('ignores consumers added during notification', () => {
+      const producer = Producer.from(() => 1);
+
+      const consumer1 = Consumer.from();
+      const consumer2 = Consumer.from();
+
+      spyOn(consumer1, 'notifyListeners').and.callThrough();
+      spyOn(consumer2, 'notifyListeners').and.callThrough();
+
+      producer.addConsumer(consumer1);
+      consumer1.listen(() => {
+        producer.addConsumer(consumer2);
+      });
+
+      producer.notifyConsumers();
+
+      expect(consumer1.notifyListeners).toHaveBeenCalled();
+      expect(consumer2.notifyListeners).not.toHaveBeenCalled();
+    });
   });
 
   describe('Consumer', () => {
diff --git a/src/signals/graph.ts b/src/signals/graph.ts
index 33344a3..45d2689 100644
--- a/src/signals/graph.ts
+++ b/src/signals/graph.ts
@@ -140,7 +140,11 @@ export class Producer<Value> {
    * {@link Producer} has changed.
    */
   public notifyConsumers(): void {
-    for (const consumer of this.#consumers) consumer.notifyListeners();
+    // Snapshot current consumers with `Array.from`, because they might change
+    // during notification callbacks.
+    for (const consumer of Array.from(this.#consumers)) {
+      consumer.notifyListeners();
+    }
   }
 }