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

feat(transport-commons): add context.http.response #2524

Merged
merged 4 commits into from
Apr 4, 2022

Conversation

vonagam
Copy link
Member

@vonagam vonagam commented Jan 9, 2022

Allows to specify any response headers for http transports.
Related to #2484.

Not sure about naming though. About response part.
If http is only about outputs (and any inputs go into params) then there is no need to specify that this is response headers.
If http can contain something besides outputs, then status part should be maybe renamed to responseStatus for consistency.

I think the first option is more attractive to me with short names - so status, headers and possibly location and cookies (those are headers too, but can be beneficial to add easier way to set them).

@vonagam vonagam force-pushed the feat-response-headers branch from 29f5c7f to 067152e Compare January 13, 2022 13:27
@daffl daffl changed the title feat(core): add context.http.responseHeaders feat(transport-commons): add context.http.response Apr 4, 2022
@daffl daffl force-pushed the feat-response-headers branch from 067152e to e95e11b Compare April 4, 2022 22:18
@daffl daffl merged commit 5bc9d44 into feathersjs:dove Apr 4, 2022
@daffl
Copy link
Member

daffl commented Apr 4, 2022

I think this one will work great. Since this allows for redirects, can we close #2484?

@vonagam
Copy link
Member Author

vonagam commented Apr 4, 2022

Seems so. Will do.

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.

2 participants