From baa5ea83faffcc4d45b757a0ccd6d8470b3925e5 Mon Sep 17 00:00:00 2001 From: Danik Vitek Date: Fri, 22 Sep 2023 20:31:04 +0300 Subject: [PATCH] fix: reimplement `Oco` cloning (#1749) --- leptos_dom/src/lib.rs | 4 +- leptos_dom/src/ssr.rs | 2 +- leptos_reactive/src/oco.rs | 146 ++++++++++++++----------------------- meta/src/link.rs | 4 +- meta/src/script.rs | 4 +- meta/src/style.rs | 4 +- router/src/hooks.rs | 4 +- 7 files changed, 67 insertions(+), 101 deletions(-) diff --git a/leptos_dom/src/lib.rs b/leptos_dom/src/lib.rs index 63e06a4e7b..bff1c996bb 100644 --- a/leptos_dom/src/lib.rs +++ b/leptos_dom/src/lib.rs @@ -929,7 +929,7 @@ thread_local! { /// This is cached as a thread-local variable, so calling `window()` multiple times /// requires only one call out to JavaScript. pub fn window() -> web_sys::Window { - WINDOW.with(|window| window.clone()) + WINDOW.with(Clone::clone) } /// Returns the [`Document`](https://developer.mozilla.org/en-US/docs/Web/API/Document). @@ -937,7 +937,7 @@ pub fn window() -> web_sys::Window { /// This is cached as a thread-local variable, so calling `document()` multiple times /// requires only one call out to JavaScript. pub fn document() -> web_sys::Document { - DOCUMENT.with(|document| document.clone()) + DOCUMENT.with(Clone::clone) } /// Returns true if running on the server (SSR). diff --git a/leptos_dom/src/ssr.rs b/leptos_dom/src/ssr.rs index 60b1f03cfe..393aba9296 100644 --- a/leptos_dom/src/ssr.rs +++ b/leptos_dom/src/ssr.rs @@ -212,7 +212,7 @@ pub fn render_to_stream_with_prefix_undisposed_with_context_and_block_replacemen .push(async move { (fragment_id, data.out_of_order.await) }); } else { fragments.push(Box::pin(async move { - (fragment_id.clone(), data.out_of_order.await) + (fragment_id, data.out_of_order.await) }) as Pin>>); } diff --git a/leptos_reactive/src/oco.rs b/leptos_reactive/src/oco.rs index 1c4d0d96df..3731f64b1a 100644 --- a/leptos_reactive/src/oco.rs +++ b/leptos_reactive/src/oco.rs @@ -32,7 +32,7 @@ pub enum Oco<'a, T: ?Sized + ToOwned + 'a> { Owned(::Owned), } -impl Oco<'_, T> { +impl<'a, T: ?Sized + ToOwned> Oco<'a, T> { /// Converts the value into an owned value. pub fn into_owned(self) -> ::Owned { match self { @@ -211,15 +211,16 @@ where } } -// ------------------------------------------------------------------------------------------------------ -// Cloning (has to be implemented manually because of the `Rc: From<&::Owned>` bound) -// ------------------------------------------------------------------------------------------------------ - -impl Clone for Oco<'_, str> { +impl<'a, T> Clone for Oco<'a, T> +where + T: ?Sized + ToOwned + 'a, + for<'b> Rc: From<&'b T>, +{ /// Returns a new [`Oco`] with the same value as this one. /// If the value is [`Oco::Owned`], this will convert it into /// [`Oco::Counted`], so that the next clone will be O(1). /// # Examples + /// [`String`] : /// ``` /// # use leptos_reactive::oco::Oco; /// let oco = Oco::::Owned("Hello".to_string()); @@ -227,106 +228,48 @@ impl Clone for Oco<'_, str> { /// assert_eq!(oco, oco2); /// assert!(oco2.is_counted()); /// ``` - fn clone(&self) -> Self { - match self { - Oco::Borrowed(v) => Oco::Borrowed(v), - Oco::Counted(v) => Oco::Counted(v.clone()), - Oco::Owned(v) => Oco::Counted(Rc::::from(v.as_str())), - } - } -} - -impl Clone for Oco<'_, CStr> { - /// Returns a new [`Oco`] with the same value as this one. - /// If the value is [`Oco::Owned`], this will convert it into - /// [`Oco::Counted`], so that the next clone will be O(1). - /// # Examples - /// ``` - /// # use leptos_reactive::oco::Oco; - /// use std::ffi::CStr; - /// - /// let oco = Oco::::Owned( - /// CStr::from_bytes_with_nul(b"Hello\0").unwrap().to_owned(), - /// ); - /// let oco2 = oco.clone(); - /// assert_eq!(oco, oco2); - /// assert!(oco2.is_counted()); - /// ``` - fn clone(&self) -> Self { - match self { - Oco::Borrowed(v) => Oco::Borrowed(v), - Oco::Counted(v) => Oco::Counted(v.clone()), - Oco::Owned(v) => Oco::Counted(Rc::::from(v.as_c_str())), - } - } -} - -impl Clone for Oco<'_, OsStr> { - /// Returns a new [`Oco`] with the same value as this one. - /// If the value is [`Oco::Owned`], this will convert it into - /// [`Oco::Counted`], so that the next clone will be O(1). - /// # Examples - /// ``` - /// # use leptos_reactive::oco::Oco; - /// use std::ffi::OsStr; - /// - /// let oco = Oco::::Owned(OsStr::new("Hello").to_owned()); - /// let oco2 = oco.clone(); - /// assert_eq!(oco, oco2); - /// assert!(oco2.is_counted()); - /// ``` - fn clone(&self) -> Self { - match self { - Oco::Borrowed(v) => Oco::Borrowed(v), - Oco::Counted(v) => Oco::Counted(v.clone()), - Oco::Owned(v) => Oco::Counted(Rc::::from(v.as_os_str())), - } - } -} - -impl Clone for Oco<'_, Path> { - /// Returns a new [`Oco`] with the same value as this one. - /// If the value is [`Oco::Owned`], this will convert it into - /// [`Oco::Counted`], so that the next clone will be O(1). - /// # Examples + /// [`Vec`] : /// ``` /// # use leptos_reactive::oco::Oco; - /// use std::path::Path; - /// - /// let oco = Oco::::Owned(Path::new("Hello").to_owned()); + /// let oco = Oco::<[u8]>::Owned(b"Hello".to_vec()); /// let oco2 = oco.clone(); /// assert_eq!(oco, oco2); /// assert!(oco2.is_counted()); /// ``` fn clone(&self) -> Self { match self { - Oco::Borrowed(v) => Oco::Borrowed(v), - Oco::Counted(v) => Oco::Counted(v.clone()), - Oco::Owned(v) => Oco::Counted(Rc::::from(v.as_path())), + Self::Borrowed(v) => Self::Borrowed(v), + Self::Counted(v) => Self::Counted(Rc::clone(v)), + Self::Owned(v) => Self::Counted(Rc::from(v.borrow())), } } } -impl Clone for Oco<'_, [T]> +impl<'a, T> Oco<'a, T> where - [T]: ToOwned>, + T: ?Sized + ToOwned + 'a, + for<'b> Rc: From<&'b T>, { - /// Returns a new [`Oco`] with the same value as this one. - /// If the value is [`Oco::Owned`], this will convert it into - /// [`Oco::Counted`], so that the next clone will be O(1). + /// Clones the value with inplace conversion into [`Oco::Counted`] if it + /// was previously [`Oco::Owned`]. /// # Examples /// ``` /// # use leptos_reactive::oco::Oco; - /// let oco = Oco::<[i32]>::Owned(vec![1, 2, 3]); - /// let oco2 = oco.clone(); - /// assert_eq!(oco, oco2); + /// let mut oco1 = Oco::::Owned("Hello".to_string()); + /// let oco2 = oco1.clone_inplace(); + /// assert_eq!(oco1, oco2); + /// assert!(oco1.is_counted()); /// assert!(oco2.is_counted()); /// ``` - fn clone(&self) -> Self { - match self { - Oco::Borrowed(v) => Oco::Borrowed(v), - Oco::Counted(v) => Oco::Counted(v.clone()), - Oco::Owned(v) => Oco::Counted(Rc::<[T]>::from(v.as_slice())), + pub fn clone_inplace(&mut self) -> Self { + match &*self { + Self::Borrowed(v) => Self::Borrowed(v), + Self::Counted(v) => Self::Counted(Rc::clone(v)), + Self::Owned(v) => { + let rc = Rc::from(v.borrow()); + *self = Self::Counted(rc.clone()); + Self::Counted(rc) + } } } } @@ -689,23 +632,46 @@ mod tests { } #[test] - fn cloned_owned_string_should_become_counted_str() { + fn cloned_owned_string_should_make_counted_str() { let s: Oco = Oco::Owned(String::from("hello")); assert!(s.clone().is_counted()); } #[test] - fn cloned_borrowed_str_should_remain_borrowed_str() { + fn cloned_borrowed_str_should_make_borrowed_str() { let s: Oco = Oco::Borrowed("hello"); assert!(s.clone().is_borrowed()); } #[test] - fn cloned_counted_str_should_remain_counted_str() { + fn cloned_counted_str_should_make_counted_str() { let s: Oco = Oco::Counted(Rc::from("hello")); assert!(s.clone().is_counted()); } + #[test] + fn cloned_inplace_owned_string_should_make_counted_str_and_become_counted() + { + let mut s: Oco = Oco::Owned(String::from("hello")); + assert!(s.clone_inplace().is_counted()); + assert!(s.is_counted()); + } + + #[test] + fn cloned_inplace_borrowed_str_should_make_borrowed_str_and_remain_borrowed( + ) { + let mut s: Oco = Oco::Borrowed("hello"); + assert!(s.clone_inplace().is_borrowed()); + assert!(s.is_borrowed()); + } + + #[test] + fn cloned_inplace_counted_str_should_make_counted_str_and_remain_counted() { + let mut s: Oco = Oco::Counted(Rc::from("hello")); + assert!(s.clone_inplace().is_counted()); + assert!(s.is_counted()); + } + #[test] fn serialization_works() { let s = serde_json::to_string(&Oco::Borrowed("foo")) diff --git a/meta/src/link.rs b/meta/src/link.rs index f8c074a3a7..3e0d14491f 100644 --- a/meta/src/link.rs +++ b/meta/src/link.rs @@ -82,11 +82,11 @@ pub fn Link( ) -> impl IntoView { let meta = use_head(); let next_id = meta.tags.get_next_id(); - let id: Oco<'static, str> = + let mut id: Oco<'static, str> = id.unwrap_or_else(|| format!("leptos-link-{}", next_id.0).into()); let builder_el = leptos::leptos_dom::html::as_meta_tag({ - let id = id.clone(); + let id = id.clone_inplace(); move || { leptos::leptos_dom::html::link() .attr("id", id) diff --git a/meta/src/script.rs b/meta/src/script.rs index 27e0fb96c2..e135a31854 100644 --- a/meta/src/script.rs +++ b/meta/src/script.rs @@ -64,11 +64,11 @@ pub fn Script( ) -> impl IntoView { let meta = use_head(); let next_id = meta.tags.get_next_id(); - let id: Oco<'static, str> = + let mut id: Oco<'static, str> = id.unwrap_or_else(|| format!("leptos-link-{}", next_id.0).into()); let builder_el = leptos::leptos_dom::html::as_meta_tag({ - let id = id.clone(); + let id = id.clone_inplace(); move || { leptos::leptos_dom::html::script() .attr("id", id) diff --git a/meta/src/style.rs b/meta/src/style.rs index 81bf9c1770..3b1a3a185a 100644 --- a/meta/src/style.rs +++ b/meta/src/style.rs @@ -43,11 +43,11 @@ pub fn Style( ) -> impl IntoView { let meta = use_head(); let next_id = meta.tags.get_next_id(); - let id: Oco<'static, str> = + let mut id: Oco<'static, str> = id.unwrap_or_else(|| format!("leptos-link-{}", next_id.0).into()); let builder_el = leptos::leptos_dom::html::as_meta_tag({ - let id = id.clone(); + let id = id.clone_inplace(); move || { leptos::leptos_dom::html::style() .attr("id", id) diff --git a/router/src/hooks.rs b/router/src/hooks.rs index 3a6953325e..51c33cb3a9 100644 --- a/router/src/hooks.rs +++ b/router/src/hooks.rs @@ -51,13 +51,13 @@ pub fn create_query_signal( where T: FromStr + ToString + PartialEq, { - let key = key.into(); + let mut key: Oco<'static, str> = key.into(); let query_map = use_query_map(); let navigate = use_navigate(); let location = use_location(); let get = create_memo({ - let key = key.clone(); + let key = key.clone_inplace(); move |_| { query_map .with(|map| map.get(&key).and_then(|value| value.parse().ok()))