yew icon indicating copy to clipboard operation
yew copied to clipboard

Unexpected nested router behaviors

Open Banyc opened this issue 3 years ago • 7 comments

Problem

The nested routers are failing to match URLs with specific patterns.

* did not catch the empty string.

Steps To Reproduce

  1. Implement the code:
use yew::prelude::*;
use yew_router::prelude::*;

#[derive(Clone, Routable, PartialEq)]
enum MainRoute {
    #[at("/")]
    Home,
    #[at("/news")]
    News,
    #[at("/contact")]
    Contact,
    #[at("/settings/*")]
    Settings,
    #[not_found]
    #[at("/404")]
    NotFound,
}

#[derive(Clone, Routable, PartialEq)]
enum SettingsRoute {
    #[at("/settings")]
    Profile,
    #[at("/settings/some/friends")]
    Friends,
    #[at("/settings/")]
    Theme,
    #[not_found]
    #[at("/settings/404")]
    NotFound,
}

fn switch_main(route: &MainRoute) -> Html {
    match route {
        MainRoute::Home => html! {<h1>{"Home"}</h1>},
        MainRoute::News => html! {<h1>{"News"}</h1>},
        MainRoute::Contact => html! {<h1>{"Contact"}</h1>},
        MainRoute::Settings => html! {
            <Switch<SettingsRoute> render={Switch::render(switch_settings)} />
        },
        MainRoute::NotFound => html! {<h1>{"Not Found"}</h1>},
    }
}

fn switch_settings(route: &SettingsRoute) -> Html {
    match route {
        SettingsRoute::Profile => html! {<h1>{"Profile"}</h1>},
        SettingsRoute::Friends => html! {<h1>{"Friends"}</h1>},
        SettingsRoute::Theme => html! {<h1>{"Theme"}</h1>},
        SettingsRoute::NotFound => html! {
            <Redirect<MainRoute> to={MainRoute::NotFound}/>
        }
    }
}

#[function_component(App)]
pub fn app() -> Html {
    html! {
        <BrowserRouter>
            <Switch<MainRoute> render={Switch::render(switch_main)} />
        </BrowserRouter>
    }
}

fn main() {
    yew::start_app::<App>();
}

  1. Go to localhost:8080/settings/some/friends
  2. See Friends
  3. Go to localhost:8080/settings/
  4. See error
  5. Go to localhost:8080/settings
  6. See error

Expected behavior

  1. Go to localhost:8080/settings/some/friends
  2. See Friends
  3. Go to localhost:8080/settings/
  4. See Theme
  5. Go to localhost:8080/settings
  6. See Not Found

Environment:

  • Yew version: 0.19.3
  • Rust version: 1.63.0
  • Target, if relevant: wasm32-unknown-unknown
  • Build tool, if relevant: trunk
  • OS, if relevant: MacOS
  • Browser and version, if relevant: Chrome v 104.0.5112.101

Questionnaire

  • [x] I'm interested in fixing this myself but don't know where to start
  • [ ] I would like to fix and I have a solution
  • [ ] I don't have time to fix this right now, but maybe later

Banyc avatar Aug 17 '22 10:08 Banyc

Seems like the router treats "/settings" and "/settings/" equally. Not sure if it's ideal. If so, this issue can be closed.

Banyc avatar Aug 17 '22 12:08 Banyc

I feel /settings and /settings/ should be treated as the same path. Like: https://github.com/settings and https://github.com/settings/ are interpreted as the same path.

yew-router uses route_recognizer as the underlying routing implementation and we cannot control this behaviour.

Following the discord conversation, I believe this has been solved?

Go to localhost:8080/settings/ See error Go to localhost:8080/settings See error

futursolo avatar Aug 17 '22 16:08 futursolo

Following the discord conversation, I believe this has been solved?

Go to localhost:8080/settings/ See error Go to localhost:8080/settings See error

I have edited the description from using x/:s to x/*. This change solved the localhost:8080/settings/some/friends problem, but not the two examples you cited.


I feel /settings and /settings/ should be treated as the same path. Like: https://github.com/settings and https://github.com/settings/ are interpreted as the same path.

yew-router uses route_recognizer as the underlying routing implementation and we cannot control this behaviour.

I just came out an new idea/issue: if /settings is the same as /settings/, then it is still possible to place /settings/ under the reign of the nested router. The control of /settings has already been transferred from the outer router to the inner router, so either /settings or /settings/ should route the view to a page successfully, but it's not what happened.


Thanks for your hard work on the yew-router package!

Banyc avatar Aug 17 '22 17:08 Banyc

I think trailing slashes should be differentiated and not just dropped, it matters for relative paths. And the reason this happens is not route-recognizer, rather I think yew is stripping trailing slashes: https://github.com/yewstack/yew/blob/5181e1aca7150592ac595e38f38be483736e5951/packages/yew-router/src/macro_helpers.rs#L20

Relevant: sveltejs/kit#733

WorldSEnder avatar Aug 17 '22 17:08 WorldSEnder

I think trailing slashes should be differentiated and not just dropped, it matters for relative paths.

If we enforce strongly typed routes, then only absolute paths can be allowed.

Relevant: https://github.com/sveltejs/kit/issues/733

This is more about the address being pushed to the address bar. yew-router preserves the original / if it presents in the original route.

futursolo avatar Aug 17 '22 17:08 futursolo

If we enforce strongly typed routes, then only absolute paths can be allowed.

That's not what I meant, sorry I should clarify, take the example:

#[derive(Clone, Routable, PartialEq)]
enum MainRoute {
    #[at("/news")]
    News,
    #[at("/settings/")]
    Settings,
}
fn switch_main(route: &MainRoute) -> Html {
    match route {
        MainRoute::News => todo!("news"),
        MainRoute::Settings => todo!("settings"),
    }
}

Inside the case for MainRoute::News, I would want to refer to relative paths so that e.g. ./banner-pic.jpg refers to /banner-pic.jpg. Inside MainRoute::Settings I'd want to trust that the same relative path refers to /settings/banner-pic.jpg. But the current behaviour, implicitly dropping the trailing slash will also render the same switch match arm for /settings where ./banner-pic.jpg resolves to /banner-pic.jpg. This is surprising behaviour on top of implicitly dropping trailing slashes, so either some sort of normalization needs to happen or (preferably) trailing slashes should not be dropped and the route just doesn't match.

  • matching /news -> (expected) MainRoute::News matches, news banner renders and resolves to /banner-pic.jpg
  • matching /news/ -> (unexpected) MainRoute::News matches, news banner renders and resolves to /news/banner-pic.jpg (probably doesn't exist)
  • matching /settings -> (unexpected) MainRoute::Settings matches, settings banner renders and resolves to /banner-pic.jpg, unfortunately aliasing the news banner
  • matching /settings/ -> (expected) MainRoute::Settings matches, settings banner renders and resolves to /settings/banner-pic.jpg

If one wants the current behavior back, one can always insert a redirect, either on client or server side.

WorldSEnder avatar Aug 17 '22 18:08 WorldSEnder

But the current behaviour, implicitly dropping the trailing slash will also render the same switch match arm for /settings where ./banner-pic.jpg resolves to /banner-pic.jpg.

This behaviour is only applied during route matching. yew-router preserves / if it presents in the original route when pushed to the history entry.

futursolo avatar Aug 17 '22 18:08 futursolo