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

Why are request headers defined as an array instead of a ZodObject? #80

Closed
adamgarcia4 opened this issue Dec 19, 2022 · 10 comments
Closed

Comments

@adamgarcia4
Copy link

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 a Zodtype[], while params and query are defined as AnyZodObject.

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 add z.object({...}).strict() to the request.body specification.

If that is the case, could request.headers also accept an AnyZodObject?

I'm building an Express middleware which accepts a RouteConfig object and:

  1. Registers this as a route in the OpenAPI documentation.
  2. Provides validation on the request object (body/params/query/headers), and rejects the response if an input is malformed.
  3. Once the route returns a response, the response is also validated against the schema.

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.

export interface RouteConfig extends OperationObject {
    method: Method;
    path: string;
    request?: {
        body?: ZodRequestBody;
        params?: AnyZodObject;
        query?: AnyZodObject;
        headers?: ZodType<unknown>[];
    };
    responses: {
        [statusCode: string]: ResponseConfig;
    };
}
@samchungy
Copy link
Contributor

samchungy commented Dec 19, 2022

I think for consistency given how these are declared in openapi it think it would make sense if it was also made an AnyZodObject. Would be a breaking change though.

@adamgarcia4
Copy link
Author

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.

@samchungy
Copy link
Contributor

samchungy commented Dec 19, 2022

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 😆

Side question: Could you provide insight how you guys are using this project beyond the "hello world" use case?

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.

  1. Types for consumers
  2. Auto documentation (Open API doc)
  3. Request Body/Query Param parsers.

@adamgarcia4
Copy link
Author

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 zod-to-openapi, I now have the actual routes being safely and strongly typed, with an exposed OpenAPI specification. This is a great first step towards end-to-end type safety. My project is spread across 2 different repos (Frontend/Backend), so sharing the types is a bit difficult.

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.

@AGalabov
Copy link
Collaborator

@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 zod-to-openapi.

@AGalabov
Copy link
Collaborator

AGalabov commented Dec 20, 2022

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:

  1. Headers are more often than not "static" within a system. That is you would probably have an Authorization header and something specific like x-my-custom-header or something else.
  2. Many endpoints would share those headers. This is opposed to query/path parameters being very specific to the used endpoint
    So that being said an example usage we have in our app is:
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 AuthorizationHeader to the default, or not based on the endpoint.

  1. The last consideration is that at the point in time we do not validate request headers using zod. So this kind of influenced our approach.

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.

@samchungy
Copy link
Contributor

samchungy commented Jan 3, 2023

I've used Zod Objects before to parse headers. For example with Koa the headers ctx object is typed as any so it's nice to not have to do the ol dance of typeof headers.X === 'string' and instead just chuck it into HeaderSchema.parse(headers).

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 🙂

@AGalabov
Copy link
Collaborator

AGalabov commented Jan 9, 2023

I just opened a PR. Will try and move it forward asap. Sorry that this took so long

AGalabov added a commit that referenced this issue Jan 10, 2023
…-for-headers

Feature/#80 use zod object for headers
@AGalabov
Copy link
Collaborator

@adamgarcia4 I've just released a v4.1.0 version with the latest code. Can you check if that works for you so that we can close the issue.

@AGalabov
Copy link
Collaborator

@adamgarcia4 closing the issue for now. Let us know if there any problems and feel free to ask any further questions.

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

No branches or pull requests

3 participants