Skip to content

Commit

Permalink
Fixes and more tests for matching "" && "/".
Browse files Browse the repository at this point in the history
This path is the exception to the rule and *should* be treated
as equivalent regardless of its trailing slash.
  • Loading branch information
Cody Casterline committed Jan 11, 2024
1 parent 4672d6e commit 9c379f0
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 18 deletions.
3 changes: 2 additions & 1 deletion examples/ssr_modes/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ 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>
<Router fallback>
<main>
<Routes>
// We’ll load the home page with out-of-order streaming and <Suspense/>
Expand Down
3 changes: 2 additions & 1 deletion examples/ssr_modes_axum/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ 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>
<Router fallback>
<main>
<Routes>
// We’ll load the home page with out-of-order streaming and <Suspense/>
Expand Down
13 changes: 11 additions & 2 deletions router/src/components/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
RouteDefinition, RouteMatch,
},
use_is_back_navigation, use_route, Redirect, RouteContext, RouterContext,
SetIsRouting, TrailingSlash,
SetIsRouting, TrailingSlash, NavigateOptions,
};
use leptos::{leptos_dom::HydrationCtx, *};
use std::{
Expand Down Expand Up @@ -728,6 +728,11 @@ fn create_routes(

/// 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()
Expand Down Expand Up @@ -780,8 +785,12 @@ fn FixTrailingSlash(add_slash: bool) -> impl IntoView {
path.pop();
path
};
let options = NavigateOptions {
replace: true,
..Default::default()
};

view! {
<Redirect path/>
<Redirect path options/>
}
}
5 changes: 4 additions & 1 deletion router/src/extract_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod test_extract_routes;

use crate::{
Branch, Method, RouterIntegrationContext, ServerIntegration, SsrMode,
StaticDataMap, StaticMode, StaticParamsMap, StaticPath,
StaticDataMap, StaticMode, StaticParamsMap, StaticPath, provide_server_redirect,
};
use leptos::*;
use std::{
Expand Down Expand Up @@ -192,6 +192,9 @@ where
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();

Expand Down
12 changes: 12 additions & 0 deletions router/src/extract_routes/test_extract_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,18 @@ fn test_unique_route_ids() {
.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,
Expand Down
9 changes: 8 additions & 1 deletion router/src/matching/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@ impl Matcher {
}

#[doc(hidden)]
pub fn test(&self, location: &str) -> Option<PathMatch> {
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);

let loc_len = loc_segments.len();
Expand Down
9 changes: 9 additions & 0 deletions router/tests/join_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,14 @@ 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("/", "/"), "");
}
}
}
41 changes: 29 additions & 12 deletions router/tests/trailing_slashes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,48 @@ use leptos_router::{params_map, Matcher};
#[test]
fn trailing_slashes_match_exactly() {
let matcher = Matcher::new("/foo/");
assert!(matches(&matcher, "/foo/"));
assert!(!matches(&matcher, "/foo"));
assert_matches(&matcher, "/foo/");
assert_no_match(&matcher, "/foo");

let matcher = Matcher::new("/foo/bar/");
assert!(matches(&matcher, "/foo/bar/"));
assert!(!matches(&matcher, "/foo/bar"));
assert!(!matches(&matcher, "/foo/"));
assert!(!matches(&matcher, "/foo"));
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_matches(&matcher, "/foo/bar/");
assert_matches(&matcher, "/foo/42/");
assert_matches(&matcher, "/foo/%20/");

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()
}

0 comments on commit 9c379f0

Please sign in to comment.