-
Notifications
You must be signed in to change notification settings - Fork 67
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
Why are request headers defined as an array instead of a ZodObject? #80
Comments
I think for consistency given how these are declared in openapi it think it would make sense if it was also made an |
Understood that would be a breaking change, unless you can simply do an array check and allow for either interfaces. Side question: Could you provide insight how you guys are using this project beyond the "hello world" use case? I'm not sure if you guys have any future development planned, but contributing to this project is something that I'd be pretty interested in helping with. |
Can't speak for the other guys - I'm just a contributor here but I think this repo falls under the same category as most open source projects if you want something - you should build it 😄 I've just needed alot so I've contributed a whole lot 😆
I'm using this at work to publish type packages for my APIs. I also use my query parameter + body parameter types to parse request bodies/query parameters. So it's kinda a 3 in 1 benefit for my team.
|
Thanks for the response! I am also using this project for Auto Documentation + request input/output parsing. I believe that the entire purpose of the package is exactly that. I'm trying to go a bit further to develop an end-to-end type safe system for my project. At this point, I'm just writing my current ideas here to see if it generates a productive conversation. I'm trying to achieve the end-to-end type safety that tRPC (https://trpc.io/) gives, however without changing the express.js REST endpoint interface (My understanding is that tRPC is simply a typed Remote Procedure Call). With I know that there are openapi codegen packages (https://github.com/ferdikoomen/openapi-typescript-codegen), but they require a build step to reflect changes in the client. I am unsure how tRPC achieves this instant type validation across the client/server without a build step. |
@adamgarcia4 thank you for the kind words. And yes you are spot on (as is @samchungy) about the purpose of the project. Basically as the README suggests we had a project where we wrote the documentation manually but also maintained zod schemas. We wanted to automate this process. And similarly to you we wanted an end-to-end type safe solution that also happens to build the documentation. The end result looks a little bit like this: router.post('/addresses', {
handler: createAddress,
route: createAddressRoute,
}); Server side this does generate the whole documentation based on where a route is used. But the route is also used for validation. And on the frontend side we have something like this.request<typeof createAddressRoute>(
{
method: 'POST',
url: '/user/addresses',
data,
}
) So basically we do have a whole framework around this. But since some of our work was very tightly connected to domain decisions etc we decided to go with a more generic openapi generator from openapi that is context independent. For example zod can be used for validation using something different from express. Or the structure you want to use might be different. So we decided to provide a building block instead of a whole architecture approach. We do however have the idea of implementing something express bound at some point but it would be as a separate project that uses |
And as for the question. I actually cannot remember fully what the reason was to keep headers as an array. However I can share some of the considerations I had back then:
const commonHeaderRegistry = new OpenAPIRegistry();
commonHeaderRegistry.registerParameter(
'userAgent',
z.ostring().openapi({
param: {
in: 'header',
name: 'user-agent',
description: 'The user device agent.',
example: 'some-actual-example'
},
})
);
// ... some more headers are registered into commonHeaderRegistry
// Getting all default headers (they would apply for every single endpoint in the system)
export const defaultHeaders = commonHeaderRegistry.definitions.map(def => def.schema);
const specificHeadersRegistry = new OpenAPIRegistry();
export const AuthorizationHeader = specificHeadersRegistry.register(
'authorization',
z.string().openapi({
param: {
in: 'header',
name: 'authorization',
description: 'Identity Authorization Token',
example: 'Bearer some-user-token',
},
})
); And then as a result we can either append the
Hopefully the example above does help in understanding our approach. That being said I am open to the idea of adding the object support mainly because of (3) - if you do validate your headers an objects seems as the better approach. |
I've used Zod Objects before to parse headers. For example with Koa the headers ctx object is typed as I think if you raised a non breaking PR @adamgarcia4 it seems like it will get through 🙂. I had a quick lookise and it shouldn't be too difficult 🙂 |
I just opened a PR. Will try and move it forward asap. Sorry that this took so long |
…-for-headers Feature/#80 use zod object for headers
@adamgarcia4 I've just released a |
@adamgarcia4 closing the issue for now. Let us know if there any problems and feel free to ask any further questions. |
First of all, great job with this project. I don't see why it isn't more adopted in the larger community.
I'm struggling to understand why the
request.headers
configuration is defined as aZodtype[]
, while params and query are defined asAnyZodObject
.My understanding behind this decision is because there are often non-user defined headers that are passed through with a request directly from the browser (cache-control, content-security-policy, etc..).
However, playing around with
request.body
revealed that you are actually able to pass in un-specified keys unless you addz.object({...}).strict()
to therequest.body
specification.If that is the case, could
request.headers
also accept anAnyZodObject
?I'm building an Express middleware which accepts a RouteConfig object and:
I'm trying to build a strongly typed end-to-end system, and the usage of zod-to-openapi as an input to the validation middleware allows stronger consistency. I've attached a gist of the middleware here. (It is ofc a work in progress).
https://gist.github.com/adamgarcia4/7b5644bb6bb5d3daabf5f8377c5b66b4
I first wanted to ask the above question, as well as showcase how I'm using your package.
The text was updated successfully, but these errors were encountered: