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

Url/Router - new API design #383

Open
4 of 7 tasks
MartinKavik opened this issue Mar 9, 2020 · 9 comments
Open
4 of 7 tasks

Url/Router - new API design #383

MartinKavik opened this issue Mar 9, 2020 · 9 comments
Labels
API design Proposal for a new or updated Seed API.

Comments

@MartinKavik MartinKavik added this to the 2. Stable basic Seed API milestone Mar 9, 2020
@MartinKavik MartinKavik added the API design Proposal for a new or updated Seed API. label Mar 9, 2020
@TatriX
Copy link
Member

TatriX commented Apr 20, 2020

What do you think about giving an iterator instead of custom methods?
Instead of

match url.next_path_part() {
   None => Page::Home,
   Some("report") => Page::Report(page::report::init(url)),
   _ => Page::Unknown(url),
}

something like

match url.into_path_iter().next() {
   None => Page::Home,
   Some("report") => Page::Report(page::report::init(url)),
   _ => Page::Unknown(url),
}

I used to ...collect::<Vec<&str>> an Url so I can

match segments {
            [""] => Screen::Home,
            ["about"] => Screen::About,
            ["profile"] => Screen::Profile,
            ["signin"] => Screen::SignIn,
            ["signup"] => Screen::SignUp,
...

UPD. found remaining_path_parts which does the job ;)

@flosse
Copy link
Member

flosse commented Apr 21, 2020

What do you think about giving an iterator instead of custom methods?

👍

what about

url.path().iter().next()

or

url.path().into_iter().next()

Instead of

url.into_path_iter().next()

@MartinKavik
Copy link
Member Author

@flosse : url.path().iter().next().map(String::as_str) is already usable. String::as_str is necessary now because I didn't want to create an extra vector for returning &[&str] instead of &[&String] from path().

I've implemented custom methods because I'm not able to come with clean API that integrates Iterators, it's efficient and mainly plays nicely with nested modules.
See examples url, pages, pages_keep_state, pages_hash_routing and auth. I'm open to new ideas. (Notice link building (macro struct_urls!), working with base url and nested routing).

@TatriX
Copy link
Member

TatriX commented Apr 21, 2020

For our work project I'm using very simple approach: there is a simple Screen enum which can be converted from Url and back to it. It works really nice from the "user" point of view. The only disadvantage is that From<Url> and Into<Url> should be kept in sync manually. Though I guess generating those via proc macros could solve the issue.

I checked pages_hash_routing example and conceptually it has all the same parts:

  • There is Page enum
  • You need to manually match url parts to construct it
  • And you have to write method for converting it to url

If I'd try to imagine how it should be then:

/// Derive `From<Url>`, `Into<Url>` and `Display`.
#[derive(Router)]
enum Page {
  #[route("/")]
  Home,
  #[route("/user/profile")]
  Profile,
  #[route("/posts")]
  Posts,
}

fn view() {
   a![ attrs!{ At::Href => Page::Home } ]
}

@MartinKavik
Copy link
Member Author

MartinKavik commented Apr 21, 2020

@TatriX How should I use it ^ with nested modules? Let's assume we have sitemap like:

- client_section (e.g. `www.example.com/client_section/[client_id]/`)
   - subscriptions
   - overview
      - detailed
      - summary
- about
- admin_section
  - content_management
  - reports (reports can be sorted and filtered by search params (e.g. `?sorted=name`))

And all pages have own Model, Msg, etc.

  • Do we have to use one central router?
  • How we can extract search params and dynamic path parts (e.g. client_id) from the Url?
  • How can I create a link for <a> in /admin_section/reports that points to /client_section/1234/subscriptions?
  • "For our work project" - I assume we can't see it (?).

@TatriX
Copy link
Member

TatriX commented Apr 21, 2020

"For our work project" - I assume we can't see it (?).

Yes, unfortunately I can't show it.

I haven't thought that much about nested modules, but I would just

mod user {
     pub enum Page {}
     fn view() {
        a![href => crate::admin::Page::Reports],
     }
}

mod admin {
   enum Page {
     Reports,
   }
}

Do we have to use one central router?

So yes, one Page/Router per module.

How we can extract search params and dynamic path parts (e.g. client_id) from the Url?

That's also something I need to find out at some point in future, but my first thought was to use approach similar to rocket:

/// Return `cane/hour` data for plotting.
#[get("/stats?<from>&<to>")]
pub fn get_stats(
    conn: db::Conn,
    from: String,
    to: String,
    _auth: Auth,
) -> Vec<Foo> {
}

Maybe something like this can be used to embed various params as well:

enum Page {
   Report{
      from: String,
      to: String,
   }
}
fn view() {
 a![ href => Page::Report{ from: todo!(), to: todo!() }]
}

@TatriX
Copy link
Member

TatriX commented Apr 21, 2020

And yet another thing: I was slightly confused about Url not being a real url, but rather some relative application specific thing. Maybe I'm wrong though.

My mental model works like this: you have Url which represents, well any valid url. It can be absolute or relative and I should be able to construct it as easily as possible:

let absolute = Url::new("https://google.com");
let relative = Url::new("/admin");

Something akin to Path from stdlib.

Then there is logical pages/screens or routing. I like to think of pages being independent from urls, but there should be one to one correspondence between them. So, Page is logical entity of my application, and Page can be converted to Url and vice versa. Maybe think about Url as a serialized version of Page. Of course, if Url is structured, then it doesn't really differ from Page, but you can say the same about struct and it's json representation.

@MartinKavik
Copy link
Member Author

MartinKavik commented Apr 21, 2020

@TatriX I see you have multiple ideas, but nothing proven/complete yet - so I suggest to release 0.7.0 with the current API (it should be good/bug-free enough).

And when you find out that it's limiting for you I would be happy to discuss your more-complete alternative for it (probably ideal would be rewritten pages example to see patterns in a "more complicated" SPA).
Your Fetch API was a big improvement so I have no doubt you will be able to design something better.

Some other notes:

  • I'm not big fan of #[get("/stats?<from>&<to>")] because it can be error prone, I can't easily compose it from consts/by builders and we would need to use another macro for it. But I can be wrong and I understand that it's pretty readable.

  • I was experimenting with many From/Into<Url> implementations in clients' projects and at the end it always becomes mess or too error prone or requires some macros or too implicit - but yeah, it depends on the project and maybe I can't use it properly.

  • you have Url which represents, well any valid url.

    Well, I don't have problems with it - https://github.com/seed-rs/seed/blob/master/src/browser/url.rs#L12-L17. I just didn't need it yet and it's modeled after ReactReason router - so Url that represents only relative URL should be proven concept.

@arn-the-long-beard
Copy link
Member

Hello guys.

I am working on the routing and I am on the way to start a PR.

Here are the ideas I have in my code for now and I will use some of the suggestion we have in the discussion :

  • Having strong unit test
  • Having a Router that centralize the navigation
  • Having Routes that we can define with options
  • Having the actual URL system inside the routing system under the hood

There are few challenges I am having :

  • How to properly unit test in the web browser for routing : How can I get the url in the unit test ?
  • I am beginner to Rust so there are a lot of optimizations, shortcuts and concepts that are new or unknown to me
  • I cannot make macros, i tried yesterday to understand it, and macros are way too much exotic and challenging for me at this stage.

This is work in progress, so I need all your inputs. Everybody is welcomed to help on it 😄

My objective is to have an easy way to declare routes and generated the code for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design Proposal for a new or updated Seed API.
Projects
None yet
Development

No branches or pull requests

4 participants