seed icon indicating copy to clipboard operation
seed copied to clipboard

Routing + link building + Url API

Open MartinKavik opened this issue 5 years ago • 10 comments

Routing + link building + Url API are ones of the hardest things to design well and they are blockers on the way to the stable Seed core api. Many related PRs and issues have been already created so I want to unite our effort in this "tracking" issue.

PRs:

  • Working out on routing - https://github.com/seed-rs/seed/pull/536
  • Url API changes - https://github.com/seed-rs/seed/pull/472

Issues:

  • Suggestions for Url-Api - https://github.com/seed-rs/seed/issues/433
  • Url::(en/de)code - https://github.com/seed-rs/seed/issues/424
  • Url/Router - new API design - https://github.com/seed-rs/seed/issues/383
  • Seed shouldn't intercept links with target attribute - https://github.com/seed-rs/seed/issues/497

Motivation for the current routing

The previous API supported the app architecture, where you have one central router with Routes. The Route was created from the Url and then the associated Page / page Model was created from this Route.

Problems:

  • The "wiring" among Url, Page and Route often introduces a lot of boilerplate (especially From implementations).
  • The central router doesn't work very well when you have many (nested) pages with dynamic parameters. You need to import many items from different pages to handle those Url -> Route -> Page transformations and it breaks encapsulation and introduces a lot of boilerplate.
  • You also need to implement the reverse process - build links from Routes. It means all pages have to have access to the central router - it breaks encapsulation even more and introduces another boilerplate when you want to have typed links (and you want to have typed links).

Also the previous routing / Url didn't take into account a custom base url path. So it was a Seed user's problem to implement it properly, unfortunately.

Feedback for the current routing / urls:

  • Url doesn't represent the entire url (it doesn't contain a domain, protocol, port, etc.).
  • It's surprising that Url is "stateful". (It's stateful because it works a bit like iterators.)
  • Link building and routing are independent. It may cause bugs when the developer accidentally implements them differently for the same route.
  • Routing is a bit exotic. Especially for guys coming from component-based frameworks because they aren't used to parse & match urls.

MartinKavik avatar Sep 10 '20 12:09 MartinKavik

Hi, I just released the enum_paths crate, which might make defining routes easier. Maybe we can use it? Basically it derives methods for nested enums to convert them from and to / separated strings.

mankinskin avatar Sep 25 '20 21:09 mankinskin

This is a router that I implemented using subscriptions to the current subs::UrlChanged messages and enum_paths: https://github.com/mankinskin/binance-bot/blob/master/src/client/router/mod.rs

I am not sure if there is anything missing, but all of these work:

  • clicking links
  • changing the url in the browser search bar
  • opening site with specific url
  • navigating history

Also there is no generated mapping between enum Route and enum Page, which causes some boilerplate. But it is not as tedious as converting strings to an enum, and I am not sure how to implement such a mapping correctly, so that it works for all cases and isn't limiting.

There also is the boilerplate in the update function of a parent component, because those always need to forward proxy messages to their children. But that is another issue.

mankinskin avatar Sep 30 '20 20:09 mankinskin

Another point might be refactoring the Url type, to make it more compatible with url paths in String shape and make it contain the full browser url including protocol, host, and all kinds of queries, paths, etc.

mankinskin avatar Sep 30 '20 20:09 mankinskin

For anyone interested, my components crate just gained a generic Router component. It listens to UrlChanged and will switch the page by parsing the url path to a Route and initializing the page with it. It is pretty simple, but should do the job. It does require the components crate though.

mankinskin avatar Nov 10 '20 00:11 mankinskin

Hello guys,

Well seed_routing is under active development and already has several features. Any help is welcomed :wink:

Here are 2 issues I have just created : https://github.com/arn-the-long-beard/seed-routing/issues.

There are several steps we need to complete before publishing it as a crate or integrating into Seed

  • Good naming for Apis.
  • Enough integration/unit test.
  • Enough features.
  • Solid and maintainable implementation.
  • Good documentation

Let me know if you have some inputs and/or have questions :wink:

arn-the-long-beard avatar Dec 28 '20 14:12 arn-the-long-beard

Hello guys,

We are still working on the seed_routing and making improvements and polishing it.

Hey @mankinskin , would you like to take a look at it ? :smile:

arn-the-long-beard avatar Feb 16 '21 11:02 arn-the-long-beard

Hi @arn-the-long-beard !

Wow, looks like you've come a long way since I last looked into that. Personally, I would keep polishing this and provide it as a decent utility crate, and maybe look to integrate it into seed natively in the future.

The seed_routing crate adds a lot of features to remove boilerplate, and usually this comes at the cost of taking away some flexibility as you are fixing some rails for fast progress. I think there are basic tools in seed already for handling routing and building your own routing component, so this is not something with high priority for me right now. But this can certainly make building apps with seed faster, when it is well integrated.

One thing I am still confused about is why you built a feature for managing the browser history, when the browser has a built in system for that? Does it do anything differently? Do you want to bypass that and do processing on the browser history on your page? Would be glad if you could explain this to me :)

mankinskin avatar Feb 16 '21 14:02 mankinskin

Thank you for your message :+1:

One thing I am still confused about is why you built a feature for managing the browser history, when the browser has a built in system for that? Does it do anything differently? Do you want to bypass that and do processing on the browser history on your page? Would be glad if you could explain this to me :)

It was written as requirement in one of the issue https://github.com/seed-rs/seed/issues/383 maybe I misunderstand it . But yeah put it this way, it looks very useless that I built it. I forgot we could have access to it this way https://developer.mozilla.org/en-US/docs/Web/API/Window/history actually.

On the other hand, I have never used the one directly from the browser in Angular and I have just discovered from your post that it is available in web_sys. Used the router from Angular to do this . I got a bias it seems.

I do not know what to think about it right now. Maybe the whole thing is not needed.

arn-the-long-beard avatar Feb 17 '21 10:02 arn-the-long-beard

Ah okay, that clears it up, I kept wondering if I was missing something. I have never used Angular, only React a very little bit, so this seemed strange to me. But an abstraction of the browser history is actually really useful I guess, so you don't have to use low-level web_sys all the time. But I think it should really just be a shell to the browser history and not have any state itself, if it isn't for caching or things like that.

mankinskin avatar Feb 17 '21 13:02 mankinskin

Yes it makes sens. Probably what I was using in angular was calling the browser Api actually. I am almost sure now.

arn-the-long-beard avatar Feb 17 '21 13:02 arn-the-long-beard