-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add closure example #3286
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
base: main
Are you sure you want to change the base?
Add closure example #3286
Conversation
This demonstrates using associated methods as handlers or middleware.
It's cool that we can do this, but personally, I don't think we should guide people towards this. I get that some people are confused about |
Macro: makes sense, I just added it because why not. Essentially it's mostly about demonstrating how to use associated methods for handlers and middleware (with the fn_mut caveat). That could even be a documentation PR that essentially documents
and
I get what you're saying about users being confused about the relationship between Arc and State but I also feel that State is not always the most appropriate place, especially in larger architectures. Even with partial state extraction the state is still there - when building e.g. an auth layer I usually like the config of that middleware being available only to that particular struct. Not sure how to proceed. What do others think? |
FWIW: I seemed to have missed that it is somewhat documented (but somewhat confusingly maybe for new users in the context of accessing State) for handlers. |
the code misses the point, in my opinion, as it's not commented on the |
Fair. Does it make sense generally in your opinion to document this pattern? If yes, as code using a more concrete example (not just Foo & Bar - I'll probably use authentication as an example) or rather as documentation? |
I don't mind the |
The example is now concrete, without abstract identifiers and a without a macro.
Made the example more simple and more concrete. Does that make sense? Furthermore: does it make sense to include a macro in axum that makes such a pattern more ergonomic or is there a general sense (like @mladedav indicated) that this is an anti-pattern (I'd argue not)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//! can be useful to create routes and middleware directly on specific structs (as opposed to | ||
//! handling everything in a central application state). | ||
//! | ||
//! This example demonstrates how do that using closures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//! This example demonstrates how do that using closures. | |
//! This example demonstrates how do that using closures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's an interesting approach
I tend to agree with @mladedav. Having too many ways to achieve the same thing tends to cause confusion at some point. If we were to add this we should at least explicitly document in which situation this is appropriate compared to the recommended approach of using router state. |
Not entirely sure how the consensus process here works - happy with the PR being merged but I can also rework it into documentation. Let me know (or merge)! Regarding the remark from above @Turbo87 - we approach state management roughly like this:
|
This demonstrates using associated methods as handlers or middleware, hopefully in a sensible fashion.
Motivation
Axum is quite powerful but (especially to beginners) it's not very straightforward to understand in terms of API. This example demonstrates a potential solution on using associated methods as an alternative / next to state.
Solution
Just an example with a simple macro. This could be changed / further expanded e.g.
Let me know if that makes sense (at all) and if I should expand on that.