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 Hooks::before_routes to give user control over initial axum::Router construction #646

Merged
merged 10 commits into from
Aug 1, 2024

Conversation

swlody
Copy link
Contributor

@swlody swlody commented Jul 20, 2024

This function can be implemented by the user to fully control the construction of the axum router before it is handed back to loco. This is necessary in order to create a fallback handler that is covered by the loco middleware, as any fallback that is added in after_routes gets initialized after all the middleware in to_router. The default implementation of Hooks::before_routes results in the same behavior as before, but the user can override the implementation like so:

impl Hooks for App {
    // ...
    async fn before_routes(ctx: &AppContext) -> Result<AxumRouter<AppContext>> {
        // Anything that we want to add to the router before the middleware is initialized
        let router = AxumRouter::new().fallback(/* some fallback_handler */);
        Ok(router)
    }
    // ...
}

Resolves #643

@jondot
Copy link
Contributor

jondot commented Jul 23, 2024

@swlody this is really great!
I always try to think in terms of process with the hooks, it keeps them more agile. So think:

"before lunch" (getting food, getting place to eat)
"having lunch" (eating)
"after lunch" (cleaning up, doing some exercise)

It doesn't always work to "force" things into that kind of shape and form but in this case, I think it might be.

I feel this PR (covering how to customize fallbacks, etc), is actually:

before_routes(ctx: &AppContext) -> Result<AxumRouter>

Where the user will

  1. Customize / build / create their own initial version of Axum router, and finish at that
  2. We (loco framework) take that router, and add the routes on it, while continuing to add the rest of the layers internally

if the user wants to add stuff "after", they can use after_routes, as you've already identified

This keeps the complexity of "wait, after I built a new router, how to I add the routes?, what are the steps I need to follow to get to a fully working router? do I need to also call after_routes myself?" - this is a cognitive overhead that most users will have but "pro" users will probably manage (by viewing the source code or experimenting). We want to make sure most users actually do the right thing always ("the pit of success").

By offering the user just the thing they need ("e.g. use before_routes, create your own router and give it to us, we'll take it from there), we're ensuring a higher chance of success, and also we're making sure the framework (Loco) is always able to change things behind the scenes (order of initialization, adding more stuff into a router, etc.)

So to summarize:

  1. Change router to before_routes, keep same signature, expect the user to only return a custom-initialized router (with fallback or etc.)
  2. On the framework side, use this as a router construction method, and continue from there

Hope this wasn't too long, makes sense?

@swlody
Copy link
Contributor Author

swlody commented Jul 23, 2024

Thanks for the detailed feedback @jondot. I've renamed to before_routes as you suggested and added a reference to the function in the docs.

@swlody swlody changed the title Add Hooks::router to give user control over axum::Router construction Add Hooks::before_routes to give user control over initial axum::Router construction Jul 23, 2024
@swlody swlody closed this Jul 30, 2024
@swlody swlody reopened this Jul 30, 2024
@kaplanelad
Copy link
Contributor

thanks @swlody

@kaplanelad kaplanelad merged commit 367fd1f into loco-rs:master Aug 1, 2024
26 checks passed
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

Successfully merging this pull request may close these issues.

Add ability to provide fallback handler to AppRoutes
3 participants