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

Feature: on Go 1.22, fill (*http.Request).PathValue with URLParam data #873

Closed
AlexVasiluta opened this issue Nov 15, 2023 · 5 comments · Fixed by #901
Closed

Feature: on Go 1.22, fill (*http.Request).PathValue with URLParam data #873

AlexVasiluta opened this issue Nov 15, 2023 · 5 comments · Fixed by #901

Comments

@AlexVasiluta
Copy link

Hey y'all!
For Go 1.22, this enhanced ServeMux routing proposal was accepted and pushed into the main tree, and *http.Request has some new methods, SetPathValue and PathValue. I think r.PathValue(...) could act as an alias for chi.URLParam(r, ...) when routing with chi.

There would probably need to be some logic with //go:build go1.22 that runs SetPathValue when inserting keys/values into chi.Context.URLParams for the corresponding http.Request. I have not dug deeper into chi's source code, but I think this would be a welcome addition for when Go 1.22 is released.

@pkieltyka
Copy link
Member

most definitely will add support for the r.PathValue(..)

@angelofallars
Copy link
Contributor

angelofallars commented Feb 12, 2024

I just opened pull request #901 for this

@ptman
Copy link

ptman commented Feb 13, 2024

@angelofallars that seems to be an urelated PR, maybe #901 instead?

@angelofallars
Copy link
Contributor

@ptman Whoops, tagged the wrong PR. Thanks for pointing that out

@ptman
Copy link

ptman commented May 3, 2024

BTW, it is sometimes handy that you can get the url param from the context instead of from the request. So thanks for keeping chi.URLParamFromCtx while still supporting r.PathValue!

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 a pull request may close this issue.

4 participants