fix: only rerun effects if they have dirty ancestors (or it's the first run)

reactive_graph/src/effect/effect.rs

@@ -1,7 +1,9 @@
 use crate::{
     channel::{channel, Receiver},
     effect::inner::EffectInner,
-    graph::{AnySubscriber, SourceSet, Subscriber, ToAnySubscriber},
+    graph::{
+        AnySubscriber, ReactiveNode, SourceSet, Subscriber, ToAnySubscriber,
+    },
     owner::{Owner, StoredValue},
     traits::Dispose,
 };
@@ -51,6 +53,7 @@ impl Effect {
     {
         let (mut rx, owner, inner) = effect_base();
         let value = Arc::new(RwLock::new(None));
+        let mut first_run = true;
 
         Executor::spawn_local({
             let value = Arc::clone(&value);
@@ -58,14 +61,17 @@ impl Effect {
 
             async move {
                 while rx.next().await.is_some() {
-                    subscriber.clear_sources(&subscriber);
+                    if first_run || subscriber.update_if_necessary() {
+                        first_run = false;
+                        subscriber.clear_sources(&subscriber);
 
-                    let old_value =
-                        mem::take(&mut *value.write().or_poisoned());
-                    let new_value = owner.with_cleanup(|| {
-                        subscriber.with_observer(|| fun(old_value))
-                    });
-                    *value.write().or_poisoned() = Some(new_value);
+                        let old_value =
+                            mem::take(&mut *value.write().or_poisoned());
+                        let new_value = owner.with_cleanup(|| {
+                            subscriber.with_observer(|| fun(old_value))
+                        });
+                        *value.write().or_poisoned() = Some(new_value);
+                    }
                 }
             }
         });
@@ -82,6 +88,7 @@ impl Effect {
         T: Send + Sync + 'static,
     {
         let (mut rx, owner, inner) = effect_base();
+        let mut first_run = true;
         let value = Arc::new(RwLock::new(None));
 
         Executor::spawn({
@@ -90,14 +97,17 @@ impl Effect {
 
             async move {
                 while rx.next().await.is_some() {
-                    subscriber.clear_sources(&subscriber);
+                    if first_run || subscriber.update_if_necessary() {
+                        first_run = false;
+                        subscriber.clear_sources(&subscriber);
 
-                    let old_value =
-                        mem::take(&mut *value.write().or_poisoned());
-                    let new_value = owner.with_cleanup(|| {
-                        subscriber.with_observer(|| fun(old_value))
-                    });
-                    *value.write().or_poisoned() = Some(new_value);
+                        let old_value =
+                            mem::take(&mut *value.write().or_poisoned());
+                        let new_value = owner.with_cleanup(|| {
+                            subscriber.with_observer(|| fun(old_value))
+                        });
+                        *value.write().or_poisoned() = Some(new_value);
+                    }
                 }
             }
         });

reactive_graph/tests/memo.rs

@@ -1,7 +1,7 @@
 use any_spawner::Executor;
 use reactive_graph::{
     computed::{ArcMemo, Memo},
-    effect::Effect,
+    effect::{Effect, RenderEffect},
     prelude::*,
     signal::RwSignal,
 };
@@ -9,6 +9,7 @@ use std::{
     mem,
     sync::{Arc, RwLock},
 };
+use tokio::task;
 
 pub async fn tick() {
     tokio::time::sleep(std::time::Duration::from_micros(1)).await;
@@ -237,3 +238,69 @@ async fn dynamic_dependencies() {
 
     assert_eq!(*combined_count.read().unwrap(), 5);
 }
+
+#[tokio::test]
+async fn render_effect_doesnt_rerun_if_memo_didnt_change() {
+    _ = Executor::init_tokio();
+
+    task::LocalSet::new()
+        .run_until(async {
+            let count = RwSignal::new(1);
+            let even = Memo::new(move |_| *count.read() % 2 == 0);
+
+            let combined_count = Arc::new(RwLock::new(0));
+
+            mem::forget(RenderEffect::new({
+                let combined_count = Arc::clone(&combined_count);
+                move |_| {
+                    *combined_count.write().unwrap() += 1;
+                    println!("even = {}", even.get());
+                }
+            }));
+
+            tick().await;
+            assert_eq!(*combined_count.read().unwrap(), 1);
+
+            count.set(2);
+            tick().await;
+            assert_eq!(*combined_count.read().unwrap(), 2);
+
+            count.set(4);
+            tick().await;
+            assert_eq!(*combined_count.read().unwrap(), 2);
+        })
+        .await
+}
+
+#[tokio::test]
+async fn effect_doesnt_rerun_if_memo_didnt_change() {
+    _ = Executor::init_tokio();
+
+    task::LocalSet::new()
+        .run_until(async {
+            let count = RwSignal::new(1);
+            let even = Memo::new(move |_| *count.read() % 2 == 0);
+
+            let combined_count = Arc::new(RwLock::new(0));
+
+            Effect::new({
+                let combined_count = Arc::clone(&combined_count);
+                move |_| {
+                    *combined_count.write().unwrap() += 1;
+                    println!("even = {}", even.get());
+                }
+            });
+
+            tick().await;
+            assert_eq!(*combined_count.read().unwrap(), 1);
+
+            count.set(2);
+            tick().await;
+            assert_eq!(*combined_count.read().unwrap(), 2);
+
+            count.set(4);
+            tick().await;
+            assert_eq!(*combined_count.read().unwrap(), 2);
+        })
+        .await
+}