Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ergonomic support for nested routers #1853

Open
ranile opened this issue May 17, 2021 · 17 comments
Open

Add ergonomic support for nested routers #1853

ranile opened this issue May 17, 2021 · 17 comments
Labels
A-yew-router Area: The yew-router crate ergonomics feature-request A feature request proposal

Comments

@ranile
Copy link
Member

ranile commented May 17, 2021

Nested routers allow users to separate parts of application.

API

I would like to have the following API:

#[derive(Routable)]
enum Route {
    #[at("/")]
    Home,
    #[at("/room/:id")]
    Room
}

// main component
html! { <Router<Routes> render=Router::render(main_switch) /> }

fn main_switch(route: &Route) -> Html {
    match route {
        Route::Home => html! { <Home /> },
        Route::Room => html! { <Room /> },
    }
}

// `Room` component
html! {
    <Router<RoomRoutes> render=Router::render(room_render) />
}

#[derive(Routable)]
#[mount_at("/room")]
enum RoomRoute {
    #[at("/")] // will be /room/
    List,
    #[at("/:id")] // will be /room/:id
    View { id: u64 },
}

fn room_render(route: &RoomRoute) -> Html {
    match route {
        RoomRoute::List => html! { <ListRooms /> },
        RoomRoute::View { id }  => html! { <DisplayRoom id=id /> },
    }
}

Implementation

In order to implement the aforementioned API, we would:

  • Add a function Routable trait for handling mount_at
    fn mount_at() -> Option<&'static str>;
  • Macro will implement this function, optionally returning a value
  • Router and functions for handling routes will use this value in order to ensure the child router works properly
@ranile
Copy link
Member Author

ranile commented May 17, 2021

PS: I didn't create a discussion for this because those can't be linked with PRs and therefore harder to track features with.

Missing labels:

  • feature

@futursolo
Copy link
Member

I think it might be beneficial to have a parent router context that tracks the part that was already matched. So any child router will only match the unmatched path.

#[derive(Debug, Routable, Clone, PartialEq)]
enum LangRoute {
    #[at("/en/")]
    English,
    #[at("/fr/")]
    French,
}

#[derive(Routable)]
enum AppRoute {
    #[at("/")]
    Home,
    #[at("/article/:id")]
    Article { id: u64 },
}

#[function_component(App)]
fn app() -> Html {
    let render_app = |route| {
        match route {
            AppRoute::Home => html! {<Home />},
            AppRoute::Article { id } => html! {<Article id={id} />},
        }
    };

    // Reads matched route information from parent router context,
    // only matching path after /en/ or /fr/
    html! { <Router<AppRoute> render={Router::render(render_app)} /> }
}

#[function_component(Root)]
fn root() -> Html {
    // No parent router context, matching from /
    // Registers itself with matched route information to parent router context.
    html! { <Router<LangRoute> render={Router::render(|_| html! {<App />})} /> }
}

@ctron
Copy link
Contributor

ctron commented Dec 14, 2021

I would also be interested in this. I think that the child router shouldn't need to know where it was mounted in the parent. And it should also be possible to mount it several times.

@ctron
Copy link
Contributor

ctron commented Dec 14, 2021

Taking another look at this, while migrating a project from 0.14 (yew 0.17) to 0.16 (yew 0.19) I noticed that current approach may be flawed.

Previously, you had to nest enums as well. So you had something like:

enum Parent {
  Page1,
  Page2(Child),
  Page3(Child),
}

enum Child {
  Details1,
  Details2,
}

With that, you could create a link to e.g. to=Parent::Page2(Child::Details2). I am not sure how this would currently be possible, as the child has no context to where it is registered to.

ctron added a commit to ctron/yew-router that referenced this issue Dec 14, 2021
ctron added a commit to patternfly-yew/patternfly-yew that referenced this issue Dec 14, 2021
BREAKING CHANGE: While most of the Patternfly Yew components stay the same, the upgrade to Yew 0.19 is a breaking change.

This also uses a fork of yew-router. For more details, see: yewstack/yew#1853
@ctron
Copy link
Contributor

ctron commented Dec 14, 2021

It wasn't to hard to port yew-router 0.15 to use yew 0.19. I do think that the current yew-router should be properly fixed.

However, in the meantime, there is a fork which brings the functionality of yew-router 0.15 to yew 0.19: https://github.com/ctron/yew-router

@ranile
Copy link
Member Author

ranile commented Dec 14, 2021

#1860 attempted to introduce this back but it couldn't get anywhere because of how it was implemented. I think that our final goal should be bringing #[bind(_)] functionality to the router, just like that PR.

CC @futursolo for input since they've been working on router a lot recently

@futursolo
Copy link
Member

There's a flaw with the previous design I previously proposed.

I was thinking to use contexts to help children to become aware of matched parent routes. However, with current design, you can have a layout like the following:

<Router>
    <Nav />
    <Switch<RouteA> /> // Switch A
    <Switch<RouteB> /> // Switch B
    <Footer />
</Router>
// Somewhere in children of Switch B.
<Switch<RouteC> /> // Switch C

Children of switch c can see matched Route C & Route B and <Link<RouteC> /> in children of Switch B can infer Route B from parent.
However, everything outside of Switch B will not be able to see matched Route B, hence cannot have a <Link<RouteC> />.

I think either the originally purposed #[mount_at("/room")] design or Nested Routable design like the old router (plus the ability to have multiple switches under the same routing context with a layout like above) would work in this case and I am not biased to either one.

@ranile
Copy link
Member Author

ranile commented Jan 5, 2022

Another API proposal

(this one is easier to implement)

We nest <BrowserRouter>s and allow it to take a pathname to match with. This will result in the following code (from the nested router docs example):

use yew::prelude::*;
use yew_router::prelude::*;

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

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

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

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

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

@voidpumpkin
Copy link
Member

@hamza1311 I like this as this just patches up the nested router hack https://yew.rs/docs/concepts/router#nested-router
It will be easy to update for people that already were using it.

Though i wish we could just drop the /settings part of all urls in SettingsRoute. Can we use that base props to achieve it?

@ranile
Copy link
Member Author

ranile commented Jan 5, 2022

My bad, I forgot to edit that part out. We should not need it

@voidpumpkin
Copy link
Member

@hamza1311 I was planning to update that nested router hack in docs with this exact idea, but haven't had the time :(

@futursolo
Copy link
Member

We nest s and allow it to take a pathname to match with. This will result in the following code (from the nested router docs example):

It's very important to have only 1 router across the entire application so that navigation can be propagated to Switches in a fixed order (Parent Switch -> Child Switch) and a use_navigator() in the entire application will connect to the same history.

(BrowserHistory and HashHistory are the same instance per thread, but MemoryHistory create a different instance every time.)

In addition, this design has the same issue as my previous proposal (#1853 (comment)).

i.e.: How do a <Link to={SettingsRoute::Profile} /> in the <header /> (header is outside of nested switch, but under <BrowserRouter />) know that it has a basename of /settings?

@ranile
Copy link
Member Author

ranile commented Jan 7, 2022

The only good solution I can think of is either allowing switch to take a base parameter (which gets appended to router's provided base) or a mount_at attribute on Routable. I'm leaning towards the former, mostly because it doesn't require the macro

@ranile
Copy link
Member Author

ranile commented Apr 6, 2022

Would it be possible to pass a base prop to Switch and prepend that to the route when navigating/matching? The route given to pushState will be basename (from <base>)/{base}/route. Similarly, the route given to route-recognizer will be {base}/route

@ya-mouse
Copy link

I really like the way how it's implemented in Rocket with module configurations.

I'm looking for the similar approach in Yew. The JS loader imports separate WASM modules and mounts to corresponding paths. The modules are instantiaed e.g. by Wasmer.

@ctron
Copy link
Contributor

ctron commented Dec 1, 2022

So with the release of Yew 0.20 you also seemed to have killed the agent stuff, in favor of the "context API".

While I do see some reasoning for using the context API, or web workers, this makes the problem with the lacking nested router implementation even worse.

With Yew 0.19 is was at least possible to he keep the old implementation alive. Now basically the full router functionality is broken.

I do understand that you might be doing this in your free time, but so do others. And introducing obstacles like that make it hard to continue using Yew.

@ctron
Copy link
Contributor

ctron commented Dec 2, 2022

I started working on this in a new project: https://github.com/ctron/yew-nested-router … this isn't complete yet, don't expect too much yet.

@ranile ranile removed this from the v0.20 milestone Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-router Area: The yew-router crate ergonomics feature-request A feature request proposal
Projects
None yet
Development

No branches or pull requests

6 participants