Explorar el Código

Merge pull request #1187 from Niedzwiedzw/feature/use-shared-state-better-diagnostics

better diagnostics for use_shared_state
Jonathan Kelley hace 1 año
padre
commit
4cd2c0f35f

+ 2 - 1
Cargo.toml

@@ -81,6 +81,7 @@ futures-util = { version = "0.3", default-features = false }
 rustc-hash = "1.1.0"
 wasm-bindgen = "0.2.87"
 html_parser = "0.7.0"
+thiserror = "1.0.40"
 
 # This is a "virtual package"
 # It is not meant to be published, but is used so "cargo run --example XYZ" works properly
@@ -117,6 +118,6 @@ rand = { version = "0.8.4", features = ["small_rng"] }
 tokio = { version = "1.16.1", features = ["full"] }
 reqwest = { version = "0.11.9", features = ["json"] }
 fern = { version = "0.6.0", features = ["colored"] }
-thiserror = "1.0.30"
 env_logger = "0.10.0"
 simple_logger = "4.0.0"
+thiserror = { workspace = true }

+ 1 - 1
packages/cli/Cargo.toml

@@ -10,7 +10,7 @@ license = "MIT OR Apache-2.0"
 [dependencies]
 # cli core
 clap = { version = "4.2", features = ["derive"] }
-thiserror = "1.0.30"
+thiserror = { workspace = true }
 wasm-bindgen-cli-support = "0.2"
 colored = "2.0.0"
 

+ 1 - 1
packages/desktop/Cargo.toml

@@ -17,7 +17,7 @@ dioxus-hot-reload = { workspace = true, optional = true }
 
 serde = "1.0.136"
 serde_json = "1.0.79"
-thiserror = "1.0.30"
+thiserror = { workspace = true }
 log = { workspace = true }
 wry = { version = "0.28.0" }
 futures-channel = { workspace = true }

+ 1 - 1
packages/dioxus/Cargo.toml

@@ -33,7 +33,7 @@ futures-util = { workspace = true }
 log = { workspace = true }
 rand = { version = "0.8.4", features = ["small_rng"] }
 criterion = "0.3.5"
-thiserror = "1.0.30"
+thiserror = { workspace = true }
 env_logger = "0.10.0"
 tokio = { workspace = true, features = ["full"] }
 # dioxus-edit-stream = { workspace = true }

+ 1 - 1
packages/fullstack/Cargo.toml

@@ -39,7 +39,7 @@ dioxus-router = { workspace = true, optional = true }
 
 log = { workspace = true }
 once_cell = "1.17.1"
-thiserror = "1.0.40"
+thiserror = { workspace = true }
 tokio = { workspace = true, features = ["full"], optional = true }
 object-pool = "0.5.4"
 anymap = "0.12.1"

+ 5 - 1
packages/hooks/Cargo.toml

@@ -9,12 +9,16 @@ repository = "https://github.com/DioxusLabs/dioxus/"
 homepage = "https://dioxuslabs.com"
 keywords = ["dom", "ui", "gui", "react"]
 
+[features]
+default = []
+nightly-features = []
 
 [dependencies]
 dioxus-core = { workspace = true }
 futures-channel = { workspace = true }
 log = { workspace = true }
-
+thiserror = { workspace = true }
+debug-cell = { git = "https://github.com/Niedzwiedzw/debug-cell", rev = "3352a1f8aff19f56f5e3b2018200a3338fd43d2e" } # waiting for the merge / official DioxusLabs fork
 
 [dev-dependencies]
 futures-util = { workspace = true, default-features = false }

+ 2 - 0
packages/hooks/src/lib.rs

@@ -1,3 +1,5 @@
+#![cfg_attr(feature = "nightly-features", feature(debug_refcell))]
+
 #[macro_export]
 /// A helper macro for using hooks and properties in async environements.
 ///

+ 148 - 12
packages/hooks/src/use_shared_state.rs

@@ -1,11 +1,79 @@
+use self::error::{UseSharedStateError, UseSharedStateResult};
 use dioxus_core::{ScopeId, ScopeState};
-use std::{
-    cell::{Ref, RefCell, RefMut},
-    collections::HashSet,
-    rc::Rc,
-    sync::Arc,
+use std::{collections::HashSet, rc::Rc, sync::Arc};
+
+#[cfg(debug_assertions)]
+pub use debug_cell::{
+    error::{BorrowError, BorrowMutError},
+    Ref, RefCell, RefMut,
 };
 
+#[cfg(not(debug_assertions))]
+pub use std::cell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut};
+
+#[macro_export]
+macro_rules! debug_location {
+    () => {{
+        #[cfg(debug_assertions)]
+        {
+            std::panic::Location::caller()
+        }
+        #[cfg(not(debug_assertions))]
+        {
+            ()
+        }
+    }};
+}
+
+pub mod error {
+    fn locations_display(locations: &[&'static std::panic::Location<'static>]) -> String {
+        locations
+            .iter()
+            .map(|location| format!(" - {location}"))
+            .collect::<Vec<_>>()
+            .join("\n")
+    }
+    #[derive(thiserror::Error, Debug)]
+    pub enum UseSharedStateError {
+        #[cfg_attr(
+            debug_assertions,
+            error(
+                "[{0}] {1} is already borrowed at, so it cannot be borrowed mutably. Previous borrows:\n[{2}]\n\n",
+                .source.attempted_at,
+                .type_name,
+                locations_display(&.source.already_borrowed_at)
+            )
+         )]
+        #[cfg_attr(
+            not(debug_assertions),
+            error("{type_name} is already borrowed, so it cannot be borrowed mutably. (More detail available in debug mode)")
+        )]
+        AlreadyBorrowed {
+            source: super::BorrowMutError,
+            type_name: &'static str,
+        },
+        #[cfg_attr(
+            debug_assertions,
+            error(
+                "[{0}] {1} is already borrowed mutably at [{2}], so it cannot be borrowed anymore.",
+                .source.attempted_at,
+                .type_name,
+                locations_display(&.source.already_borrowed_at)
+            )
+         )]
+        #[cfg_attr(
+            not(debug_assertions),
+            error("{type_name} is already borrowed mutably, so it cannot be borrowed anymore. (More detail available in debug mode)")
+        )]
+        AlreadyBorrowedMutably {
+            source: super::BorrowError,
+            type_name: &'static str,
+        },
+    }
+
+    pub type UseSharedStateResult<T> = Result<T, UseSharedStateError>;
+}
+
 type ProvidedState<T> = Rc<RefCell<ProvidedStateInner<T>>>;
 
 // Tracks all the subscribers to a shared State
@@ -95,7 +163,7 @@ pub fn use_shared_state<T: 'static>(cx: &ScopeState) -> Option<&UseSharedState<T
 
         root.borrow_mut().consumers.insert(scope_id);
 
-        let state = UseSharedState { inner: root };
+        let state = UseSharedState::new(root);
         let owner = UseSharedStateOwner { state, scope_id };
         Some(owner)
     });
@@ -122,29 +190,97 @@ pub struct UseSharedState<T> {
 }
 
 impl<T> UseSharedState<T> {
+    fn new(inner: Rc<RefCell<ProvidedStateInner<T>>>) -> Self {
+        Self { inner }
+    }
+
     /// Notify all consumers of the state that it has changed. (This is called automatically when you call "write")
     pub fn notify_consumers(&self) {
         self.inner.borrow_mut().notify_consumers();
     }
 
+    /// Try reading the shared state
+    #[cfg_attr(debug_assertions, track_caller)]
+    #[cfg_attr(debug_assertions, inline(never))]
+    pub fn try_read(&self) -> UseSharedStateResult<Ref<'_, T>> {
+        match self.inner.try_borrow() {
+            Ok(value) => Ok(Ref::map(value, |inner| &inner.value)),
+            Err(source) => Err(UseSharedStateError::AlreadyBorrowedMutably {
+                source,
+                type_name: std::any::type_name::<Self>(),
+            }),
+        }
+    }
+
     /// Read the shared value
+    #[cfg_attr(debug_assertions, track_caller)]
+    #[cfg_attr(debug_assertions, inline(never))]
     pub fn read(&self) -> Ref<'_, T> {
-        Ref::map(self.inner.borrow(), |inner| &inner.value)
+        match self.try_read() {
+            Ok(value) => value,
+            Err(message) => panic!(
+                "Reading the shared state failed: {}\n({:?})",
+                message, message
+            ),
+        }
+    }
+
+    /// Try writing the shared state
+    #[cfg_attr(debug_assertions, track_caller)]
+    #[cfg_attr(debug_assertions, inline(never))]
+    pub fn try_write(&self) -> UseSharedStateResult<RefMut<'_, T>> {
+        match self.inner.try_borrow_mut() {
+            Ok(mut value) => {
+                value.notify_consumers();
+                Ok(RefMut::map(value, |inner| &mut inner.value))
+            }
+            Err(source) => Err(UseSharedStateError::AlreadyBorrowed {
+                source,
+                type_name: std::any::type_name::<Self>(),
+            }),
+        }
     }
 
     /// Calling "write" will force the component to re-render
     ///
     ///
     // TODO: We prevent unncessary notifications only in the hook, but we should figure out some more global lock
+    #[cfg_attr(debug_assertions, track_caller)]
+    #[cfg_attr(debug_assertions, inline(never))]
     pub fn write(&self) -> RefMut<'_, T> {
-        let mut value = self.inner.borrow_mut();
-        value.notify_consumers();
-        RefMut::map(value, |inner| &mut inner.value)
+        match self.try_write() {
+            Ok(value) => value,
+            Err(message) => panic!(
+                "Writing to shared state failed: {}\n({:?})",
+                message, message
+            ),
+        }
+    }
+
+    /// Tries writing the value without forcing a re-render
+    #[cfg_attr(debug_assertions, track_caller)]
+    #[cfg_attr(debug_assertions, inline(never))]
+    pub fn try_write_silent(&self) -> UseSharedStateResult<RefMut<'_, T>> {
+        match self.inner.try_borrow_mut() {
+            Ok(value) => Ok(RefMut::map(value, |inner| &mut inner.value)),
+            Err(source) => Err(UseSharedStateError::AlreadyBorrowed {
+                source,
+                type_name: std::any::type_name::<Self>(),
+            }),
+        }
     }
 
-    /// Allows the ability to write the value without forcing a re-render
+    /// Writes the value without forcing a re-render
+    #[cfg_attr(debug_assertions, track_caller)]
+    #[cfg_attr(debug_assertions, inline(never))]
     pub fn write_silent(&self) -> RefMut<'_, T> {
-        RefMut::map(self.inner.borrow_mut(), |inner| &mut inner.value)
+        match self.try_write_silent() {
+            Ok(value) => value,
+            Err(message) => panic!(
+                "Writing to shared state silently failed: {}\n({:?})",
+                message, message
+            ),
+        }
     }
 }
 

+ 1 - 1
packages/liveview/Cargo.toml

@@ -9,7 +9,7 @@ description = "Build server-side apps with Dioxus"
 license = "MIT OR Apache-2.0"
 
 [dependencies]
-thiserror = "1.0.38"
+thiserror = { workspace = true }
 log = { workspace = true }
 slab = { workspace = true }
 futures-util = { workspace = true, default-features = false, features = [

+ 3 - 0
packages/router/Cargo.toml

@@ -14,6 +14,9 @@ dioxus = { workspace = true }
 dioxus-router-macro = { workspace = true }
 gloo = { version = "0.8.0", optional = true }
 log = { workspace = true }
+thiserror = { workspace = true }
+futures-util = { workspace = true }
+serde_urlencoded = { version = "0.7.1", optional = true }
 serde = { version = "1", features = ["derive"], optional = true }
 thiserror = "1.0.37"
 url = "2.3.1"

+ 2 - 0
packages/ssr/Cargo.toml

@@ -21,6 +21,8 @@ tokio = { version = "1.28", features = ["full"] }
 
 [dev-dependencies]
 dioxus = { workspace = true }
+thiserror = { workspace = true }
+log = { workspace = true }
 fern = { version = "0.6.0", features = ["colored"] }
 anyhow = "1.0"
 argh = "0.1.4"