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

Router/history library #13

Closed
fitzgen opened this issue Feb 26, 2019 · 28 comments
Closed

Router/history library #13

fitzgen opened this issue Feb 26, 2019 · 28 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Feb 26, 2019

Needs some exploratory survey and design work!

  • # fragment?
  • pushState and history API integration?
@Pauan
Copy link
Contributor

Pauan commented Mar 13, 2019

My understanding is that routers have pretty universally moved to pushState style.

@David-OConnor
Copy link
Member

David-OConnor commented Mar 13, 2019

Advantages of PushState over Hash, from MDN:


- The new URL can be any URL in the same origin as the current URL. In contrast, setting window.location keeps you at the same document only if you modify only the hash.
- You don't have to change the URL if you don't want to. In contrast, setting window.location = "#foo"; creates a new history entry only if the current hash isn't #foo.
- You can associate arbitrary data with your new history entry. With the hash-based approach, you need to encode all of the relevant data into a short string.
- If title is subsequently used by browsers, this data can be utilized (independent of, say, the hash).

I went through several implementations of routing implementation and API, and settled on one inspired by ReasonML:

The route is described by a Url struct which contains a Vec<&str> which describes the relative path, hash, search, and the (by all browsers?) unimplemented title property. The app-creator passes a function when initializing the app, that accepts at Url, and uses whatever logic he/she likes to determine what state-change message to return. Pattern matching ins a nice approach.

Initial routing is fed through the 'routes' function. Two ways to initialize routing.
A: Give link etc s an event listener (eg Click) which calls a Gloo::pushRoute (accepts a Url, eg if need to use hash/search), or Gloo::pushPath (accept a Vec<&str>; simpler, and should be majority use case)) The state change logic then calls pushState., then triggers the state change logic.

B: Any element given a 'href' attribute, will trigger routing according to the routes fn when clicked. Non-standard HTML, but I think it provides a concise API.

Details

@David-OConnor
Copy link
Member

David-OConnor commented Mar 13, 2019

Implementation details: We use serde to serialize the URL struct into a JsValue, which is passed to web_sys::window::push_state_with_url, then deserialized in a popstate listener, which handles fwd/back events.

I'm not sure the best way to make this standalone, since it involves calling a framework-specific update function. Overall, need to address what Gloo's endpoints are before adding a PR.

@Pauan
Copy link
Contributor

Pauan commented Mar 14, 2019

I'm not sure the best way to make this standalone, since it involves calling a framework-specific update function.

Perhaps it should return a Stream of updates? That would allow it to work with anything which supports Futures.

@David-OConnor
Copy link
Member

David-OConnor commented Mar 14, 2019

I don't know much about Futures/Stream, but doesn't it imply a (potential) delay? Doesn't seem semantically appropriate. Maybe an Update, or RouteUpdate trait? Might be too opinionated.

@Pauan
Copy link
Contributor

Pauan commented Mar 14, 2019

There is a 1-tick delay when you start listening to updates, but there isn't any delay when an update occurs. I think the semantics match very well.

Another option is to make the URL a Signal, which also matches very well.

@David-OConnor
Copy link
Member

David-OConnor commented Mar 14, 2019

I mean, Futures, as described on this page, are intended to be used for something that may be slow like network requests, which routing is not.

My understanding of Futures/signals are both weak, but I get the impression this could be a drop-in replacement where we currently call the framework-specific update function. Need to figure out how to replace all occurrences of app in this file, with something generic.

Also need to verify that search/hash is working properly, as I'm not sure how they're intended to be used. Eg using location.hash vs appending # + the hash route to the path.

@Pauan
Copy link
Contributor

Pauan commented Mar 14, 2019

I mean, Futures, as described on this page, are intended to be used for something that may be slow like network requests, which routing is not.

Futures and Streams have nothing to do with "slowness", they have to do with asynchronous values.

An asynchronous value is a value which isn't here now, but will be here in the future. By that definition, of course a router update is asynchronous.

There are plenty of Futures which are extremely fast (including Futures like ready which are actually synchronous).

As for Streams, they're basically a way of retrieving multiple asynchronous values, which makes them perfect for router updates (since there can be multiple router updates, and each update is asynchronous).

@David-OConnor
Copy link
Member

David-OConnor commented Mar 14, 2019

Based on your description, perhaps Futures/Stream can be applied more broadly in how Gloo modules interact with each other and outside code...

@David-OConnor
Copy link
Member

#26

Sorted using generics

@David-OConnor
Copy link
Member

David-OConnor commented Mar 19, 2019

Ref #26

Summary

Add a frontend router, based on a Url struct, and a user-passed function which accepts this struct, and return a generic type indicating how to handle the route.

Motivation

First router proposal for Gloo

Detailed Explanation

I propose adding a router similar to the one described in the PR above. It uses a Url struct, which contains fields path: a Vec<&str>, which is the route entered by the user, and optionally a hash, search, and title. To initiate routing, the app creator calls push_route() with a Url as the arg, or push_path(), with the path as the arg. The app creator also passes a function which accepts a Url, and outputs a generic type. (sig fn(&Url) -> Ms), where Ms semantically implies a message, but can mean anything to the framework. This func can be structured in any way.

An additional endpoint is a function which stores a Closure, which is used for memory purposes. Sig impl Fn(Closure<FnMut(web_sys::Event)>) + 'static).

A (more unconventional, and perhaps controversial) API is also availible, which allows any element with an Href attribute, and local path to initiate routing. This is implemented by parsing local paths (Defined as starting with /) to a URL, preventing a page refresh, and triggering routing as above. This provides a concise API.

Overall, I think the most appealing part of this approach is its external API.

Drawbacks, Rationale, and Alternatives

There are other ways to design a router; by nature this is opinionated, and there may be improvements, or better designs proposed. Some framework styles may not work well with the generic types used as endpoints. Applying Href to non-anchor elements is non-standard HTML.

This approach is inspired by reason-react's router and designed to avoid mistakes that make react-router clumsy.

Unresolved Questions

I have a poor grasp on how hash and search work. Their implementation may be buggy or incorrect, and is absent for the Href api. Additionally, there may be other ways to handle prepended /s in path and Href.

@fitzgen
Copy link
Member Author

fitzgen commented Mar 19, 2019

Thanks for writing this up @David-OConnor!

First off, I'm not super familiar with client side routers and the design space, so I appreciate y'all's patience with me :)

I have a few questions:

  • Can/should we reuse the url crate for URLs?

  • Can we slice this proposal up into layers, similar to the timers crate and the stuff we've been talking about in How deeply should we integrate with the ecosystem? #27? That is, a layer above the web-sys bindings that is a direct translation of the history API into Rust types (eg FnMut instead of js_sys::Function, etc), a layer on top of that uses futures or streams (if that even makes sense), and then finally the high-level router API layer?

There are other ways to design a router; by nature this is opinionated, and there may be improvements, or better designs proposed. Some framework styles may not work well with the generic types used as endpoints. Applying Href to non-anchor elements is non-standard HTML.

What are the other designs? What are their pros and cons? I don't feel like this proposal quite gives me enough to have an informed position on the topic.

If there are multiple high-level designs we could potentially implement, can they all be implemented well on top of our just-above-web-sys onion layer? I think it is important that is possible, before we even get to the higher-level bits.

This approach is inspired by reason-react's router and designed to avoid mistakes that make react-router clumsy.

Which mistakes are these? Do they consider it a mistake that they keep around for backwards compatibility? (Want to make sure we aren't "simplifying" an API by accidentally making valid use cases unsupportable.)

@David-OConnor
Copy link
Member

David-OConnor commented Mar 24, 2019

My thoughts are mixed on the url crate. It includes useful methods like extracting the Host, Origin etc, and using prior work is a good default. The approach I used is deliberately terse: The Url struct included, with 4 fields and no methods, contains, what I believe to be all we need. (It's path field is all that's required for many use cases, hash may see some use, search perhaps, and title, perhaps in the future) I'm suspicious I'm missing something, and that using the crate would be a better long-term solution. @jesskfullwood wrote a spin-off of the module I PRed which uses the crate, and could provide better comparison/reasoning.

Re splitting into layers: What immediately comes to mind is the recent discussion about event systems. This (and probably most/all) routing approaches involve a popstate listener. If we could expose a higher-level layer instead of impl Fn(Closure<FnMut(web_sys::Event)>) + 'static, I think we should. Same if we wrap things like:

    web_sys::window()
        .expect_throw("Can't find window")
        .history()
        .expect_throw("Can't find history")
        .push_state_with_url

with a cleaner API.

Other routing approaches that come to mind: parsing URLs using regex. (eg Django) Using HashMap instead of a routing function (Probably intractable due to lack of flexibility/dynamic routing). Maybe a HashMap that includes a Vec<&str> for path as keys, and either urls, or generics (or Steams...?) as values. More I'm sure I'm missing.

An alternative for the href attribute: We could enable it only for anchor elements, and use the href property instead, which may be more robust. This avoids the non-standard HTML, while preventing clicking local links from initiating a page reload, and allowing the concise API, albeit only for text links.

react-router requires wrapping large blocks of view syntax (ie components) that use routing in special components. Subjectively, this is an odd mix of declarative view syntax, and routing/state-management. it causes extra indent levels, and consideration over which parts of the view code should be wrapped. I think the approach I proposed is less coupled to the view, and integrates more naturally with an event system (via the generic), or with HTML links via the href mimic.

My biggest concern: There are probably use-cases my proposal doesn't handle elegantly, or at all, which I haven't thought of. There are probably better ways to design the API and internal code, that I reject for no reason other than not being aware of them. My experience with routing is limited: We need other opinions on this, from people experienced with different styles of routing who can better criticize this plan, and offer improvements/alternatives.

I'd specifically like feedback from someone who uses hash and search. Are they obsolete? Still relevant? If parsing a string that includes them, do we include them in push_state's url field, location.set_search / location.set_hash, or both?

@Pauan
Copy link
Contributor

Pauan commented Mar 24, 2019

react-router requires wrapping large blocks of view syntax (ie components) that use routing in special components.

There is an advantage of that: it allows you to have a header/footer that is the same on all pages, thus only the middle part of the page changes on route changes.

I don't have strong opinions about routing, since I personally haven't used routing much. So take what I say with a grain of salt.

@Aloso
Copy link

Aloso commented Mar 25, 2019

@David-OConnor I don't like the names Gloo::push_path() and Gloo::push_route(). Could we make a generic push_state() that accepts either a Vec<&str> or a Url? This would also make it more flexible and extendable.

I guess this would work:

fn push_state<U: Into<Url>>(url: U)

@Aloso
Copy link

Aloso commented Mar 25, 2019

Actually, the history API allows you to omit the URL when calling pushState(), so maybe we should similarly also accept a UrlUnchanged value.

Here are the things of the History API that aren't covered yet:

  • History.replaceState()
  • The state object
  • The 2nd argument title (which is used in Safari and, at least I think, in Firefox, in the navigation dropdown)
  • back() / forward() / go(steps)
  • History.length

@Aloso
Copy link

Aloso commented Mar 25, 2019

If parsing a string that includes them, do we include them in push_state's url field, location.set_search / location.set_hash, or both?

When you call History.pushState() with a URL, you don't have to modify window.location. That also applies to search and hash parts AFAIK.

Another question: Do we even need to parse URLs? This is already done by the browser in window.location

@David-OConnor
Copy link
Member

David-OConnor commented Mar 25, 2019

Agree on generic push_state(). We can ditch push_path() entirely, and the push_route() name was intended to be re-exported in a framework, which doesn't make sense here. (ie glooframework::push_route makes sense, but gloo::routing::push_route doesn't) What do you think of push() ?

In the PR I submitted, title can be passed by the user, ie using push_route with a Url literal, but it doens't interact with anything. Do you know how we'd use it? The MDN docs call it "ignored", and "Firefox currently ignores this parameter, although it may use it in the future. Passing the empty string here should be safe against future changes to the method. Alternatively, you could pass a short title for the state to which you're moving." The state object is how the URL is stored/serialized for use in the popstate listener; thoughts on how to handle that if state could be set in the api? Maybe have it be a (serialized) struct with fields for url and api, or url, and whichever ones a user sets?

URL parsing is only required for the Href API, and perhaps for cases we haven't considered.

The hash and search consideration applies when pushing. Do we do this:

    if let Some(hash) = url.hash {
        location
            .set_hash(&hash)
            .expect("Problem setting hash");
    }

    if let Some(search) = url.search {
        location
            .set_search(&search)
            .expect("Problem setting search");
    }

Or when setting the path for push_state_with_url:

let mut path = String::from("/") + &url.path.join("/");
if let Some(hash) = url.hash {
    path += "#" + &hash;
}

@Aloso
Copy link

Aloso commented Mar 26, 2019

The MDN docs call it "ignored"

I thought Firefox used title in the right-click menu of the back-button, but maybe I was wrong. But I read that Safari uses it.

state doesn't contain the URL. It's just an object associated with the URL, as a way to pass additional information to the popstate listener. I don't know how the API for this should look like in Rust though.

The hash and search consideration applies when pushing.

I wasn't sure what you meant, so I looked into your PR (which was reverted): When push_route is called, you call history.pushState() and set location.hash/location.search, which is wrong. Instead, generate a URL, like /my/path?q=Hello+World&page=50#abstract, and pass it to history.pushState(). Don't modify location when working with the History API.

@yoshuawuyts
Copy link
Collaborator

yoshuawuyts commented Mar 26, 2019

Hi! -- I just saw this thread, and have done quite a bit with the History API in the past, so this is an attempt at sharing some of my experiences. I hope this can be of help for Gloo's design!

Events / Hooks

In Choo we ended up having about 5 events for interacting with the router (ref impl):

  • navigate: Choo emits this event whenever routes change. This is triggered by either
    'pushState', 'replaceState' or 'popState'.

  • pushState: This event should be emitted to navigate to a new route. The new route is added
    to the browser's history stack, and will emit 'navigate' and 'render'.
    Similar to history.pushState.

  • replaceState: This event should be emitted to navigate to a new route. The new route replaces
    the current entry in the browser's history stack, and will emit 'navigate'
    and 'render'. Similar to history.replaceState.

  • popState: This event is emitted when the user hits the 'back' button in their browser.
    The new route will be a previous entry in the browser's history stack, and
    immediately afterward the'navigate' and 'render'events will be emitted.
    Similar to history.popState.

  • DOMTitleChange: This event should be emitted whenever the document.title needs to be updated. It will set both document.title and state.title. This value can be used
    when server rendering to accurately include a <title> tag in the header.

Even if Gloo's router might not be event-based, this should cover the base functionality for interacting with the browser's history API.

Triggering route changes

The way people would trigger route changes was actually kind of nice: instead of using custom types that needed to be passed around, we declared a global listener for all click events, and would figure out if it came from an anchor tag pointing to an address on the same host.

In practice this meant that connecting to the router was as simple as:

app.view('/my-route', (state, emit) => {
  html`
    <body>
      <a href="/other-route">Go to other route</a>
    </body>
  `
})

This turned out to be a rather nice model to work with, and people often seemed pleasantly surprised at how little boilerplate was required to get things to work.

Other Notes

Regarding hash routing: we added optional support for it in Choo (by setting an option), but I think in general it's not a hard requirement. Especially when combined with a server that was able to handle view rendering on the same routes as the browser.

Which brings me to another point: it's good to think about ways of exporting the routes declared in Gloo, so they can be used in servers. The exact ways of doing this don't matter too much, as long as it's something that's considered from the start. If not it can become tedious to sync routes between two code bases.

And finally: we've experimented a lot with the state object in the browser's history API, but it ended up being rather clunky to work with. In the end just keeping application state outside of it was much easier to reason about, and lead to fewer bugs. We probably didn't quite nail the UX for changing titles, but there's probably a way of using traits to make that a bit nicer.

Conclusion

I by no means have all answers for how we should design a router in Rust. But I hope that sharing my experiences of building a routing story in JS can help in Gloo's design. I hope this is helpful!

@David-OConnor
Copy link
Member

David-OConnor commented Apr 7, 2019

What changes would y'all like to make this happen? Or are we waiting for alternative designs/implementations to compare with?

@fitzgen
Copy link
Member Author

fitzgen commented Apr 8, 2019

What changes would y'all like to make this happen? Or are we waiting for alternative designs/implementations to compare with?

To me, I still have a little bit of fuzziness with the design, and I think the proposal would benefit from an explicit API skeleton that we've been formalizing in #63 and #65

However: I'm not super familiar with client-side routing (at this point I have more experience with "client" / Web frontend programming inside the firefox devtools rather than with a Web page talking to a server, and it isn't quite the same even though it is all Web technologies in both cases; e.g. we never needed routing in the devtools). Therefore, I think it would be helpful if some other Gloo team members could step up and be the design reviewers for this proposal. @yoshuawuyts @Pauan would you two be willing to do that?

@yoshuawuyts
Copy link
Collaborator

yoshuawuyts commented Apr 9, 2019

@fitzgen would be happy to! -- I like the proposal skeleton we have a lot (:

@richard-uk1
Copy link
Contributor

richard-uk1 commented Apr 19, 2019

I'd like whatever gloo decides for routing to be modular. One part of that for me is some macros for generating maps from Route -> url and from url -> Route.

I'm doing some experimenting along these lines (see https://github.com/derekdreery/route-exp/blob/master/examples/simple.rs). That's kinda how I would like it to look. Take a look and tell me what you think of the design. (I haven't implemented parsing yet, only Route -> url, and then it's very basic, just using Display of the fields. But the principal is there)

@bschwind
Copy link

bschwind commented May 8, 2019

At the request of another reddit user, I'm posting my project here:

app_route

I wouldn't say it's stabilized yet, but feel free to either use it directly or take pieces of the code that may be helpful. I especially don't like the AppRoute trait at the moment and will probably alter its signature in the future.

It's interesting to see an enum approach (in route-exp) to that problem as well, I hadn't thought of that.

@richard-uk1
Copy link
Contributor

I'm also having a play in this space: I've published rooty as a package for parsing/serializing routes. It works slightly differently to @bschwind.

Eventually we'd want to unify all these efforts, but for now I think it's good to experiment.

I've also been playing with implementing history in rust. I haven't bothered with the hash version though, as all browsers that support wasm support the history API.

Also I have to say that I love syn. You can parse rust very easily. The rooty crate should provide nice error messages with relevant spans.

@David-OConnor
Copy link
Member

Thoughts on getting a not-perfect-but workable module released?

ranile added a commit that referenced this issue Feb 15, 2022
When the WebSocket is used with frameworks, passed down as props, it might be `drop`ed automatically, which closes the WebSocket connection. Initially `Clone` was added so sender and receiver can be in different `spawn_local`s but it turns out that `StreamExt::split` solves that problem very well.

See #13 for more information about the issue
@ranile
Copy link
Collaborator

ranile commented Feb 15, 2022

Done in #171 & #178. Actual use of these crates can be found in yew-router

@ranile ranile closed this as completed Feb 15, 2022
ranile added a commit that referenced this issue Feb 16, 2022
* Initial commit

* provide a better interface for errors, rename `RequestMethod` to `Method`

* remove method for array buffer and blob in favor of as_raw

* prepare for release

* add CI, update readme

* hide JsError in the docs

* fix CI?

* Install wasm-pack in CI

* misc

* websocket API

Fixes: ranile/reqwasm#1

* add tests for websocket

* update documentation, prepare for release

* fix mistake in documentation

* Rewrite WebSockets code (#4)

* redo websockets

* docs + tests

* remove gloo-console

* fix CI

* Add getters for the underlying WebSocket fields

* better API

* better API part 2 electric boogaloo

* deserialize Blob to Vec<u8> (#9)

* Update to Rust 2021 and use JsError from gloo-utils (#10)

* Update to Rust 2021 and use JsError from gloo-utils

* use new toolchain

* Add response.binary method to obtain response as Vec<u8>

Fixes: ranile/reqwasm#7

* Remove `Clone` impl from WebSocket.

When the WebSocket is used with frameworks, passed down as props, it might be `drop`ed automatically, which closes the WebSocket connection. Initially `Clone` was added so sender and receiver can be in different `spawn_local`s but it turns out that `StreamExt::split` solves that problem very well.

See #13 for more information about the issue

* Rustfmt + ignore editor config files

* Fix onclose handling (#14)

* feat: feature gate json, websocket and http; enable them by default (#16)

* feat: feature gate json support

* feat: feature gate weboscket api

* ci: check websocket and json features seperately in CI, check no default features

* feat: feature gate the http API

* refactor: use futures-core and futures-sink instead of depending on whole of futures

* ci: test http feature seperately in CI

* fix: only compile error conversion funcs if either APIs are enabled

* fix: add futures to dev-deps for tests, fix doc test

* 0.3.0

* Fix outdated/missing docs

* 0.3.1

* Change edition from 2021 to 2018 (#18)

* Change edition from 2021 to 2018

* Fix missing import due to edition 2021 prelude

* hopefully this will fix the issue (#19)

* There's no message

* Replace `async-broadcast` with `futures-channel::mpsc` (#21)

We no longer need a multi-producer-multi-consumer channel. There's only one consumer as of ranile/reqwasm@445e9a5

* Release 0.4.0

* Fix message ordering not being preserved (#29)

The websocket specification guarantees that messages are received in the
same order they are sent. The implementation in this library can violate
this guarantee because messages are parsed in a spawn_local block before
being sent over the channel. When multiple messages are received before
the next executor tick the scheduling order of the futures is
unspecified.
We fix this by performing all operations synchronously. The only part
where async is needed is the conversion of Blob to ArrayBuffer which we
obsolete by setting the websocket to always receive binary data as
ArrayBuffer.

* 0.4.1

* move files for gloo merge

* remove licence files

* gloo-specific patches

* fix CI

* re-export net from gloo

Co-authored-by: Michael Hueschen <[email protected]>
Co-authored-by: Stepan Henek <[email protected]>
Co-authored-by: Yusuf Bera Ertan <[email protected]>
Co-authored-by: Luke Chu <[email protected]>
Co-authored-by: Valentin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants