From 242d35cc37e3f437b4503069ee5cedc0c49f2176 Mon Sep 17 00:00:00 2001
From: Greg Johnston <greg.johnston@gmail.com>
Date: Fri, 24 May 2024 08:33:13 -0400
Subject: [PATCH] add proper dirty checking on AsyncDerived so it can read from
 memos properly

---
 .../async_derived/arc_async_derived.rs        |  1 +
 .../src/computed/async_derived/inner.rs       | 38 ++++++++++---------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/reactive_graph/src/computed/async_derived/arc_async_derived.rs b/reactive_graph/src/computed/async_derived/arc_async_derived.rs
index f45ee3b3f..032400140 100644
--- a/reactive_graph/src/computed/async_derived/arc_async_derived.rs
+++ b/reactive_graph/src/computed/async_derived/arc_async_derived.rs
@@ -91,6 +91,7 @@ macro_rules! spawn_derived {
             notifier,
             sources: SourceSet::new(),
             subscribers: SubscriberSet::new(),
+            dirty: false
         }));
         let value = Arc::new(RwLock::new($initial));
         let wakers = Arc::new(RwLock::new(Vec::new()));
diff --git a/reactive_graph/src/computed/async_derived/inner.rs b/reactive_graph/src/computed/async_derived/inner.rs
index a0d56c42b..114ea4803 100644
--- a/reactive_graph/src/computed/async_derived/inner.rs
+++ b/reactive_graph/src/computed/async_derived/inner.rs
@@ -18,11 +18,14 @@ pub(crate) struct ArcAsyncDerivedInner {
     pub subscribers: SubscriberSet,
     // when a source changes, notifying this will cause the async work to rerun
     pub notifier: Sender,
+    pub dirty: bool,
 }
 
 impl ReactiveNode for RwLock<ArcAsyncDerivedInner> {
     fn mark_dirty(&self) {
-        self.write().or_poisoned().notifier.notify();
+        let mut lock = self.write().or_poisoned();
+        lock.dirty = true;
+        lock.notifier.notify();
     }
 
     fn mark_check(&self) {
@@ -37,23 +40,22 @@ impl ReactiveNode for RwLock<ArcAsyncDerivedInner> {
     }
 
     fn update_if_necessary(&self) -> bool {
-        // if update_is_necessary is being called, that mean that a subscriber
-        // wants to know if our latest value has changed
-        //
-        // this could be the case either because
-        // 1) we have updated, and asynchronously woken the subscriber back up
-        // 2) a different source has woken up the subscriber, and it's now asking us
-        //    if we've changed
-        //
-        // if we return `false` it will short-circuit that subscriber
-        // if we return `true` it means "yes, we may have changed"
-        //
-        // returning `true` here means that an AsyncDerived behaves like a signal (it always says
-        // "sure, I"ve changed) and not like a memo (checks whether it has *actually* changed)
-        //
-        // TODO is there a dirty-checking mechanism that would work here? we would need a
-        // memoization process like a memo has, to ensure we don't over-notify
-        true
+        let mut guard = self.write().or_poisoned();
+        let (is_dirty, sources) =
+            (guard.dirty, (!guard.dirty).then(|| guard.sources.clone()));
+
+        if is_dirty {
+            guard.dirty = false;
+            return true;
+        }
+
+        drop(guard);
+        for source in sources.into_iter().flatten() {
+            if source.update_if_necessary() {
+                return true;
+            }
+        }
+        false
     }
 }