From 43cbb34be849e16cda14413afba0906097cc6e2e Mon Sep 17 00:00:00 2001
From: Luke Chu <37006668+lukechu10@users.noreply.github.com>
Date: Sun, 7 Mar 2021 09:25:58 -0800
Subject: [PATCH] Do not subscribe multiple times to a same dependency (#7)

* Do not subscribe multiple times to a same dependency

* cargo fmt
---
 maple-core-macro/src/lib.rs |  2 +-
 maple-core/src/lib.rs       |  2 +-
 maple-core/src/reactive.rs  | 77 +++++++++++++++++++++++++++++++++++--
 3 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/maple-core-macro/src/lib.rs b/maple-core-macro/src/lib.rs
index 5aa8a7de7..f9c78f538 100644
--- a/maple-core-macro/src/lib.rs
+++ b/maple-core-macro/src/lib.rs
@@ -32,7 +32,7 @@ impl ToTokens for HtmlTree {
 }
 
 /// A macro for ergonomically creating complex UI structures.
-/// 
+///
 /// TODO: write some more docs
 #[proc_macro]
 pub fn template(input: TokenStream) -> TokenStream {
diff --git a/maple-core/src/lib.rs b/maple-core/src/lib.rs
index 3804990b6..b155a3a33 100644
--- a/maple-core/src/lib.rs
+++ b/maple-core/src/lib.rs
@@ -1,7 +1,7 @@
 //! # Maple API Documentation
 //!
 //! Maple is a VDOM-less web library with fine-grained reactivity.
-//! 
+//!
 //! This is the API docs for maple. If you are looking for the usage docs, checkout the [README](https://github.com/lukechu10/maple).
 //!
 //! ## Supported Targets
diff --git a/maple-core/src/reactive.rs b/maple-core/src/reactive.rs
index c0882bab0..f341f4fae 100644
--- a/maple-core/src/reactive.rs
+++ b/maple-core/src/reactive.rs
@@ -18,7 +18,18 @@ impl<T> Signal<T> {
     }
 
     fn observe(&mut self, handler: Rc<Computation>) {
-        self.observers.push(handler);
+        // make sure handler is not already in self.observers
+        if self
+            .observers
+            .iter()
+            .find(|observer| {
+                observer.as_ref() as *const Computation == handler.as_ref() as *const Computation
+                /* do reference equality */
+            })
+            .is_none()
+        {
+            self.observers.push(handler);
+        }
     }
 
     fn update(&mut self, new_value: T) {
@@ -44,14 +55,14 @@ thread_local! {
 
 /// Creates a new signal.
 /// The function will return a pair of getter/setters to modify the signal and update corresponding dependencies.
-/// 
+///
 /// # Example
 /// ```rust
 /// use maple_core::prelude::*;
-/// 
+///
 /// let (state, set_state) = create_signal(0);
 /// assert_eq!(*state(), 0);
-/// 
+///
 /// set_state(1);
 /// assert_eq!(*state(), 1);
 /// ```
@@ -193,6 +204,64 @@ mod tests {
         assert_eq!(*double(), 4);
     }
 
+    #[test]
+    #[should_panic(expected = "cannot create cyclic dependency")]
+    fn cyclic_effects_fail() {
+        let (state, set_state) = create_signal(0);
+
+        create_effect({
+            let state = state.clone();
+            let set_state = set_state.clone();
+            move || {
+                set_state(*state() + 1);
+            }
+        });
+
+        set_state(1);
+    }
+
+    #[test]
+    #[should_panic(expected = "cannot create cyclic dependency")]
+    fn cyclic_effects_fail_2() {
+        let (state, set_state) = create_signal(0);
+
+        create_effect({
+            let state = state.clone();
+            let set_state = set_state.clone();
+            move || {
+                let value = *state();
+                set_state(value + 1);
+            }
+        });
+
+        set_state(1);
+    }
+
+    #[test]
+    fn effect_should_subscribe_once() {
+        let (state, set_state) = create_signal(0);
+
+        // use a Cell instead of a signal to prevent circular dependencies
+        // TODO: change to create_signal once explicit tracking is implemented
+        let counter = Rc::new(Cell::new(0));
+
+        create_effect({
+            let counter = counter.clone();
+            move || {
+                counter.set(counter.get() + 1);
+
+                // call state() twice but should subscribe once
+                state();
+                state();
+            }
+        });
+
+        assert_eq!(counter.get(), 1);
+
+        set_state(1);
+        assert_eq!(counter.get(), 2);
+    }
+
     #[test]
     fn memo() {
         let (state, set_state) = create_signal(0);