-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Type safe state inheritance #1532
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
Conversation
I think the reason I didn't do it this way is that you then can't have fallback inheritance for nested routers that have a different state type. |
Yeah I ran into that here. I changed |
Well, I still somewhat strongly prefer the previous compromise because while you could be surprised about that too, the surprise with nesting a |
We discussed this on Discord (privately, sorry) and because of how state inheritance is implemented now, this can't actually cause the problem that I was worried about. I guess it makes it impossible to merge two routers with different state though? Maybe that's fine. It's at least not something that you notice when the service is already running 🙂 |
The changelog for this is very weird, I only noticed now. There is a double |
I was going to improve the discoverability of docs for how state works with `Router::merge` and `Router::nest` based on my comment in tokio-rs#1313, but tokio-rs#1532 removes `Router::inherit_state` so I discovered that the change I originally suggested isn't necessary anymore. I decided to make a few other minor documentation fixes while I was reading, however: * Corrected the docs for `Router::nest` to say that it nests another `Router` rather than a `Service`. There is a separate function (`Router::nest_service`) for the latter. * Changed one of the headings in nest.md to use more idiomatic English. * Changed awkward phrasing under the "Sharing state with handlers" heading of the root documentation page. * Removed a trailing period from one of three list items for consistency.
This change seems to have broken |
@zvkemp it didn't. You gotta call |
Yes, we are calling This method (as well as https://docs.rs/axum/0.6.0/axum/routing/struct.Router.html#method.into_make_service |
Oh actually I think I understand what is happening — I didn't realize that the signature of |
Exactly. Next time if you have an issue though, please open a discussion with a reproducible example instead of posting to a merged PR. |
Lately I've been wanting to explore a more type safe way to passing state to routers, that don't run things like #1516. It think its a little unfortunate that axum 0.6's headline feature is type safe state, only for
inherit_state
to not actually be type safe.This changes the API such that
Router::with_state
requires the state which is then passed to everything when building the finalRouterService
:The upside of this approach is that we don't need additional type parameters or runtime stuff to track whether the state has been supplied or not.
Nesting (and merging) is only possible with a
Router
which doesn't have the state so it becomes more type safe. You can still usenest_service
but that requires turning theRouter
into aRouterService
by supplying the state.@jplatte what do you think?
Fixes #1516
TODO