-
Notifications
You must be signed in to change notification settings - Fork 207
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
[http] Add Parameter Decorator @cookie
to Specify Cookie Parameters
#4761
base: main
Are you sure you want to change the base?
Conversation
85c896a
to
a6f8a1c
Compare
a6f8a1c
to
b9656c3
Compare
I think that |
aren't cookies allowed in response(server asking client to set the following cookies) we might need to design and decide exactly what we want in those cases though |
I noticed that cookies can also be allowed or required in responses (e.g. in communication between servers). |
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.
Thanks for the contribution this is awesome! I think there is a little design around the response cookies we'll want to sort out before we merge this but this looks great overall!
context.program.stateMap(HttpStateKeys.cookie).set(entity, options); | ||
}; | ||
|
||
export function getCookieParamOptions(program: Program, entity: Type): QueryParameterOptions { |
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.
would be awesome if you could add some js doc comment to those new functions, we are in the process of documenting all our api
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.
not asking you to do more work but just so you know this won't add support for it to openapi3 emitter if thats what you were looking for. Would need a few more changes in the packages/openapi3
pkg probably
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.
I tried it in local playground, and it works for me.
It seems there is no specific support for parameters in openapi3, so I think there is no need to add support.
I'll try to add some tests for openapi3 👀
Co-authored-by: Timothee Guerin <[email protected]>
close #4739
This PR adds
@cookie
decorator to@typespec/http
, to decorate parameters as cookie parameters.