Support listener recursion (#79)

* Fix listener recursion

Fixes a panic when trying to reduce a store from the listener of that
same store. To accomplish this we had to restrict Listener::on_change
to be immutable borrow of self only. Although this means we can no
longer keep local state in listener's, allowing recursion and avoiding
the panic seems an improvement.

Now to track and manage separate state, we need only create a new store.

* Fix docs

* Improve wording

crates/yewdux-macros/src/store.rs

@@ -22,7 +22,7 @@ pub(crate) fn derive(input: DeriveInput) -> TokenStream {
         .map(|path| {
             quote! {
                 ::yewdux::listener::init_listener(
-                    #path, cx
+                    || #path, cx
                 );
             }
         })
@@ -53,7 +53,7 @@ pub(crate) fn derive(input: DeriveInput) -> TokenStream {
                 #[cfg(target_arch = "wasm32")]
                 fn new(cx: &::yewdux::Context) -> Self {
                     ::yewdux::listener::init_listener(
-                        ::yewdux::storage::StorageListener::<Self>::new(#area),
+                        || ::yewdux::storage::StorageListener::<Self>::new(#area),
                         cx
                     );
                     #(#extra_listeners)*

crates/yewdux-utils/src/lib.rs

@@ -24,7 +24,7 @@ impl<T: Store + PartialEq> Reducer<HistoryStore<T>> for HistoryChangeMessage<T>
 impl<T: Store + PartialEq> Listener for HistoryListener<T> {
     type Store = T;
 
-    fn on_change(&mut self, cx: &Context, state: Rc<Self::Store>) {
+    fn on_change(&self, cx: &Context, state: Rc<Self::Store>) {
         Dispatch::<HistoryStore<T>>::new(cx).apply(HistoryChangeMessage::<T>(state))
     }
 }

crates/yewdux/src/context.rs

@@ -66,7 +66,15 @@ impl Context {
             .expect("CONTEXTS thread local key init failed")
     }
 
-    pub(crate) fn get_or_init<S: Store>(&self) -> Entry<S> {
+    /// Initialize a store using a custom constructor. `Store::new` will not be called in this
+    /// case. If already initialized, the custom constructor will not be called.
+    pub fn init<S: Store, F: FnOnce(&Self) -> S>(&self, new_store: F) {
+        self.get_or_init(new_store);
+    }
+
+    /// Get or initialize a store using a custom constructor. `Store::new` will not be called in
+    /// this case. If already initialized, the custom constructor will not be called.
+    pub(crate) fn get_or_init<S: Store, F: FnOnce(&Self) -> S>(&self, new_store: F) -> Entry<S> {
         // Get context, or None if it doesn't exist.
         //
         // We use an option here because a new Store should not be created during this borrow. We
@@ -87,7 +95,7 @@ impl Context {
             // Init store outside of borrow. This allows the store to access other stores when it
             // is being created.
             let entry = Entry {
-                store: Mrc::new(Rc::new(S::new(self))),
+                store: Mrc::new(Rc::new(new_store(self))),
             };
 
             *maybe_entry.borrow_mut() = Some(entry);
@@ -102,8 +110,13 @@ impl Context {
         entry
     }
 
+    /// Get or initialize a store with a default Store::new implementation.
+    pub(crate) fn get_or_init_default<S: Store>(&self) -> Entry<S> {
+        self.get_or_init(S::new)
+    }
+
     pub fn reduce<S: Store, R: Reducer<S>>(&self, r: R) {
-        let entry = self.get_or_init::<S>();
+        let entry = self.get_or_init_default::<S>();
         let should_notify = entry.reduce(r);
 
         if should_notify {
@@ -126,12 +139,12 @@ impl Context {
 
     /// Get current state.
     pub fn get<S: Store>(&self) -> Rc<S> {
-        Rc::clone(&self.get_or_init::<S>().store.borrow())
+        Rc::clone(&self.get_or_init_default::<S>().store.borrow())
     }
 
     /// Send state to all subscribers.
     pub fn notify_subscribers<S: Store>(&self, state: Rc<S>) {
-        let entry = self.get_or_init::<Mrc<Subscribers<S>>>();
+        let entry = self.get_or_init_default::<Mrc<Subscribers<S>>>();
         entry.store.borrow().notify(state);
     }
 
@@ -140,15 +153,15 @@ impl Context {
         // Notify subscriber with inital state.
         on_change.call(self.get::<S>());
 
-        self.get_or_init::<Mrc<Subscribers<S>>>()
+        self.get_or_init_default::<Mrc<Subscribers<S>>>()
             .store
             .borrow()
             .subscribe(on_change)
     }
 
     /// Similar to [Self::subscribe], however state is not called immediately.
     pub fn subscribe_silent<S: Store, N: Callable<S>>(&self, on_change: N) -> SubscriberId<S> {
-        self.get_or_init::<Mrc<Subscribers<S>>>()
+        self.get_or_init_default::<Mrc<Subscribers<S>>>()
             .store
             .borrow()
             .subscribe(on_change)
@@ -177,7 +190,7 @@ mod tests {
     struct TestState2(u32);
     impl Store for TestState2 {
         fn new(cx: &Context) -> Self {
-            cx.get_or_init::<TestState>();
+            cx.get_or_init_default::<TestState>();
             Self(0)
         }
 
@@ -188,7 +201,7 @@ mod tests {
 
     #[test]
     fn can_access_other_store_for_new_of_current_store() {
-        let _context = Context::new().get_or_init::<TestState2>();
+        let _context = Context::new().get_or_init_default::<TestState2>();
     }
 
     #[derive(Clone, PartialEq, Eq)]
@@ -215,9 +228,21 @@ mod tests {
     #[test]
     fn store_new_is_only_called_once() {
         let cx = Context::new();
-        cx.get_or_init::<StoreNewIsOnlyCalledOnce>();
-        let entry = cx.get_or_init::<StoreNewIsOnlyCalledOnce>();
+        cx.get_or_init_default::<StoreNewIsOnlyCalledOnce>();
+        let entry = cx.get_or_init_default::<StoreNewIsOnlyCalledOnce>();
 
         assert!(entry.store.borrow().0.get() == 1)
     }
+
+    #[test]
+    fn recursive_reduce() {
+        let cx = Context::new();
+        let cx2 = cx.clone();
+        cx.reduce::<TestState, _>(|_s: Rc<TestState>| {
+            cx2.reduce::<TestState, _>(|s: Rc<TestState>| TestState(s.0 + 1).into());
+            TestState(cx2.get::<TestState>().0 + 1).into()
+        });
+
+        assert_eq!(cx.get::<TestState>().0, 2);
+    }
 }

crates/yewdux/src/dispatch.rs

@@ -760,7 +760,7 @@ mod tests {
     #[test]
     fn dispatch_unsubscribes_when_dropped() {
         let cx = Context::new();
-        let entry = cx.get_or_init::<Mrc<Subscribers<TestState>>>();
+        let entry = cx.get_or_init_default::<Mrc<Subscribers<TestState>>>();
 
         assert!(entry.store.borrow().borrow().0.is_empty());
 
@@ -776,7 +776,7 @@ mod tests {
     #[test]
     fn dispatch_clone_and_original_unsubscribe_when_both_dropped() {
         let cx = Context::new();
-        let entry = cx.get_or_init::<Mrc<Subscribers<TestState>>>();
+        let entry = cx.get_or_init_default::<Mrc<Subscribers<TestState>>>();
 
         assert!(entry.store.borrow().borrow().0.is_empty());
 

crates/yewdux/src/listener.rs

@@ -1,36 +1,37 @@
 use std::rc::Rc;
 
-use crate::{context::Context, dispatch::Dispatch, mrc::Mrc, store::Store};
+use crate::{context::Context, dispatch::Dispatch, store::Store};
 
 /// Listens to [Store](crate::store::Store) changes.
 pub trait Listener: 'static {
     type Store: Store;
 
-    fn on_change(&mut self, cx: &Context, state: Rc<Self::Store>);
+    fn on_change(&self, cx: &Context, state: Rc<Self::Store>);
 }
 
-struct ListenerStore<L: Listener>(Option<Dispatch<L::Store>>);
-impl<L: Listener> Store for Mrc<ListenerStore<L>> {
+#[allow(unused)]
+struct ListenerStore<L: Listener>(Dispatch<L::Store>);
+impl<L: Listener> Store for ListenerStore<L> {
     fn new(_cx: &Context) -> Self {
-        ListenerStore(None).into()
+        unimplemented!()
     }
 
-    fn should_notify(&self, other: &Self) -> bool {
-        self != other
+    fn should_notify(&self, _other: &Self) -> bool {
+        false
     }
 }
 
-/// Initiate a [Listener]. If this listener has already been initiated, it is dropped and replaced
-/// with the new one.
-pub fn init_listener<L: Listener>(listener: L, cx: &Context) {
-    let dispatch = {
-        let listener = Mrc::new(listener);
-        let cxo = cx.clone();
-        Dispatch::new(cx)
-            .subscribe_silent(move |state| listener.borrow_mut().on_change(&cxo, state))
-    };
-
-    cx.reduce_mut(|state: &mut Mrc<ListenerStore<L>>| state.borrow_mut().0 = Some(dispatch));
+/// Initiate a [Listener]. Does nothing if listener is already initiated.
+pub fn init_listener<L: Listener, F: FnOnce() -> L>(new_listener: F, cx: &Context) {
+    cx.get_or_init(|cx| {
+        let dispatch = {
+            let listener = new_listener();
+            let cx = cx.clone();
+            Dispatch::new(&cx).subscribe_silent(move |state| listener.on_change(&cx, state))
+        };
+
+        ListenerStore::<L>(dispatch)
+    });
 }
 
 #[cfg(test)]
@@ -57,7 +58,7 @@ mod tests {
     impl Listener for TestListener {
         type Store = TestState;
 
-        fn on_change(&mut self, _cx: &Context, state: Rc<Self::Store>) {
+        fn on_change(&self, _cx: &Context, state: Rc<Self::Store>) {
             self.0.set(state.0);
         }
     }
@@ -67,7 +68,7 @@ mod tests {
     impl Listener for AnotherTestListener {
         type Store = TestState;
 
-        fn on_change(&mut self, _cx: &Context, state: Rc<Self::Store>) {
+        fn on_change(&self, _cx: &Context, state: Rc<Self::Store>) {
             self.0.set(state.0);
         }
     }
@@ -76,7 +77,7 @@ mod tests {
     struct TestState2;
     impl Store for TestState2 {
         fn new(cx: &Context) -> Self {
-            init_listener(TestListener2, cx);
+            init_listener(|| TestListener2, cx);
             Self
         }
 
@@ -90,39 +91,73 @@ mod tests {
     impl Listener for TestListener2 {
         type Store = TestState2;
 
-        fn on_change(&mut self, _cx: &Context, _state: Rc<Self::Store>) {}
+        fn on_change(&self, _cx: &Context, _state: Rc<Self::Store>) {}
+    }
+
+    #[derive(Clone, PartialEq, Eq)]
+    struct TestStateRecursive(u32);
+    impl Store for TestStateRecursive {
+        fn new(_cx: &Context) -> Self {
+            Self(0)
+        }
+
+        fn should_notify(&self, other: &Self) -> bool {
+            self != other
+        }
+    }
+
+    #[derive(Clone)]
+    struct TestListenerRecursive;
+    impl Listener for TestListenerRecursive {
+        type Store = TestStateRecursive;
+
+        fn on_change(&self, cx: &Context, state: Rc<Self::Store>) {
+            let dispatch = Dispatch::<TestStateRecursive>::new(cx);
+            if state.0 < 10 {
+                dispatch.reduce_mut(|state| state.0 += 1);
+            }
+        }
+    }
+
+    #[test]
+    fn recursion() {
+        let cx = Context::new();
+        init_listener(|| TestListenerRecursive, &cx);
+        let dispatch = Dispatch::<TestStateRecursive>::new(&cx);
+        dispatch.reduce_mut(|state| state.0 = 1);
+        assert_eq!(dispatch.get().0, 10);
     }
 
     #[test]
     fn listener_is_called() {
         let cx = Context::new();
         let listener = TestListener(Default::default());
 
-        init_listener(listener.clone(), &cx);
+        init_listener(|| listener.clone(), &cx);
 
         Dispatch::new(&cx).reduce_mut(|state: &mut TestState| state.0 = 1);
 
         assert_eq!(listener.0.get(), 1)
     }
 
     #[test]
-    fn listener_is_replaced() {
+    fn listener_is_not_replaced() {
         let cx = Context::new();
         let listener1 = TestListener(Default::default());
         let listener2 = TestListener(Default::default());
 
-        init_listener(listener1.clone(), &cx);
+        init_listener(|| listener1.clone(), &cx);
 
         Dispatch::new(&cx).reduce_mut(|state: &mut TestState| state.0 = 1);
 
         assert_eq!(listener1.0.get(), 1);
 
-        init_listener(listener2.clone(), &cx);
+        init_listener(|| listener2.clone(), &cx);
 
         Dispatch::new(&cx).reduce_mut(|state: &mut TestState| state.0 = 2);
 
-        assert_eq!(listener1.0.get(), 1);
-        assert_eq!(listener2.0.get(), 2);
+        assert_eq!(listener1.0.get(), 2);
+        assert_eq!(listener2.0.get(), 0);
     }
 
     #[test]
@@ -131,13 +166,13 @@ mod tests {
         let listener1 = TestListener(Default::default());
         let listener2 = AnotherTestListener(Default::default());
 
-        init_listener(listener1.clone(), &cx);
+        init_listener(|| listener1.clone(), &cx);
 
         cx.reduce_mut(|state: &mut TestState| state.0 = 1);
 
         assert_eq!(listener1.0.get(), 1);
 
-        init_listener(listener2.clone(), &cx);
+        init_listener(|| listener2.clone(), &cx);
 
         cx.reduce_mut(|state: &mut TestState| state.0 = 2);
 

crates/yewdux/src/storage.rs

@@ -86,7 +86,7 @@ where
 {
     type Store = T;
 
-    fn on_change(&mut self, _cx: &Context, state: Rc<Self::Store>) {
+    fn on_change(&self, _cx: &Context, state: Rc<Self::Store>) {
         if let Err(err) = save(state.as_ref(), self.area) {
             crate::log::error!("Error saving state to storage: {:?}", err);
         }

crates/yewdux/src/subscriber.rs

@@ -126,7 +126,7 @@ mod tests {
     #[test]
     fn subscribe_adds_to_list() {
         let cx = Context::new();
-        let entry = cx.get_or_init::<Mrc<Subscribers<TestState>>>();
+        let entry = cx.get_or_init_default::<Mrc<Subscribers<TestState>>>();
 
         assert!(entry.store.borrow().borrow().0.is_empty());
 
@@ -138,7 +138,7 @@ mod tests {
     #[test]
     fn unsubscribe_removes_from_list() {
         let cx = Context::new();
-        let entry = cx.get_or_init::<Mrc<Subscribers<TestState>>>();
+        let entry = cx.get_or_init_default::<Mrc<Subscribers<TestState>>>();
 
         assert!(entry.store.borrow().borrow().0.is_empty());
 
@@ -154,7 +154,7 @@ mod tests {
     #[test]
     fn subscriber_id_unsubscribes_when_dropped() {
         let cx = Context::new();
-        let entry = cx.get_or_init::<Mrc<Subscribers<TestState>>>();
+        let entry = cx.get_or_init_default::<Mrc<Subscribers<TestState>>>();
 
         assert!(entry.store.borrow().borrow().0.is_empty());