Fixing a bug with RefCounter / MutableListener

src/dom.rs

@@ -5,7 +5,7 @@ use std::future::Future;
 use std::task::{Context, Poll};
 
 use once_cell::sync::Lazy;
-use futures_signals::signal::{Signal, MutableSignal, not};
+use futures_signals::signal::{Signal, Mutable, MutableSignal, not};
 use futures_signals::signal_vec::SignalVec;
 use futures_util::FutureExt;
 use futures_channel::oneshot;
@@ -20,7 +20,7 @@ use crate::traits::*;
 use crate::fragment::{Fragment, FragmentBuilder};
 use crate::operations;
 use crate::operations::{for_each, spawn_future};
-use crate::utils::{EventListener, on, MutableListener, UnwrapJsExt, ValueDiscard, FnDiscard};
+use crate::utils::{EventListener, on, RefCounter, MutableListener, UnwrapJsExt, ValueDiscard, FnDiscard};
 
 #[cfg(doc)]
 use crate::fragment;
@@ -245,7 +245,7 @@ impl WindowSize {
 
 
 thread_local! {
-    static WINDOW_SIZE: MutableListener<WindowSize> = MutableListener::new(WindowSize::new());
+    static WINDOW_SIZE: RefCounter<MutableListener<WindowSize>> = RefCounter::new();
 }
 
 
@@ -278,14 +278,20 @@ impl Drop for WindowSizeSignal {
 /// When the window is resized, it will automatically update with the new size.
 pub fn window_size() -> impl Signal<Item = WindowSize> {
     let signal = WINDOW_SIZE.with(|size| {
-        size.increment(|size| {
-            let size = size.clone();
+        let size = size.increment(|| {
+            let size = Mutable::new(WindowSize::new());
 
-            WINDOW.with(move |window| {
-                on(window, &EventOptions::default(), move |_: crate::events::Resize| {
-                    size.set_neq(WindowSize::new());
+            let listener = {
+                let size = size.clone();
+
+                WINDOW.with(move |window| {
+                    on(window, &EventOptions::default(), move |_: crate::events::Resize| {
+                        size.set_neq(WindowSize::new());
+                    })
                 })
-            })
+            };
+
+            MutableListener::new(size, listener)
         });
 
         size.as_mutable().signal()

src/routing.rs

@@ -6,7 +6,7 @@ use futures_signals::signal::{Mutable, ReadOnlyMutable};
 use crate::bindings;
 use crate::bindings::WINDOW;
 use crate::dom::{Dom, DomBuilder, EventOptions};
-use crate::utils::{EventListener, MutableListener};
+use crate::utils::{EventListener, RefCounter, MutableListener};
 use crate::events;
 
 
@@ -25,41 +25,53 @@ fn change_url(mutable: &Mutable<String>) {
 
 
 thread_local! {
-    // TODO can this be made more efficient ?
-    static CURRENT_URL: MutableListener<String> = MutableListener::new(String::from(bindings::current_url()));
+    static CURRENT_URL: RefCounter<MutableListener<String>> = RefCounter::new();
 }
 
-fn with_url<A, F>(f: F) -> A where F: FnOnce(&Mutable<String>) -> A {
+
+// If the CURRENT_URL is initialized, then run the function `f`
+fn try_url<F>(f: F) where F: FnOnce(&Mutable<String>) {
+    CURRENT_URL.with(|url| {
+        if let Some(url) = &*url.try_borrow() {
+            f(url.as_mutable());
+        }
+    })
+}
+
+
+pub fn url() -> ReadOnlyMutable<String> {
     CURRENT_URL.with(|url| {
         // TODO this needs to call decrement to clean up the listener
-        url.increment(|url| {
-            let url = url.clone();
+        let url = url.increment(|| {
+            // TODO can this be made more efficient ?
+            let url = Mutable::new(String::from(bindings::current_url()));
+
+            let listener = {
+                let url = url.clone();
 
-            WINDOW.with(move |window| {
-                EventListener::new(window, "popstate", &EventOptions::default(), move |_| {
-                    change_url(&url);
+                WINDOW.with(move |window| {
+                    EventListener::new(window, "popstate", &EventOptions::default(), move |_| {
+                        change_url(&url);
+                    })
                 })
-            })
+            };
+
+            MutableListener::new(url, listener)
         });
 
-        f(url.as_mutable())
+        url.as_mutable().read_only()
     })
 }
 
 
-pub fn url() -> ReadOnlyMutable<String> {
-    with_url(|mutable| mutable.read_only())
-}
-
-
 /// Update the current route by adding a new entry to the history.
 #[inline]
 #[track_caller]
 pub fn go_to_url(new_url: &str) {
     // TODO intern ?
     bindings::go_to_url(new_url);
 
-    with_url(change_url);
+    try_url(change_url);
 }
 
 /// Update the current route by replacing the history.
@@ -72,7 +84,7 @@ pub fn replace_url(new_url: &str) {
     // TODO intern ?
     bindings::replace_url(new_url);
 
-    with_url(change_url);
+    try_url(change_url);
 }
 
 #[deprecated(since = "0.5.1", note = "Use the on_click_go_to_url macro instead")]

src/utils.rs

@@ -1,5 +1,5 @@
 use std::borrow::Cow;
-use std::cell::{Cell, RefCell};
+use std::cell::{Cell, RefCell, Ref, RefMut};
 use std::mem::ManuallyDrop;
 
 use wasm_bindgen::{JsValue, UnwrapThrowExt, intern};
@@ -27,6 +27,13 @@ impl<A> RefCounter<A> {
         }
     }
 
+    /// Gives a reference to the data.
+    ///
+    /// It will be `None` if the data hasn't been initialized yet.
+    pub(crate) fn try_borrow(&self) -> Ref<'_, Option<A>> {
+        self.data.borrow()
+    }
+
     /// Decrements the ref count, cleaning up the data if the count is 0
     pub(crate) fn decrement(&self) {
         let counter = self.counter.get().checked_sub(1).unwrap();
@@ -44,47 +51,37 @@ impl<A> RefCounter<A> {
     /// If the [`RefCounter`] hasn't been initialized, it is initialized with the `FnOnce` closure.
     ///
     /// Regardless of whether it is initialized, it increments the ref count.
-    pub(crate) fn increment<F>(&self, f: F) where F: FnOnce() -> A {
-        {
-            let mut lock = self.data.borrow_mut();
+    pub(crate) fn increment<F>(&self, f: F) -> RefMut<'_, A> where F: FnOnce() -> A {
+        let mut lock = self.data.borrow_mut();
 
-            if lock.is_none() {
-                *lock = Some(f());
-            }
+        if lock.is_none() {
+            *lock = Some(f());
         }
 
         let counter = self.counter.get().checked_add(1).unwrap();
         self.counter.set(counter);
+
+        RefMut::map(lock, |data| data.as_mut().unwrap())
     }
 }
 
 
 pub(crate) struct MutableListener<A> {
     mutable: Mutable<A>,
-    counter: RefCounter<DiscardOnDrop<EventListener>>,
+    listener: DiscardOnDrop<EventListener>,
 }
 
 impl<A> MutableListener<A> {
-    pub(crate) fn new(value: A) -> Self {
+    pub(crate) fn new(mutable: Mutable<A>, listener: EventListener) -> Self {
         Self {
-            mutable: Mutable::new(value),
-            counter: RefCounter::new(),
+            mutable,
+            listener: DiscardOnDrop::new(listener),
         }
     }
 
     pub(crate) fn as_mutable(&self) -> &Mutable<A> {
         &self.mutable
     }
-
-    pub(crate) fn increment<F>(&self, f: F) where F: FnOnce(&Mutable<A>) -> EventListener {
-        self.counter.increment(|| {
-            DiscardOnDrop::new(f(&self.mutable))
-        });
-    }
-
-    pub(crate) fn decrement(&self) {
-        self.counter.decrement();
-    }
 }