From 5bcfd0e2ec8d697f7bb7a9b2d890ec831d59d5d6 Mon Sep 17 00:00:00 2001 From: Finn Bear Date: Thu, 12 Sep 2024 14:17:22 -0700 Subject: [PATCH 1/4] yew-router: Dynamic basename. --- packages/yew-router/src/lib.rs | 4 ++ packages/yew-router/src/navigator.rs | 2 +- packages/yew-router/src/router.rs | 33 +++++++++-- packages/yew-router/tests/link.rs | 83 +++++++++++++++++++++++++--- 4 files changed, 108 insertions(+), 14 deletions(-) diff --git a/packages/yew-router/src/lib.rs b/packages/yew-router/src/lib.rs index 1cf262e21d8..efb6e7841df 100644 --- a/packages/yew-router/src/lib.rs +++ b/packages/yew-router/src/lib.rs @@ -88,6 +88,10 @@ pub mod history { AnyHistory, BrowserHistory, HashHistory, History, HistoryError, HistoryResult, Location, MemoryHistory, }; + + pub(crate) mod query { + pub(crate) use gloo::history::query::Raw; + } } pub mod prelude { diff --git a/packages/yew-router/src/navigator.rs b/packages/yew-router/src/navigator.rs index 48345609f83..dec228821b8 100644 --- a/packages/yew-router/src/navigator.rs +++ b/packages/yew-router/src/navigator.rs @@ -159,7 +159,7 @@ impl Navigator { pub(crate) fn prefix_basename<'a>(&self, route_s: &'a str) -> Cow<'a, str> { match self.basename() { Some(base) => { - if route_s.is_empty() && route_s.is_empty() { + if base.is_empty() && route_s.is_empty() { Cow::from("/") } else { Cow::from(format!("{base}{route_s}")) diff --git a/packages/yew-router/src/router.rs b/packages/yew-router/src/router.rs index b3becda9ea1..f0a1a61fa38 100644 --- a/packages/yew-router/src/router.rs +++ b/packages/yew-router/src/router.rs @@ -1,9 +1,11 @@ //! Router Component. +use std::borrow::Cow; use std::rc::Rc; use yew::prelude::*; use yew::virtual_dom::AttrValue; +use crate::history::query::Raw; use crate::history::{AnyHistory, BrowserHistory, HashHistory, History, Location}; use crate::navigator::Navigator; use crate::utils::{base_url, strip_slash_suffix}; @@ -72,16 +74,37 @@ fn base_router(props: &RouterProps) -> Html { basename, } = props.clone(); + let basename = basename.map(|m| strip_slash_suffix(&m).to_string()); + let navigator = Navigator::new(history.clone(), basename.clone()); + + let old_basename = use_state_eq(|| Option::::None); + if basename != *old_basename { + // If `old_basename` is `Some`, path is probably prefixed with `old_basename`. + // If `old_basename` is `None`, path may or may not be prefixed with the new `basename`, + // depending on whether this is the first render. + let old_navigator = Navigator::new( + history.clone(), + old_basename.as_ref().or(basename.as_ref()).cloned(), + ); + old_basename.set(basename.clone()); + let location = history.location(); + let stripped = old_navigator.strip_basename(Cow::from(location.path())); + let prefixed = navigator.prefix_basename(&stripped); + + if prefixed != location.path() { + history + .replace_with_query(prefixed, Raw(location.query_str())) + .unwrap_or_else(|never| match never {}); + } + } + + let navi_ctx = NavigatorContext { navigator }; + let loc_ctx = use_reducer(|| LocationContext { location: history.location(), ctr: 0, }); - let basename = basename.map(|m| strip_slash_suffix(&m).to_string()); - let navi_ctx = NavigatorContext { - navigator: Navigator::new(history.clone(), basename), - }; - { let loc_ctx_dispatcher = loc_ctx.dispatcher(); diff --git a/packages/yew-router/tests/link.rs b/packages/yew-router/tests/link.rs index 50510b79242..47552751ef3 100644 --- a/packages/yew-router/tests/link.rs +++ b/packages/yew-router/tests/link.rs @@ -89,12 +89,11 @@ fn root_for_browser_router() -> Html { } } -#[test] async fn link_in_browser_router() { let div = gloo::utils::document().create_element("div").unwrap(); let _ = div.set_attribute("id", "browser-router"); let _ = gloo::utils::body().append_child(&div); - yew::Renderer::::with_root(div).render(); + let handle = yew::Renderer::::with_root(div).render(); sleep(Duration::ZERO).await; @@ -113,26 +112,43 @@ async fn link_in_browser_router() { "/search?q=Rust&lang=en_US", link_href("#browser-router ul > li.search-q-lang > a") ); + + handle.destroy(); +} + +#[derive(PartialEq, Properties)] +struct BasenameProps { + basename: Option, } #[function_component(RootForBasename)] -fn root_for_basename() -> Html { +fn root_for_basename(props: &BasenameProps) -> Html { html! { - + } } -#[test] async fn link_with_basename() { let div = gloo::utils::document().create_element("div").unwrap(); let _ = div.set_attribute("id", "with-basename"); let _ = gloo::utils::body().append_child(&div); - yew::Renderer::::with_root(div).render(); + let mut handle = yew::Renderer::::with_root_and_props( + div, + BasenameProps { + basename: Some("/base/".to_owned()), + }, + ) + .render(); sleep(Duration::ZERO).await; + assert_eq!( + "/base/", + gloo::utils::window().location().pathname().unwrap() + ); + assert_eq!("/base/posts", link_href("#with-basename ul > li.posts > a")); assert_eq!( "/base/posts?page=2", @@ -151,6 +167,48 @@ async fn link_with_basename() { "/base/search?q=Rust&lang=en_US", link_href("#with-basename ul > li.search-q-lang > a") ); + + // Some(a) -> Some(b) + handle.update(BasenameProps { + basename: Some("/bayes/".to_owned()), + }); + + sleep(Duration::ZERO).await; + + assert_eq!( + "/bayes/", + gloo::utils::window().location().pathname().unwrap() + ); + + assert_eq!( + "/bayes/posts", + link_href("#with-basename ul > li.posts > a") + ); + + // Some -> None + handle.update(BasenameProps { basename: None }); + + sleep(Duration::ZERO).await; + + assert_eq!("/", gloo::utils::window().location().pathname().unwrap()); + + assert_eq!("/posts", link_href("#with-basename ul > li.posts > a")); + + // None -> Some + handle.update(BasenameProps { + basename: Some("/bass/".to_string()), + }); + + sleep(Duration::ZERO).await; + + assert_eq!( + "/bass/", + gloo::utils::window().location().pathname().unwrap() + ); + + assert_eq!("/bass/posts", link_href("#with-basename ul > li.posts > a")); + + handle.destroy(); } #[function_component(RootForHashRouter)] @@ -162,12 +220,11 @@ fn root_for_hash_router() -> Html { } } -#[test] async fn link_in_hash_router() { let div = gloo::utils::document().create_element("div").unwrap(); let _ = div.set_attribute("id", "hash-router"); let _ = gloo::utils::body().append_child(&div); - yew::Renderer::::with_root(div).render(); + let handle = yew::Renderer::::with_root(div).render(); sleep(Duration::ZERO).await; @@ -186,4 +243,14 @@ async fn link_in_hash_router() { "#/search?q=Rust&lang=en_US", link_href("#hash-router ul > li.search-q-lang > a") ); + + handle.destroy(); +} + +// These cannot be run in concurrently because they all read/write the URL. +#[test] +async fn sequential_tests() { + link_in_hash_router().await; + link_in_browser_router().await; + link_with_basename().await; } From 3acd667fffecf67e25726853ac74c5d13fe55f8a Mon Sep 17 00:00:00 2001 From: Finn Bear Date: Thu, 19 Sep 2024 10:11:57 -0700 Subject: [PATCH 2/4] Revisions. --- packages/yew-router/src/lib.rs | 4 ---- packages/yew-router/src/router.rs | 7 ++++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/yew-router/src/lib.rs b/packages/yew-router/src/lib.rs index efb6e7841df..1cf262e21d8 100644 --- a/packages/yew-router/src/lib.rs +++ b/packages/yew-router/src/lib.rs @@ -88,10 +88,6 @@ pub mod history { AnyHistory, BrowserHistory, HashHistory, History, HistoryError, HistoryResult, Location, MemoryHistory, }; - - pub(crate) mod query { - pub(crate) use gloo::history::query::Raw; - } } pub mod prelude { diff --git a/packages/yew-router/src/router.rs b/packages/yew-router/src/router.rs index f0a1a61fa38..1eea8abec8a 100644 --- a/packages/yew-router/src/router.rs +++ b/packages/yew-router/src/router.rs @@ -2,10 +2,10 @@ use std::borrow::Cow; use std::rc::Rc; +use gloo::history::query::Raw; use yew::prelude::*; use yew::virtual_dom::AttrValue; -use crate::history::query::Raw; use crate::history::{AnyHistory, BrowserHistory, HashHistory, History, Location}; use crate::navigator::Navigator; use crate::utils::{base_url, strip_slash_suffix}; @@ -77,7 +77,8 @@ fn base_router(props: &RouterProps) -> Html { let basename = basename.map(|m| strip_slash_suffix(&m).to_string()); let navigator = Navigator::new(history.clone(), basename.clone()); - let old_basename = use_state_eq(|| Option::::None); + let old_basename = use_mut_ref(|| Option::::None); + let mut old_basename = old_basename.borrow_mut(); if basename != *old_basename { // If `old_basename` is `Some`, path is probably prefixed with `old_basename`. // If `old_basename` is `None`, path may or may not be prefixed with the new `basename`, @@ -86,7 +87,7 @@ fn base_router(props: &RouterProps) -> Html { history.clone(), old_basename.as_ref().or(basename.as_ref()).cloned(), ); - old_basename.set(basename.clone()); + *old_basename = basename.clone(); let location = history.location(); let stripped = old_navigator.strip_basename(Cow::from(location.path())); let prefixed = navigator.prefix_basename(&stripped); From 8aa8588b696f88450c7cc9fdc693de2b12f5234d Mon Sep 17 00:00:00 2001 From: Finn Bear Date: Mon, 23 Sep 2024 11:13:25 -0700 Subject: [PATCH 3/4] Test location.path and navigator.basename match expectations at each step. --- packages/yew-router/src/router.rs | 7 ++++- packages/yew-router/tests/link.rs | 50 +++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/packages/yew-router/src/router.rs b/packages/yew-router/src/router.rs index 1eea8abec8a..0c4b4dedf4c 100644 --- a/packages/yew-router/src/router.rs +++ b/packages/yew-router/src/router.rs @@ -74,7 +74,7 @@ fn base_router(props: &RouterProps) -> Html { basename, } = props.clone(); - let basename = basename.map(|m| strip_slash_suffix(&m).to_string()); + let basename = basename.map(|m| strip_slash_suffix(&m).to_owned()); let navigator = Navigator::new(history.clone(), basename.clone()); let old_basename = use_mut_ref(|| Option::::None); @@ -96,6 +96,11 @@ fn base_router(props: &RouterProps) -> Html { history .replace_with_query(prefixed, Raw(location.query_str())) .unwrap_or_else(|never| match never {}); + } else { + // Reaching here is possible if the page loads with the correct path, including the + // initial basename. In that case, the new basename would be stripped and then + // prefixed right back. While replacing the history would probably be harmless, + // we might as well avoid doing it. } } diff --git a/packages/yew-router/tests/link.rs b/packages/yew-router/tests/link.rs index 47552751ef3..8ff7ff30ffb 100644 --- a/packages/yew-router/tests/link.rs +++ b/packages/yew-router/tests/link.rs @@ -1,3 +1,4 @@ +use std::sync::atomic::{AtomicU8, Ordering}; use std::time::Duration; use serde::{Deserialize, Serialize}; @@ -47,8 +48,20 @@ enum Routes { Search, } +#[derive(PartialEq, Properties)] +struct NavigationMenuProps { + #[prop_or(None)] + assertion: Option, +} + #[function_component(NavigationMenu)] -fn navigation_menu() -> Html { +fn navigation_menu(props: &NavigationMenuProps) -> Html { + let navigator = use_navigator().unwrap(); + let location = use_location().unwrap(); + if let Some(assertion) = props.assertion { + assertion(&navigator, &location); + } + html! {
  • @@ -119,13 +132,14 @@ async fn link_in_browser_router() { #[derive(PartialEq, Properties)] struct BasenameProps { basename: Option, + assertion: fn(&Navigator, &Location), } #[function_component(RootForBasename)] fn root_for_basename(props: &BasenameProps) -> Html { html! { - + } } @@ -134,10 +148,20 @@ async fn link_with_basename() { let div = gloo::utils::document().create_element("div").unwrap(); let _ = div.set_attribute("id", "with-basename"); let _ = gloo::utils::body().append_child(&div); + let mut handle = yew::Renderer::::with_root_and_props( div, BasenameProps { basename: Some("/base/".to_owned()), + assertion: |navigator, location| { + static TIMES: AtomicU8 = AtomicU8::new(0); + assert!( + TIMES.fetch_add(1, Ordering::Relaxed) < 2, + "expect 2 renders" + ); + assert_eq!(navigator.basename(), Some("/base")); + assert_eq!(location.path(), "/base/"); + }, }, ) .render(); @@ -171,6 +195,12 @@ async fn link_with_basename() { // Some(a) -> Some(b) handle.update(BasenameProps { basename: Some("/bayes/".to_owned()), + assertion: |navigator, location| { + static TIMES: AtomicU8 = AtomicU8::new(0); + assert!(TIMES.fetch_add(1, Ordering::Relaxed) < 1, "expect 1 render"); + assert_eq!(navigator.basename(), Some("/bayes")); + assert_eq!(location.path(), "/bayes/"); + }, }); sleep(Duration::ZERO).await; @@ -186,7 +216,15 @@ async fn link_with_basename() { ); // Some -> None - handle.update(BasenameProps { basename: None }); + handle.update(BasenameProps { + basename: None, + assertion: |navigator, location| { + static TIMES: AtomicU8 = AtomicU8::new(0); + assert!(TIMES.fetch_add(1, Ordering::Relaxed) < 1, "expect 1 render"); + assert_eq!(navigator.basename(), None); + assert_eq!(location.path(), "/"); + }, + }); sleep(Duration::ZERO).await; @@ -197,6 +235,12 @@ async fn link_with_basename() { // None -> Some handle.update(BasenameProps { basename: Some("/bass/".to_string()), + assertion: |navigator, location| { + static TIMES: AtomicU8 = AtomicU8::new(0); + assert!(TIMES.fetch_add(1, Ordering::Relaxed) < 1, "expect 1 render"); + assert_eq!(navigator.basename(), Some("/bass")); + assert_eq!(location.path(), "/bass/"); + }, }); sleep(Duration::ZERO).await; From 21216625f50111b00bb5d6aa0573be86567d0d33 Mon Sep 17 00:00:00 2001 From: Finn Bear Date: Mon, 23 Sep 2024 11:27:27 -0700 Subject: [PATCH 4/4] Better coverage of edge case. --- packages/yew-router/tests/link.rs | 52 +++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/packages/yew-router/tests/link.rs b/packages/yew-router/tests/link.rs index 8ff7ff30ffb..8e3f5bc5f41 100644 --- a/packages/yew-router/tests/link.rs +++ b/packages/yew-router/tests/link.rs @@ -1,6 +1,8 @@ use std::sync::atomic::{AtomicU8, Ordering}; use std::time::Duration; +use gloo::utils::window; +use js_sys::{JsString, Object, Reflect}; use serde::{Deserialize, Serialize}; use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure}; use yew::functional::function_component; @@ -144,7 +146,20 @@ fn root_for_basename(props: &BasenameProps) -> Html { } } -async fn link_with_basename() { +async fn link_with_basename(correct_initial_path: bool) { + if correct_initial_path { + let cookie = Object::new(); + Reflect::set(&cookie, &JsString::from("foo"), &JsString::from("bar")).unwrap(); + window() + .history() + .unwrap() + .replace_state_with_url(&cookie, "", Some("/base/")) + .unwrap(); + } + + static RENDERS: AtomicU8 = AtomicU8::new(0); + RENDERS.store(0, Ordering::Relaxed); + let div = gloo::utils::document().create_element("div").unwrap(); let _ = div.set_attribute("id", "with-basename"); let _ = gloo::utils::body().append_child(&div); @@ -154,11 +169,7 @@ async fn link_with_basename() { BasenameProps { basename: Some("/base/".to_owned()), assertion: |navigator, location| { - static TIMES: AtomicU8 = AtomicU8::new(0); - assert!( - TIMES.fetch_add(1, Ordering::Relaxed) < 2, - "expect 2 renders" - ); + RENDERS.fetch_add(1, Ordering::Relaxed); assert_eq!(navigator.basename(), Some("/base")); assert_eq!(location.path(), "/base/"); }, @@ -168,6 +179,20 @@ async fn link_with_basename() { sleep(Duration::ZERO).await; + if correct_initial_path { + // If the initial path was correct, the router shouldn't have mutated the history. + assert_eq!( + Reflect::get( + &window().history().unwrap().state().unwrap(), + &JsString::from("foo") + ) + .unwrap() + .as_string() + .as_deref(), + Some("bar") + ); + } + assert_eq!( "/base/", gloo::utils::window().location().pathname().unwrap() @@ -196,8 +221,7 @@ async fn link_with_basename() { handle.update(BasenameProps { basename: Some("/bayes/".to_owned()), assertion: |navigator, location| { - static TIMES: AtomicU8 = AtomicU8::new(0); - assert!(TIMES.fetch_add(1, Ordering::Relaxed) < 1, "expect 1 render"); + RENDERS.fetch_add(1, Ordering::Relaxed); assert_eq!(navigator.basename(), Some("/bayes")); assert_eq!(location.path(), "/bayes/"); }, @@ -219,8 +243,7 @@ async fn link_with_basename() { handle.update(BasenameProps { basename: None, assertion: |navigator, location| { - static TIMES: AtomicU8 = AtomicU8::new(0); - assert!(TIMES.fetch_add(1, Ordering::Relaxed) < 1, "expect 1 render"); + RENDERS.fetch_add(1, Ordering::Relaxed); assert_eq!(navigator.basename(), None); assert_eq!(location.path(), "/"); }, @@ -236,8 +259,7 @@ async fn link_with_basename() { handle.update(BasenameProps { basename: Some("/bass/".to_string()), assertion: |navigator, location| { - static TIMES: AtomicU8 = AtomicU8::new(0); - assert!(TIMES.fetch_add(1, Ordering::Relaxed) < 1, "expect 1 render"); + RENDERS.fetch_add(1, Ordering::Relaxed); assert_eq!(navigator.basename(), Some("/bass")); assert_eq!(location.path(), "/bass/"); }, @@ -253,6 +275,9 @@ async fn link_with_basename() { assert_eq!("/bass/posts", link_href("#with-basename ul > li.posts > a")); handle.destroy(); + + // 1 initial, 1 rerender after initial, 3 props changes + assert_eq!(RENDERS.load(Ordering::Relaxed), 5); } #[function_component(RootForHashRouter)] @@ -296,5 +321,6 @@ async fn link_in_hash_router() { async fn sequential_tests() { link_in_hash_router().await; link_in_browser_router().await; - link_with_basename().await; + link_with_basename(false).await; + link_with_basename(true).await; }