Change response types to ensure proper status and headers are set #11
Conversation
|
I would also be interested in your thoughts, @alabs-tomscholz |
luqasn
force-pushed
the
more-type-safety
branch
from
4040c68 to
9ea66cd
Compare
| export type ResponseBody<OperationId extends keyof operations> = | ||
| ValueOf<Property<ValueOf<Property<operations[OperationId], 'responses', EmptyObject>>, 'content'>, void>; | ||
| export type OperationResponse<OpName extends keyof operations> = { |
There was a problem hiding this comment.
Perhaps stay consistent with naming OpName OperationId instead?
| ValueOf<Property<ValueOf<Property<operations[OperationId], 'responses', EmptyObject>>, 'content'>, void>; | ||
| export type OperationResponse<OpName extends keyof operations> = { | ||
| [Status in keyof operations[OpName]['responses']]: | ||
| operations[OpName]['responses'][Status] extends { |
There was a problem hiding this comment.
Matter of style, but personally I prefer to name keys in mapped types P or K or similar to make it clear, but no big deal.
I would perhaps also consider extracting a type alias such as:
type OperationResponseMap<OperationId extends keyof operations> = operations[OperationId]['responses'];
... to also make it clearer since this is duplicated several times.
| "scripts": { | ||
| "pretest": "npm -w ../lib run build && openapi-ts-backend generate-types api.yml src/gen", | ||
| "test": "LOG_LEVEL=${LOG_LEVEL:=error} jest" | ||
| "test": "tsc && LOG_LEVEL=${LOG_LEVEL:=error} jest" |
There was a problem hiding this comment.
why should the test script implicitly compile? Not that it hurts, but seems a bit unnecessary. Or at least move it to pretest?
(Or is ts-jest not used so jest runs on the transpiled files? Too long since I worked on this so I don't remember. If so perhaps a better solution is to fix the jest setup?)
There was a problem hiding this comment.
I added it because the tests would also pass when there were typescript errors between the handler implemented in the test and the generated code. But you are right in that calling tsc here manually seems random and like a crutch.
2 participants
Hi Henrik,
thanks again for your great library!
One thing that was bothering me when working with it was that I kept forgetting to set the correct content type for the responses and then diligent tools (like when using supertest I think) said "what
body? there is no json here!".So I thought "can't be too hard to fix this, we have the complete api definition!". Turns out my TypeScript foo is not as great as I hoped, so I employed ChatGPTs help for figuring out the correct shapes. It was a give and take though, as I manually had to fix a couple things.
So the type system now makes you return the complete response (
statusCode,body,content-type), not only the body. And the type system enforces that those match the api definition.Would love to get your thoughts on this experiment! It does make things a bit stiffer, but I think with the
jsonhelper I introduced it should not be much more verbose in most common api usecases.