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.

Sign up for GitHub

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

Jump to bottom

Remove or change impl FromRequest* for Option<impl FromRequest*>? #2298

Open
jplatte opened this issue Oct 31, 2023 · 2 comments · May be fixed by #2475
Open

Remove or change impl FromRequest* for Option<impl FromRequest*>? #2298

jplatte opened this issue Oct 31, 2023 · 2 comments · May be fixed by #2475
Labels
A-axum-core breaking change A PR that makes a breaking change. C-musings Category: musings about a better world I-needs-decision Issues in need of decision.
Milestone
0.8

Comments

@jplatte
Copy link
Member

jplatte commented Oct 31, 2023 •
edited

The possibility of using Option<Extractor> as an extractor has lead to a bunch of confusion because people have expected or wanted it to have different semantics for specific inner extractors. We should consider removing or altering the two implementations.

One option: We could have separate traits OptionalFromRequest and OptionalFromRequestParts and implement them only for a handful of types where it actually makes sense to discard the error. We could also implement different behavior, such as making Option<Path<_>> equivalent to the current OptionalPath from axum-extra. If you feel like this is worth exploring, I can open an issue.

This idea originally came up in #2281 (reply in thread).
@jplatte jplatte added C-musings Category: musings about a better world I-needs-decision Issues in need of decision. breaking change A PR that makes a breaking change. A-axum-core labels Oct 31, 2023
@jplatte jplatte added this to the 0.7 milestone Oct 31, 2023
@davidpdrsn davidpdrsn modified the milestones: 0.7, 0.8 Nov 10, 2023
@jplatte
Copy link
Member Author

jplatte commented Dec 30, 2023

@davidpdrsn How positive are you about the proposal from the second paragraph of the issue description? I could make a PR with that today or tomorrow.

@davidpdrsn
Copy link
Member

I think that sounds like a good path!

@jplatte jplatte linked a pull request Dec 30, 2023 that will close this issue
Draft
5 tasks
@jplatte jplatte mentioned this issue Apr 7, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-axum-core breaking change A PR that makes a breaking change. C-musings Category: musings about a better world I-needs-decision Issues in need of decision.
Projects
None yet
Milestone
0.8
Development

Successfully merging a pull request may close this issue.

Add a separate trait for optional extractors tokio-rs/axum
2 participants
@davidpdrsn @jplatte