diff --git a/examples/router/e2e/tests/router.spec.ts b/examples/router/e2e/tests/router.spec.ts index b90e0d13f6..5fffeb8607 100644 --- a/examples/router/e2e/tests/router.spec.ts +++ b/examples/router/e2e/tests/router.spec.ts @@ -5,6 +5,10 @@ test.describe("Test Router example", () => { await page.goto("/"); }); + test("Starts on correct home page", async({ page }) => { + await expect(page.getByText("Select a contact.")).toBeVisible(); + }); + const links = [ { label: "Bill Smith", url: "/0" }, { label: "Tim Jones", url: "/1" }, diff --git a/examples/ssr_modes/src/app.rs b/examples/ssr_modes/src/app.rs index 9da69a2158..8d167d4f14 100644 --- a/examples/ssr_modes/src/app.rs +++ b/examples/ssr_modes/src/app.rs @@ -9,13 +9,12 @@ use thiserror::Error; pub fn App() -> impl IntoView { // Provides context that manages stylesheets, titles, meta tags, etc. provide_meta_context(); - let fallback = || view! { "Page not found." }.into_view(); view! { - <Router fallback> + <Router> <main> <Routes> // We’ll load the home page with out-of-order streaming and <Suspense/> diff --git a/examples/ssr_modes_axum/src/app.rs b/examples/ssr_modes_axum/src/app.rs index 6b24570f2b..4592375441 100644 --- a/examples/ssr_modes_axum/src/app.rs +++ b/examples/ssr_modes_axum/src/app.rs @@ -9,13 +9,12 @@ use thiserror::Error; pub fn App() -> impl IntoView { // Provides context that manages stylesheets, titles, meta tags, etc. provide_meta_context(); - let fallback = || view! { "Page not found." }.into_view(); view! { <Stylesheet id="leptos" href="/pkg/ssr_modes.css"/> <Title text="Welcome to Leptos"/> - <Router fallback> + <Router> <main> <Routes> // We’ll load the home page with out-of-order streaming and <Suspense/> diff --git a/examples/todo_app_sqlite/src/todo.rs b/examples/todo_app_sqlite/src/todo.rs index 1f9d2ca18d..74a5f862dc 100644 --- a/examples/todo_app_sqlite/src/todo.rs +++ b/examples/todo_app_sqlite/src/todo.rs @@ -22,6 +22,7 @@ cfg_if! { } } +/// Server functions can be given doc comments. #[server(GetTodos, "/api")] pub async fn get_todos() -> Result<Vec<Todo>, ServerFnError> { // this is just an example of how to access server context injected in the handlers diff --git a/integrations/actix/tests/extract_routes.rs b/integrations/actix/tests/extract_routes.rs deleted file mode 100644 index 67765b187e..0000000000 --- a/integrations/actix/tests/extract_routes.rs +++ /dev/null @@ -1,148 +0,0 @@ -use leptos::*; -use leptos_actix::generate_route_list; -use leptos_router::{Route, Router, Routes, TrailingSlash}; - -#[component] -fn DefaultApp() -> impl IntoView { - let view = || view! { "" }; - view! { - <Router> - <Routes> - <Route path="/foo" view/> - <Route path="/bar/" view/> - <Route path="/baz/:id" view/> - <Route path="/baz/:name/" view/> - <Route path="/baz/*any" view/> - </Routes> - </Router> - } -} - -#[test] -fn test_default_app() { - let routes = generate_route_list(DefaultApp); - - // We still have access to the original (albeit normalized) Leptos paths: - assert_same( - &routes, - |r| r.leptos_path(), - &["/bar", "/baz/*any", "/baz/:id", "/baz/:name", "/foo"], - ); - - // ... But leptos-actix has also reformatted "paths" to work for Actix. - assert_same( - &routes, - |r| r.path(), - &["/bar", "/baz/{id}", "/baz/{name}", "/baz/{tail:.*}", "/foo"], - ); -} - -#[component] -fn ExactApp() -> impl IntoView { - let view = || view! { "" }; - let trailing_slash = TrailingSlash::Exact; - view! { - <Router trailing_slash> - <Routes> - <Route path="/foo" view/> - <Route path="/bar/" view/> - <Route path="/baz/:id" view/> - <Route path="/baz/:name/" view/> - <Route path="/baz/*any" view/> - </Routes> - </Router> - } -} - -#[test] -fn test_exact_app() { - let routes = generate_route_list(ExactApp); - - // In Exact mode, the Leptos paths no longer have their trailing slashes stripped: - assert_same( - &routes, - |r| r.leptos_path(), - &["/bar/", "/baz/*any", "/baz/:id", "/baz/:name/", "/foo"], - ); - - // Actix paths also have trailing slashes as a result: - assert_same( - &routes, - |r| r.path(), - &[ - "/bar/", - "/baz/{id}", - "/baz/{name}/", - "/baz/{tail:.*}", - "/foo", - ], - ); -} - -#[component] -fn RedirectApp() -> impl IntoView { - let view = || view! { "" }; - let trailing_slash = TrailingSlash::Redirect; - view! { - <Router trailing_slash> - <Routes> - <Route path="/foo" view/> - <Route path="/bar/" view/> - <Route path="/baz/:id" view/> - <Route path="/baz/:name/" view/> - <Route path="/baz/*any" view/> - </Routes> - </Router> - } -} - -#[test] -fn test_redirect_app() { - let routes = generate_route_list(RedirectApp); - - assert_same( - &routes, - |r| r.leptos_path(), - &[ - "/bar", - "/bar/", - "/baz/*any", - "/baz/:id", - "/baz/:id/", - "/baz/:name", - "/baz/:name/", - "/foo", - "/foo/", - ], - ); - - // ... But leptos-actix has also reformatted "paths" to work for Actix. - assert_same( - &routes, - |r| r.path(), - &[ - "/bar", - "/bar/", - "/baz/{id}", - "/baz/{id}/", - "/baz/{name}", - "/baz/{name}/", - "/baz/{tail:.*}", - "/foo", - "/foo/", - ], - ); -} - -fn assert_same<'t, T, F, U>( - input: &'t Vec<T>, - mapper: F, - expected_sorted_values: &[U], -) where - F: Fn(&'t T) -> U + 't, - U: Ord + std::fmt::Debug, -{ - let mut values: Vec<U> = input.iter().map(mapper).collect(); - values.sort(); - assert_eq!(values, expected_sorted_values); -} diff --git a/leptos_dom/src/hydration.rs b/leptos_dom/src/hydration.rs index 104ec175f8..91502da241 100644 --- a/leptos_dom/src/hydration.rs +++ b/leptos_dom/src/hydration.rs @@ -53,7 +53,7 @@ mod hydrate_only { } }); - pub static IS_HYDRATING: Cell<bool> = Cell::new(true); + pub static IS_HYDRATING: Cell<bool> = const { Cell::new(true) }; } #[allow(unused)] @@ -133,7 +133,7 @@ mod tests { } } -thread_local!(static ID: RefCell<HydrationKey> = RefCell::new(HydrationKey { outlet: 0, fragment: 0, error: 0, id: 0 })); +thread_local!(static ID: RefCell<HydrationKey> = const {RefCell::new(HydrationKey { outlet: 0, fragment: 0, error: 0, id: 0 })}); /// Control and utility methods for hydration. pub struct HydrationCtx; diff --git a/leptos_dom/src/macro_helpers/tracing_property.rs b/leptos_dom/src/macro_helpers/tracing_property.rs index 55b30611bb..365cef56f2 100644 --- a/leptos_dom/src/macro_helpers/tracing_property.rs +++ b/leptos_dom/src/macro_helpers/tracing_property.rs @@ -117,13 +117,13 @@ fn match_primitive() { assert_eq!(prop, r#"{"name": "test", "value": -1}"#); // f64 - let test = 3.14; + let test = 3.25; let prop = (&&Match { name: stringify! {test}, value: std::cell::Cell::new(Some(&test)), }) .spez(); - assert_eq!(prop, r#"{"name": "test", "value": 3.14}"#); + assert_eq!(prop, r#"{"name": "test", "value": 3.25}"#); // bool let test = true; diff --git a/leptos_reactive/src/diagnostics.rs b/leptos_reactive/src/diagnostics.rs index d026592b51..15e12c144d 100644 --- a/leptos_reactive/src/diagnostics.rs +++ b/leptos_reactive/src/diagnostics.rs @@ -26,7 +26,7 @@ cfg_if::cfg_if! { use std::cell::Cell; thread_local! { - static IS_SPECIAL_ZONE: Cell<bool> = Cell::new(false); + static IS_SPECIAL_ZONE: Cell<bool> = const { Cell::new(false) }; } } } diff --git a/leptos_reactive/src/hydration.rs b/leptos_reactive/src/hydration.rs index 936e68f35f..c2166787ac 100644 --- a/leptos_reactive/src/hydration.rs +++ b/leptos_reactive/src/hydration.rs @@ -332,7 +332,7 @@ impl Default for SharedContext { #[cfg(feature = "experimental-islands")] thread_local! { - pub static NO_HYDRATE: Cell<bool> = Cell::new(true); + pub static NO_HYDRATE: Cell<bool> = const { Cell::new(true) }; } #[cfg(feature = "experimental-islands")] diff --git a/leptos_reactive/src/resource.rs b/leptos_reactive/src/resource.rs index 3d5e89839f..84acc41037 100644 --- a/leptos_reactive/src/resource.rs +++ b/leptos_reactive/src/resource.rs @@ -1518,7 +1518,7 @@ impl<S, T> UnserializableResource for ResourceState<S, T> { } thread_local! { - static SUPPRESS_RESOURCE_LOAD: Cell<bool> = Cell::new(false); + static SUPPRESS_RESOURCE_LOAD: Cell<bool> = const { Cell::new(false) }; } #[doc(hidden)] diff --git a/leptos_reactive/src/stored_value.rs b/leptos_reactive/src/stored_value.rs index 0ec2e95a62..5344831e1d 100644 --- a/leptos_reactive/src/stored_value.rs +++ b/leptos_reactive/src/stored_value.rs @@ -242,7 +242,7 @@ impl<T> StoredValue<T> { with_runtime(|runtime| { let n = { let values = runtime.stored_values.borrow(); - values.get(self.id).map(Rc::clone) + values.get(self.id).cloned() }; if let Some(n) = n { diff --git a/router/src/components/route.rs b/router/src/components/route.rs index d25355cb0b..1dcda396c4 100644 --- a/router/src/components/route.rs +++ b/router/src/components/route.rs @@ -1,7 +1,6 @@ use crate::{ matching::{resolve_path, PathMatch, RouteDefinition, RouteMatch}, ParamsMap, RouterContext, SsrMode, StaticData, StaticMode, StaticParamsMap, - TrailingSlash, }; use leptos::{leptos_dom::Transparent, *}; use std::{ @@ -15,17 +14,7 @@ use std::{ }; thread_local! { - static ROUTE_ID: Cell<usize> = Cell::new(0); -} - -// RouteDefinition.id is `pub` and required to be unique. -// Should we make this public so users can generate unique IDs? -pub(in crate::components) fn new_route_id() -> usize { - ROUTE_ID.with(|id| { - let next = id.get() + 1; - id.set(next); - next - }) + static ROUTE_ID: Cell<usize> = const { Cell::new(0) }; } /// Represents an HTTP method that can be handled by this route. @@ -76,11 +65,6 @@ pub fn Route<E, F, P>( /// accessed with [`use_route_data`](crate::use_route_data). #[prop(optional, into)] data: Option<Loader>, - /// How this route should handle trailing slashes in its path. - /// Overrides any setting applied to [`crate::components::Router`]. - /// Serves as a default for any inner Routes. - #[prop(optional)] - trailing_slash: Option<TrailingSlash>, /// `children` may be empty or include nested routes. #[prop(optional)] children: Option<Children>, @@ -99,7 +83,6 @@ where data, None, None, - trailing_slash, ) } @@ -132,11 +115,6 @@ pub fn ProtectedRoute<P, E, F, C>( /// accessed with [`use_route_data`](crate::use_route_data). #[prop(optional, into)] data: Option<Loader>, - /// How this route should handle trailing slashes in its path. - /// Overrides any setting applied to [`crate::components::Router`]. - /// Serves as a default for any inner Routes. - #[prop(optional)] - trailing_slash: Option<TrailingSlash>, /// `children` may be empty or include nested routes. #[prop(optional)] children: Option<Children>, @@ -165,7 +143,6 @@ where data, None, None, - trailing_slash, ) } @@ -194,11 +171,6 @@ pub fn StaticRoute<E, F, P, S>( /// accessed with [`use_route_data`](crate::use_route_data). #[prop(optional, into)] data: Option<Loader>, - /// How this route should handle trailing slashes in its path. - /// Overrides any setting applied to [`crate::components::Router`]. - /// Serves as a default for any inner Routes. - #[prop(optional)] - trailing_slash: Option<TrailingSlash>, /// `children` may be empty or include nested routes. #[prop(optional)] children: Option<Children>, @@ -221,7 +193,6 @@ where data, Some(mode), Some(Arc::new(static_params)), - trailing_slash, ) } @@ -239,7 +210,6 @@ pub(crate) fn define_route( data: Option<Loader>, static_mode: Option<StaticMode>, static_params: Option<StaticData>, - trailing_slash: Option<TrailingSlash>, ) -> RouteDefinition { let children = children .map(|children| { @@ -256,8 +226,14 @@ pub(crate) fn define_route( }) .unwrap_or_default(); + let id = ROUTE_ID.with(|id| { + let next = id.get() + 1; + id.set(next); + next + }); + RouteDefinition { - id: new_route_id(), + id, path, children, view, @@ -266,7 +242,6 @@ pub(crate) fn define_route( data, static_mode, static_params, - trailing_slash, } } diff --git a/router/src/components/router.rs b/router/src/components/router.rs index 4ce714333d..0238821303 100644 --- a/router/src/components/router.rs +++ b/router/src/components/router.rs @@ -32,16 +32,13 @@ pub fn Router( /// A signal that will be set while the navigation process is underway. #[prop(optional, into)] set_is_routing: Option<SignalSetter<bool>>, - /// How trailing slashes should be handled in [`Route`] paths. - #[prop(optional)] - trailing_slash: Option<TrailingSlash>, /// The `<Router/>` should usually wrap your whole page. It can contain /// any elements, and should include a [`Routes`](crate::Routes) component somewhere /// to define and display [`Route`](crate::Route)s. children: Children, ) -> impl IntoView { // create a new RouterContext and provide it to every component beneath the router - let router = RouterContext::new(base, fallback, trailing_slash); + let router = RouterContext::new(base, fallback); provide_context(router); provide_context(GlobalSuspenseContext::new()); if let Some(set_is_routing) = set_is_routing { @@ -63,7 +60,6 @@ pub(crate) struct RouterContextInner { id: usize, pub location: Location, pub base: RouteContext, - trailing_slash: Option<TrailingSlash>, pub possible_routes: RefCell<Option<Vec<Branch>>>, #[allow(unused)] // used in CSR/hydrate base_path: String, @@ -100,7 +96,6 @@ impl RouterContext { pub(crate) fn new( base: Option<&'static str>, fallback: Option<fn() -> View>, - trailing_slash: Option<TrailingSlash>, ) -> Self { cfg_if! { if #[cfg(any(feature = "csr", feature = "hydrate"))] { @@ -182,7 +177,6 @@ impl RouterContext { path_stack: store_value(vec![location.pathname.get_untracked()]), location, base, - trailing_slash, history: Box::new(history), reference, @@ -221,10 +215,6 @@ impl RouterContext { self.inner.id } - pub(crate) fn trailing_slash(&self) -> Option<TrailingSlash> { - self.inner.trailing_slash.clone() - } - /// A list of all possible routes this router can match. pub fn possible_branches(&self) -> Vec<Branch> { self.inner @@ -482,71 +472,3 @@ impl Default for NavigateOptions { } } } - -/// Declares how you would like to handle trailing slashes in Route paths. This -/// can be set on [`Router`] and overridden in [`crate::components::Route`] -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum TrailingSlash { - /// This is the default behavior as of Leptos 0.5. Trailing slashes in your - /// `Route` path are stripped. i.e.: the following two route declarations - /// are equivalent: - /// * `<Route path="/foo">` - /// * `<Route path="/foo/">` - Drop, - - /// This mode will respect your path as it is written. Ex: - /// * If you specify `<Route path="/foo">`, then `/foo` matches, but - /// `/foo/` does not. - /// * If you specify `<Route path="/foo/">`, then `/foo/` matches, but - /// `/foo` does not. - Exact, - - /// Like `Exact`, this mode respects your path as-written. But it will also - /// add redirects to the specified path if a user nagivates to a URL that is - /// off by only the trailing slash. - /// - /// Given `<Route path="/foo">` - /// * Visiting `/foo` is valid. - /// * Visiting `/foo/` serves a redirect to `/foo` - /// - /// Given `<Route path="/foo/">` - /// * Visiting `/foo` serves a redirect to `/foo/` - /// * Visiting `/foo/` is valid. - Redirect, -} - -impl Default for TrailingSlash { - fn default() -> Self { - // This is the behavior in 0.5. Keeping it the default for backward compatibility. - // TODO: Change to `Redirect` in 0.6? - Self::Drop - } -} - -impl TrailingSlash { - /// Should we redirect requests that come in with the wrong (extra/missing) trailng slash? - pub(crate) fn should_redirect(&self) -> bool { - use TrailingSlash::*; - match self { - Redirect => true, - Drop | Exact => false, - } - } - - pub(crate) fn normalize_route_path(&self, path: &mut String) { - if !self.should_drop() { - return; - } - while path.ends_with('/') { - path.pop(); - } - } - - fn should_drop(&self) -> bool { - use TrailingSlash::*; - match self { - Redirect | Exact => false, - Drop => true, - } - } -} diff --git a/router/src/components/routes.rs b/router/src/components/routes.rs index fe5153447c..c83d1dd95c 100644 --- a/router/src/components/routes.rs +++ b/router/src/components/routes.rs @@ -1,12 +1,10 @@ use crate::{ animation::*, - components::route::new_route_id, matching::{ expand_optionals, get_route_matches, join_paths, Branch, Matcher, RouteDefinition, RouteMatch, }, - use_is_back_navigation, use_route, NavigateOptions, Redirect, RouteContext, - RouterContext, SetIsRouting, TrailingSlash, + use_is_back_navigation, RouteContext, RouterContext, SetIsRouting, }; use leptos::{leptos_dom::HydrationCtx, *}; use std::{ @@ -84,7 +82,7 @@ pub fn Routes( let base_route = router.base(); let base = base.unwrap_or_default(); - Branches::initialize(&router, &base, children()); + Branches::initialize(router_id, &base, children()); #[cfg(feature = "ssr")] if let Some(context) = use_context::<crate::PossibleBranchContext>() { @@ -166,7 +164,7 @@ pub fn AnimatedRoutes( let base_route = router.base(); let base = base.unwrap_or_default(); - Branches::initialize(&router, &base, children()); + Branches::initialize(router_id, &base, children()); #[cfg(feature = "ssr")] if let Some(context) = use_context::<crate::PossibleBranchContext>() { @@ -203,8 +201,8 @@ pub fn AnimatedRoutes( let matches = get_route_matches(router_id, &base, next_route.clone()); let same_route = prev_matches - .and_then(|p| p.first().as_ref().map(|r| r.route.key.clone())) - == matches.first().as_ref().map(|r| r.route.key.clone()); + .and_then(|p| p.first().map(|r| r.route.key.clone())) + == matches.first().map(|r| r.route.key.clone()); if same_route { (animation_state, next_route) } else { @@ -280,7 +278,7 @@ thread_local! { } impl Branches { - pub fn initialize(router: &RouterContext, base: &str, children: Fragment) { + pub fn initialize(router_id: usize, base: &str, children: Fragment) { BRANCHES.with(|branches| { #[cfg(debug_assertions)] { @@ -295,9 +293,9 @@ impl Branches { } let mut current = branches.borrow_mut(); - if !current.contains_key(&(router.id(), Cow::from(base))) { + if !current.contains_key(&(router_id, Cow::from(base))) { let mut branches = Vec::new(); - let mut children = children + let children = children .as_children() .iter() .filter_map(|child| { @@ -317,7 +315,6 @@ impl Branches { }) .cloned() .collect::<Vec<_>>(); - inherit_settings(&mut children, router); create_branches( &children, base, @@ -326,7 +323,7 @@ impl Branches { true, base, ); - current.insert((router.id(), Cow::Owned(base.into())), branches); + current.insert((router_id, Cow::Owned(base.into())), branches); } }) } @@ -347,38 +344,6 @@ impl Branches { } } -// <Route>s may inherit settings from each other or <Router>. -// This mutates RouteDefinitions to propagate those settings. -fn inherit_settings(children: &mut [RouteDefinition], router: &RouterContext) { - struct InheritProps { - trailing_slash: Option<TrailingSlash>, - } - fn route_def_inherit( - children: &mut [RouteDefinition], - inherited: InheritProps, - ) { - for child in children { - if child.trailing_slash.is_none() { - child.trailing_slash = inherited.trailing_slash.clone(); - } - route_def_inherit( - &mut child.children, - InheritProps { - trailing_slash: child.trailing_slash.clone(), - }, - ); - } - } - route_def_inherit( - children, - InheritProps { - trailing_slash: router - .trailing_slash() - .or(Some(TrailingSlash::default())), - }, - ); -} - fn route_states( router_id: usize, base: String, @@ -588,7 +553,6 @@ pub(crate) struct RouterState { #[derive(Debug, Clone, PartialEq)] pub struct RouteData { - // This ID is always the same as key.id. Deprecate? pub id: usize, pub key: RouteDefinition, pub pattern: String, @@ -683,114 +647,24 @@ fn create_routes( parents_path, route_def.path ); } - let trailing_slash = route_def - .trailing_slash - .clone() - .expect("trailng_slash should be set by this point"); let mut acc = Vec::new(); for original_path in expand_optionals(&route_def.path) { - let mut path = join_paths(base, &original_path).to_string(); - trailing_slash.normalize_route_path(&mut path); + let path = join_paths(base, &original_path); let pattern = if is_leaf { path - } else if let Some((path, _splat)) = path.split_once("/*") { - path.to_string() } else { - path + path.split("/*") + .next() + .map(|n| n.to_string()) + .unwrap_or(path) }; - - let route_data = RouteData { + acc.push(RouteData { key: route_def.clone(), id: route_def.id, matcher: Matcher::new_with_partial(&pattern, !is_leaf), pattern, original_path: original_path.into_owned(), - }; - - if route_data.matcher.is_wildcard() { - // already handles trailing_slash - } else if let Some(redirect_route) = redirect_route_for(route_def) { - let pattern = &redirect_route.path; - let redirect_route_data = RouteData { - id: redirect_route.id, - matcher: Matcher::new_with_partial(pattern, !is_leaf), - pattern: pattern.to_owned(), - original_path: pattern.to_owned(), - key: redirect_route, - }; - acc.push(redirect_route_data); - } - - acc.push(route_data); + }); } acc } - -/// A new route that redirects to `route` with the correct trailng slash. -fn redirect_route_for(route: &RouteDefinition) -> Option<RouteDefinition> { - if matches!(route.path.as_str(), "" | "/") { - // Root paths are an exception to the rule and are always equivalent: - return None; - } - - let trailing_slash = route - .trailing_slash - .clone() - .expect("trailing_slash should be defined by now"); - - if !trailing_slash.should_redirect() { - return None; - } - - // Are we creating a new route that adds or removes a slash? - let add_slash = route.path.ends_with('/'); - let view = Rc::new(move || { - view! { - <FixTrailingSlash add_slash /> - } - .into_view() - }); - - let new_pattern = if add_slash { - // If we need to add a slash, we need to match on the path w/o it: - let mut path = route.path.clone(); - path.pop(); - path - } else { - format!("{}/", route.path) - }; - let new_route = RouteDefinition { - path: new_pattern, - children: vec![], - data: None, - methods: route.methods, - id: new_route_id(), - view, - ssr_mode: route.ssr_mode, - static_mode: route.static_mode, - static_params: None, - trailing_slash: None, // Shouldn't be needed/used from here on out - }; - - Some(new_route) -} - -#[component] -fn FixTrailingSlash(add_slash: bool) -> impl IntoView { - let route = use_route(); - let path = if add_slash { - format!("{}/", route.path()) - } else { - let mut path = route.path().to_string(); - path.pop(); - path - }; - let options = NavigateOptions { - replace: true, - ..Default::default() - }; - - view! { - <Redirect path options/> - } -} diff --git a/router/src/extract_routes.rs b/router/src/extract_routes.rs index 8acc5b1637..031632f13d 100644 --- a/router/src/extract_routes.rs +++ b/router/src/extract_routes.rs @@ -1,9 +1,6 @@ -mod test_extract_routes; - use crate::{ - provide_server_redirect, Branch, Method, RouterIntegrationContext, - ServerIntegration, SsrMode, StaticDataMap, StaticMode, StaticParamsMap, - StaticPath, + Branch, Method, RouterIntegrationContext, ServerIntegration, SsrMode, + StaticDataMap, StaticMode, StaticParamsMap, StaticPath, }; use leptos::*; use std::{ @@ -45,9 +42,6 @@ impl RouteListing { } /// The path this route handles. - /// - /// This should be formatted for whichever web server integegration is being used. (ex: leptos-actix.) - /// When returned from leptos-router, it matches `self.leptos_path()`. pub fn path(&self) -> &str { &self.path } @@ -136,9 +130,21 @@ where { let runtime = create_runtime(); - let branches = get_branches(app_fn, additional_context); - let branches = branches.0.borrow(); + let integration = ServerIntegration { + path: "http://leptos.rs/".to_string(), + }; + provide_context(RouterIntegrationContext::new(integration)); + let branches = PossibleBranchContext::default(); + provide_context(branches.clone()); + + additional_context(); + + leptos::suppress_resource_load(true); + _ = app_fn().into_view(); + leptos::suppress_resource_load(false); + + let branches = branches.0.borrow(); let mut static_data_map: StaticDataMap = HashMap::new(); let routes = branches .iter() @@ -178,29 +184,3 @@ where runtime.dispose(); (routes, static_data_map) } - -fn get_branches<IV>( - app_fn: impl Fn() -> IV + 'static + Clone, - additional_context: impl Fn() + 'static + Clone, -) -> PossibleBranchContext -where - IV: IntoView + 'static, -{ - let integration = ServerIntegration { - path: "http://leptos.rs/".to_string(), - }; - - provide_context(RouterIntegrationContext::new(integration)); - let branches = PossibleBranchContext::default(); - provide_context(branches.clone()); - // Suppress startup warning about using <Redirect/> without ServerRedirectFunction: - provide_server_redirect(|_str| ()); - - additional_context(); - - leptos::suppress_resource_load(true); - _ = app_fn().into_view(); - leptos::suppress_resource_load(false); - - branches -} diff --git a/router/src/extract_routes/test_extract_routes.rs b/router/src/extract_routes/test_extract_routes.rs deleted file mode 100644 index 69b41bb6ad..0000000000 --- a/router/src/extract_routes/test_extract_routes.rs +++ /dev/null @@ -1,258 +0,0 @@ -// This is here, vs /router/tests/, because it accesses some `pub(crate)` -// features to test crate internals that wouldn't be available there. - -#![cfg(all(test, feature = "ssr"))] - -use crate::*; -use itertools::Itertools; -use leptos::*; -use std::{cell::RefCell, rc::Rc}; - -#[component] -fn DefaultApp() -> impl IntoView { - let view = || view! { "" }; - view! { - <Router> - <Routes> - <Route path="/foo" view/> - <Route path="/bar/" view/> - <Route path="/baz/:id" view/> - <Route path="/name/:name/" view/> - <Route path="/any/*any" view/> - </Routes> - </Router> - } -} - -#[component] -fn ExactApp() -> impl IntoView { - let view = || view! { "" }; - let trailing_slash = TrailingSlash::Exact; - view! { - <Router trailing_slash> - <Routes> - <Route path="/foo" view/> - <Route path="/bar/" view/> - <Route path="/baz/:id" view/> - <Route path="/name/:name/" view/> - <Route path="/any/*any" view/> - </Routes> - </Router> - } -} - -#[component] -fn RedirectApp() -> impl IntoView { - let view = || view! { "" }; - let trailing_slash = TrailingSlash::Redirect; - view! { - <Router trailing_slash> - <Routes> - <Route path="/foo" view/> - <Route path="/bar/" view/> - <Route path="/baz/:id" view/> - <Route path="/name/:name/" view/> - <Route path="/any/*any" view/> - </Routes> - </Router> - } -} -#[test] -fn test_generated_routes_default() { - // By default, we use the behavior as of Leptos 0.5, which is equivalent to TrailingSlash::Drop. - assert_generated_paths( - DefaultApp, - &["/any/*any", "/bar", "/baz/:id", "/foo", "/name/:name"], - ); -} - -#[test] -fn test_generated_routes_exact() { - // Allow users to precisely define whether slashes are present: - assert_generated_paths( - ExactApp, - &["/any/*any", "/bar/", "/baz/:id", "/foo", "/name/:name/"], - ); -} - -#[test] -fn test_generated_routes_redirect() { - // TralingSlashes::Redirect generates paths to redirect to the path with the "correct" trailing slash ending (or lack thereof). - assert_generated_paths( - RedirectApp, - &[ - "/any/*any", - "/bar", - "/bar/", - "/baz/:id", - "/baz/:id/", - "/foo", - "/foo/", - "/name/:name", - "/name/:name/", - ], - ) -} - -#[test] -fn test_rendered_redirect() { - // Given an app that uses TrailngSlsahes::Redirect, rendering the redirected path - // should render the redirect. Other paths should not. - - let expected_redirects = &[ - ("/bar", "/bar/"), - ("/baz/some_id/", "/baz/some_id"), - ("/name/some_name", "/name/some_name/"), - ("/foo/", "/foo"), - ]; - - let redirect_result = Rc::new(RefCell::new(Option::None)); - let rc = redirect_result.clone(); - let server_redirect = move |new_value: &str| { - rc.replace(Some(new_value.to_string())); - }; - - let _runtime = Disposable(create_runtime()); - let history = TestHistory::new("/"); - provide_context(RouterIntegrationContext::new(history.clone())); - provide_server_redirect(server_redirect); - - // We expect these redirects to exist: - for (src, dest) in expected_redirects { - let loc = format!("https://example.com{src}"); - history.goto(&loc); - redirect_result.replace(None); - RedirectApp().into_view().render_to_string(); - let redirected_to = redirect_result.borrow().clone(); - assert!( - redirected_to.is_some(), - "Should redirect from {src} to {dest}" - ); - assert_eq!(redirected_to.unwrap(), *dest); - } - - // But the destination paths shouldn't themselves redirect: - redirect_result.replace(None); - for (_src, dest) in expected_redirects { - let loc = format!("https://example.com{dest}"); - history.goto(&loc); - RedirectApp().into_view().render_to_string(); - let redirected_to = redirect_result.borrow().clone(); - assert!( - redirected_to.is_none(), - "Destination of redirect shouldn't also redirect: {dest}" - ); - } -} - -struct Disposable(RuntimeId); - -// If the test fails, and we don't dispose, we get irrelevant panics. -impl Drop for Disposable { - fn drop(&mut self) { - self.0.dispose() - } -} - -#[derive(Clone)] -struct TestHistory { - loc: RwSignal<LocationChange>, -} - -impl TestHistory { - fn new(initial: &str) -> Self { - let lc = LocationChange { - value: initial.to_owned(), - ..Default::default() - }; - Self { - loc: create_rw_signal(lc), - } - } - - fn goto(&self, loc: &str) { - let change = LocationChange { - value: loc.to_string(), - ..Default::default() - }; - - self.navigate(&change); - } -} - -impl History for TestHistory { - fn location(&self) -> ReadSignal<LocationChange> { - self.loc.read_only() - } - - fn navigate(&self, new_loc: &LocationChange) { - self.loc.update(|loc| loc.value = new_loc.value.clone()) - } -} - -// WARNING! -// -// Despite generate_route_list_inner() using a new leptos_reactive::RuntimeID -// each time we call this function, somehow Routes are leaked between different -// apps. To avoid that, make sure to put each call in a separate #[test] method. -// -// TODO: Better isolation for different apps to avoid this issue? -fn assert_generated_paths<F, IV>(app: F, expected_sorted_paths: &[&str]) -where - F: Clone + Fn() -> IV + 'static, - IV: IntoView + 'static, -{ - let (routes, static_data) = generate_route_list_inner(app); - - let mut paths = routes.iter().map(|route| route.path()).collect_vec(); - paths.sort(); - - assert_eq!(paths, expected_sorted_paths); - - let mut keys = static_data.keys().collect_vec(); - keys.sort(); - assert_eq!(paths, keys); - - // integrations can update "path" to be valid for themselves, but - // when routes are returned by leptos_router, these are equal: - assert!(routes - .iter() - .all(|route| route.path() == route.leptos_path())); -} - -#[test] -fn test_unique_route_ids() { - let branches = get_branches(RedirectApp); - assert!(!branches.is_empty()); - - assert!(branches - .iter() - .flat_map(|branch| &branch.routes) - .map(|route| route.id) - .all_unique()); -} - -#[test] -fn test_unique_route_patterns() { - let branches = get_branches(RedirectApp); - assert!(!branches.is_empty()); - - assert!(branches - .iter() - .flat_map(|branch| &branch.routes) - .map(|route| route.pattern.as_str()) - .all_unique()); -} - -fn get_branches<F, IV>(app_fn: F) -> Vec<Branch> -where - F: Fn() -> IV + Clone + 'static, - IV: IntoView + 'static, -{ - let runtime = create_runtime(); - let additional_context = || (); - let branches = super::get_branches(app_fn, additional_context); - let branches = branches.0.borrow().clone(); - runtime.dispose(); - branches -} diff --git a/router/src/matching/matcher.rs b/router/src/matching/matcher.rs index d64bde6738..9351267bca 100644 --- a/router/src/matching/matcher.rs +++ b/router/src/matching/matcher.rs @@ -31,11 +31,14 @@ impl Matcher { Some((p, s)) => (p, Some(s.to_string())), None => (path, None), }; - let segments = get_segments(pattern) - .iter() - .map(|s| s.to_string()) + let segments = pattern + .split('/') + .filter(|n| !n.is_empty()) + .map(|n| n.to_string()) .collect::<Vec<_>>(); + let len = segments.len(); + Self { splat, segments, @@ -45,28 +48,23 @@ impl Matcher { } #[doc(hidden)] - pub fn test(&self, mut location: &str) -> Option<PathMatch> { - // URL root paths "/" and "" are equivalent. - // Web servers (at least, Axum and Actix-Web) will send us a path of "/" - // even if we've routed "". Always treat these as equivalent: - if location == "/" && self.len == 0 { - location = "" - } - - let loc_segments = get_segments(location); + pub fn test(&self, location: &str) -> Option<PathMatch> { + let loc_segments = location + .split('/') + .filter(|n| !n.is_empty()) + .collect::<Vec<_>>(); let loc_len = loc_segments.len(); let len_diff: i32 = loc_len as i32 - self.len as i32; - let trailing_slashes = - location.chars().rev().take_while(|n| *n == '/').count(); + let trailing_iter = location.chars().rev().take_while(|n| *n == '/'); // quick path: not a match if // 1) matcher has add'l segments not found in location // 2) location has add'l segments, there's no splat, and partial matches not allowed if loc_len < self.len || (len_diff > 0 && self.splat.is_none() && !self.partial) - || (self.splat.is_none() && trailing_slashes > 1) + || (self.splat.is_none() && trailing_iter.clone().count() > 1) { None } @@ -91,12 +89,17 @@ impl Matcher { if let Some(splat) = &self.splat { if !splat.is_empty() { - let value = if len_diff > 0 { + let mut value = if len_diff > 0 { loc_segments[self.len..].join("/") } else { "".into() }; + // add trailing slashes to splat + let trailing_slashes = + trailing_iter.skip(1).collect::<String>(); + value.push_str(&trailing_slashes); + params.insert(splat.into(), value); } } @@ -104,19 +107,4 @@ impl Matcher { Some(PathMatch { path, params }) } } - - #[doc(hidden)] - pub(crate) fn is_wildcard(&self) -> bool { - self.splat.is_some() - } -} - -fn get_segments(pattern: &str) -> Vec<&str> { - pattern - .split('/') - .enumerate() - // Only remove a leading slash, not trailing slashes: - .skip_while(|(i, part)| *i == 0 && part.is_empty()) - .map(|(_, part)| part) - .collect() } diff --git a/router/src/matching/resolve_path.rs b/router/src/matching/resolve_path.rs index 043e2a9fb0..3f236cef9b 100644 --- a/router/src/matching/resolve_path.rs +++ b/router/src/matching/resolve_path.rs @@ -51,14 +51,7 @@ fn has_scheme(path: &str) -> bool { #[doc(hidden)] fn normalize(path: &str, omit_slash: bool) -> Cow<'_, str> { - let s = path.trim_start_matches('/'); - let trim_end = s - .chars() - .rev() - .take_while(|c| *c == '/') - .count() - .saturating_sub(1); - let s = &s[0..s.len() - trim_end]; + let s = path.trim_start_matches('/').trim_end_matches('/'); if s.is_empty() || omit_slash || begins_with_query_or_hash(s) { s.into() } else { @@ -77,10 +70,9 @@ fn begins_with_query_or_hash(text: &str) -> bool { } fn remove_wildcard(text: &str) -> String { - text.rsplit_once('*') - .map(|(prefix, _)| prefix) + text.split_once('*') + .map(|(prefix, _)| prefix.trim_end_matches('/')) .unwrap_or(text) - .trim_end_matches('/') .to_string() } @@ -91,14 +83,4 @@ mod tests { fn normalize_query_string_with_opening_slash() { assert_eq!(normalize("/?foo=bar", false), "?foo=bar"); } - - #[test] - fn normalize_retain_trailing_slash() { - assert_eq!(normalize("foo/bar/", false), "/foo/bar/"); - } - - #[test] - fn normalize_dedup_trailing_slashes() { - assert_eq!(normalize("foo/bar/////", false), "/foo/bar/"); - } } diff --git a/router/src/matching/route.rs b/router/src/matching/route.rs index cf73e1413e..24fe6b67e1 100644 --- a/router/src/matching/route.rs +++ b/router/src/matching/route.rs @@ -1,4 +1,4 @@ -use crate::{Loader, Method, SsrMode, StaticData, StaticMode, TrailingSlash}; +use crate::{Loader, Method, SsrMode, StaticData, StaticMode}; use leptos::leptos_dom::View; use std::rc::Rc; @@ -25,8 +25,6 @@ pub struct RouteDefinition { pub static_mode: Option<StaticMode>, /// The data required to fill any dynamic segments in the path during static rendering. pub static_params: Option<StaticData>, - /// How a trailng slash in `path` should be handled. - pub trailing_slash: Option<TrailingSlash>, } impl core::fmt::Debug for RouteDefinition { @@ -36,7 +34,6 @@ impl core::fmt::Debug for RouteDefinition { .field("children", &self.children) .field("ssr_mode", &self.ssr_mode) .field("static_render", &self.static_mode) - .field("trailing_slash", &self.trailing_slash) .finish() } } diff --git a/router/tests/join_paths.rs b/router/tests/join_paths.rs index f1fadfec32..a0f6828407 100644 --- a/router/tests/join_paths.rs +++ b/router/tests/join_paths.rs @@ -44,14 +44,5 @@ cfg_if! { assert_eq!(join_paths("/foo", ":bar/baz"), "/foo/:bar/baz"); assert_eq!(join_paths("", ":bar/baz"), "/:bar/baz"); } - - // Additional tests NOT from Solid Router: - #[test] - fn join_paths_for_root() { - assert_eq!(join_paths("", ""), ""); - assert_eq!(join_paths("", "/"), ""); - assert_eq!(join_paths("/", ""), ""); - assert_eq!(join_paths("/", "/"), ""); - } } } diff --git a/router/tests/trailing_slashes.rs b/router/tests/trailing_slashes.rs deleted file mode 100644 index cacbc7f8cb..0000000000 --- a/router/tests/trailing_slashes.rs +++ /dev/null @@ -1,58 +0,0 @@ -//! Some extra tests for Matcher NOT based on SolidJS's tests cases (as in matcher.rs) - -use leptos_router::{params_map, Matcher}; - -#[test] -fn trailing_slashes_match_exactly() { - let matcher = Matcher::new("/foo/"); - assert_matches(&matcher, "/foo/"); - assert_no_match(&matcher, "/foo"); - - let matcher = Matcher::new("/foo/bar/"); - assert_matches(&matcher, "/foo/bar/"); - assert_no_match(&matcher, "/foo/bar"); - - let matcher = Matcher::new("/"); - assert_matches(&matcher, "/"); - assert_no_match(&matcher, ""); - - let matcher = Matcher::new(""); - assert_matches(&matcher, ""); - - // Despite returning a pattern of "", web servers (known: Actix-Web and Axum) - // may send us a path of "/". We should match those at the root: - assert_matches(&matcher, "/"); -} - -#[test] -fn trailng_slashes_params_match_exactly() { - let matcher = Matcher::new("/foo/:bar/"); - assert_matches(&matcher, "/foo/bar/"); - assert_matches(&matcher, "/foo/42/"); - assert_matches(&matcher, "/foo/%20/"); - - assert_no_match(&matcher, "/foo/bar"); - assert_no_match(&matcher, "/foo/42"); - assert_no_match(&matcher, "/foo/%20"); - - let m = matcher.test("/foo/asdf/").unwrap(); - assert_eq!(m.params, params_map! { "bar" => "asdf" }); -} - -fn assert_matches(matcher: &Matcher, path: &str) { - assert!( - matches(matcher, path), - "{matcher:?} should match path {path:?}" - ); -} - -fn assert_no_match(matcher: &Matcher, path: &str) { - assert!( - !matches(matcher, path), - "{matcher:?} should NOT match path {path:?}" - ); -} - -fn matches(m: &Matcher, loc: &str) -> bool { - m.test(loc).is_some() -}