API improvement suggestion

[EmilIvanov]

Hi, thank you for the great library. I'm playing around with it and I think it would be useful to be able to specify the schema name in the meta, e.g.:

const UserSchema = z.object({
  id: z.string().openapi({ example: '1212121', schemaName: 'ID' }),
  name: z.string().openapi({ example: 'John Doe', schemaName: 'Name' }),
  age: z.number().openapi({ example: 42, schemaName: 'Age' }),
}).openapi({ schemaName: 'User' })

This way schema registration can be done separately.

[AGalabov]

Hi @EmilIvanov thank you for the kind words. I am not quite sure what you'd want to achieve by doing that 🤔? Is this meant to be an alternative to the current registry.register('name', schema) - where name would be the reference name under component/schemas?

If that is the case then how would you go about passing all schemas into the OpenAPIGenerator? And if it not that, could you please elaborate some more on your proposal and the added benefits?

[EmilIvanov]

Thank you for the quick response. Here's the tl;dr;

The OpenApi definitions only change when your API changes. This means they can be generated just once at build time and then served statically. Imagine that you have routes.ts that exports your route definitions, server.ts that will serve them and a generate-openapi.ts script that will import the routes and produce a swagger.json. I've implemented (a way worse version of) this using joi and joi-to-swagger which uses the schema's meta. The idea is that you just pass the route definitions to the generator function. It can register any schema on the fly the first time it encounters it and then just ref if. You don't really need to register any schema that's not on a path as it's not really a part of your API (though I'm not arguing that it's useful to be able to produce definitions for schemas that are not on your paths, it's just that they should not be mandatory at path definition generation).

And so, if I can put the refId in the meta I can use that to register it in that generate-openapi.ts script and omit this work in my application on startup. I'm fine with still having to register all the schemas - I can just import them and loop over them before I do the generation. Ideally, the library API (and that's just an opinion, really) would look like

const { paths, components } = generatePaths({ paths, components });

As an added bonus, you can have a schema just lying in a file. If you need to register it at definition time, then you need to have a registry instance available and so now you need to export a function of the registry to a schema.

Hopefully at least some of this makes sense.

[samchungy]

I think I was trying to achieve something like that earlier on but I never could envision it.

I think I get it though.

The idea it seems would be declaring your schema eg.

const name = z.string().openapi({schemaRef: "Name"});

and then when we use it:

const someObj = z.object({firstName: name});

When we run into something using schemaRef we immediately "register" it and chuck it into the components.

We could generate as

someObj:
  properties:
    name:
     $ref: "#/components/schemas/Name" 

Without needing to register it. It might be possible. I can probably have a go at this over the weekend to see if it's possible or how much work it would take, seems like a fun challenge.

That way we could bypass needing to create a registry at runtime which I think is what he's trying to say. This is possibly a little bit cleaner than registry.register()

[AGalabov]

I am going to mention @samchungy comment from the PR here so that we can all discuss:

I'm thinking that, while we traverse the Zod object - whenever we run into refId or schemaRef or field name TBD, we add it to this.schemaRefs

We are already doing this in some sense 🤔. The only difference is that we are only "traversing" schemas that we've passed to the generator. And from what I understood the idea of @EmilIvanov was to just use what was already passed through the paths, right 🤔?

If that is the case I think the overall approach should be something along the lines of:

1. If a route uses a schema that has a refId

• generate it
• add it to the references array
• reference it in the generated path documentation

2. If a route uses a schema that does not have a refId

• generate a schema
• inline it in the generated path documentation

Is that the target state? If so I don't think that would be too hard to implement. However that would break some previous encapsulation ideas. Right now everything that is internal to the work of zod-to-openapi has been managed within .openapi._internal. The proper way to alter it is by using .internal_openapi (which is deprecated - i.e not supposed to be used by "clients").

I would like to share how we've setup stuff for our project and how it works:

1. We create a global registry where we can register all our schemas and components. We decide which ones we want to register and which not.
2. At build time we generate the documentation from all the accumulated definitions within that registry
3. At runtime the registry has no effect since the zod class instances have already been built

From what I've gathered what you're trying to solve/implement is a way to just pass in paths and not the schemas. And reference them based on the presence of refId. I do not see any development benefit from that approach 🤔 You would still need to "manually" decide which schemas must have a refId and which not.

[AGalabov]

cc: @georgyangelov I imagine you'd also have thoughts on that.

[EmilIvanov]

Let me try to explain everything. There are multiple things going on here and this is turning out to be something more than the original issue.

First and foremost - metadata

Intro. zod doesn't support attaching arbitrary metadata, i.e. schema.meta({ anyKey: anyValue }) (here's a link). Of course your goal isn't to add support for metadata and so you added what you needed via the the schema.openapi method. Ideally, this would be implemented in zod. Then you could:

• list they keys that your library is using, because you want to be able to share keys with other implementors or
• export symbols to be used as keys, so that it's not possible to have overlapping keys with other implementors.

Since this is not the case, I checked if you had support for a key that would hold the schema name. I found this example in your documentation:

z.string().nullable().openapi(refId: 'name');

Unfortunately, refId isn't actually allowed (never-mind the missing curlies).

This is the original request. And so, I thought I'd try asking you to allow for an additional key in your metadata object where I could put the schema name.

Why does this matter?

1. If I can place the name of the schema on the schema itself, then I can avoid the extra work of registering the schemas at application startup.
2. If zod adds support for metadata, then this library can move to devDependencies.

Second - ease of use

Now, even though this wasn't in the original request, you kinda caught on to it probably since it doesn't make sense to add that new key and not use it.

Here is be my suggestion without providing specifics.

In addition to:

const schema = ...;

const registry = new Registry();

const registeredSchema = registry.register(schema);

const route = {
  request: {
    params: registeredSchema
  },
  ...
};

registry.registerPath(route);

const generator = new OpenAPIGenerator(registry.definitions, version);
const document = generator.generateDocument(openApiConfigObject);

Provide an API similar to:

const schema = ...;

const route = {
  request: {
    params: registeredSchema
  },
  ...
};
// -------------------- THIS LINE IS THE IMPORTANT PART -----------------
const document = generateDocument({
  schemas: [schema],
  paths: [route],
  webhooks: ...,
  ...rest of the definition types
}, version, openApiConfigObject)

Now with the addition of the new meta key, you wouldn't need to pass schemas, but that's the only difference.

[samchungy]

At runtime the registry has no effect since the zod class instances have already been built

By runtime I mean - when we call registry.register(schema) that's a call at runtime.

For my projects in particular the generated schema isn't required at runtime. Technically, runtime includes when everything is being what I believe you are calling build time.

So we don't need a registry or the generator for that matter, just the Zod types really and adding eg. The ability to set refId as well as the dynamic referencing would allow me to avoid needing to create a registry at runtime. Personally I am fine with creating a generator class - not a biggie for me but if we could make the registry optional that would be awesome.

We decide which ones we want to register and which not.

This should also be possible by being able to specify refId I believe.

[EmilIvanov]

Psst, hey, want some zod-metadata? Probably, not production ready, though.

[georgyangelov]

Hi all, first thank you all for the great discussion.

This is an interesting suggestion for sure. I'm wondering whether the important part here is "not having to call registry.register(...)" or "let's figure out how to make zod-to-openapi a dev-only dependency". In my mind it's the second thing that's really the point. Let me explain why I think that:

1. Having to call registry.register(...) is maybe a nuisance in some cases but it's not something that takes away performance. The register method is just "add a refId" and "push to an internal array". That internal array contains references to objects that would exist anyway.
2. Even if we provide some mechanism to auto-register schemas, I'm opposed to completely removing the registry. It's useful in more cases, for example when you want to generate partial OpenAPI documents, e.g. if you already have an OpenAPI definition some other way and you want to gradually migrate to zod-to-openapi. Then you don't have the "root" schemas (of the paths) to walk the graph from.
3. As I see it, the "problem" currently is that even though the registry.register(...) calls are not a performance issue at runtime, you have to include the code of the entire zod-to-openapi library in the production code build. This is not a problem for server apps which are the main use-case. However, if you (like us) have a JS library that clients can include as well, then that becomes more size to the bundle which you don't use.

Maybe we should be thinking in that direction - how to make it possible to have zod-to-openapi as a dev dependency. I'm not sure I like completely removing the registry - it gives us a lot of flexibility. Maybe something like a separate package called zod-to-openapi-definitions which adds .openapi and the registry, then this one could be the generation parts which can be a dev dependency. How do you feel about that?

As far as the refId itself, I'm a bit on the fence of having just metadata keys control what gets extracted and what doesn't - we'll have to think about stuff like two different schemas with the same name, how objects are wrapped (nullable, extends, etc.).

[samchungy]

I think being able to specify the refId decouples the registry from the generator a little. At the moment they are so tightly coupled.

I'm fine with keeping the registry around but would love the be able to specify refId as opposed to .register purely out of aesthetics.

Performance aside, I chose this repo over https://github.com/anatine/zod-plugins/blob/main/packages/zod-openapi/README.md mainly because of the elegance of .openapi() over their extendApi(z.object()) wrapper. However needing to import the registry and registry.register(z.object) just feels like the above clunky extendApi.

[EmilIvanov]

In case anyone is reading this and waiting for the update, here's how you can achieve this on your own in the meanwhile.

1. Extend zod-to-openapi

declare module '@asteasolutions/zod-to-openapi' {
  interface ZodOpenAPIMetadata {
    schemaName: string;
  }
}

2. Create your OpenAPI script:

import { Schema } from 'zod';
import { OpenAPIGenerator, OpenAPIRegistry, RouteConfig } from '@asteasolutions/zod-to-openapi';
import { OpenApiVersion } from '@asteasolutions/zod-to-openapi/dist/openapi-generator';

const schemaName = (schema: Schema) =>
  schema._def.openapi?.metadata?.schemaName;
  
const registerSchema = (registry: OpenAPIRegistry, schema: Schema) => {
  const name = schemaName(schema);

  if (!name) {
    return;
  }

  const registered = !!registry.definitions.find(
    (d) => d.type === 'schema' && name === schemaName(d.schema)
  );

  if (registered) {
    return;
  }

  registry.register(name, schema);
};

const registerRoutes = (registry: OpenAPIRegistry, routes: RouteConfig[]) => {
  for (const route of routes) {
    if (route.request?.query) {
      registerSchema(registry, route.request.query);
    }

    // register headers, params, body and responses

    registry.registerPath(route)
  }
};

export const generateDocument = (version: OpenApiVersion, routes: RouteConfig[]) => {
  const registry = new OpenAPIRegistry();
  registerRoutes(registry, routes);
  const generator = new OpenAPIGenerator(registry.definitions, version);
  return generator.generateDocument({ /** your document info */ })
};

[AGalabov]

@samchungy just to mention - the decoupling is still possible. You can just use .internal_openapi. If it would help we can remove the deprecation. That would allow you to add the refId. I would still be against adding within in the normal .openapi since we quite often use a spread operator of whatever metadata is passed. And having the refId there seems like something we can easily miss. And it also feels as an "internal" thing and not regular openapi.

[fomin-alexander]

Hi everyone,

Thank you so much for the awesome library! It's exactly what I was looking for.

I am currently working on incorporating zod-to-openapi into my TypeScript monorepo. I love the idea of using RouteConfig instances as API definitions in a shared package and utilizing them in both the frontend and backend for consistency and validation. In my use case, it would be really helpful to have the flexibility to define and export route configs as static objects without the need to register() related schemas before connecting them.

My current workaround is using internal_openapi, but it's deprecated and not supposed to be used from outside:

export const MySchema = z.object({
  success: z.literal(true),
}).internal_openapi({ refId: 'MySchema' });

Just trying to make a point for decoupling definitions from registry.

[AGalabov]

@EmilIvanov I got some extra time and tried to play a little bit with this idea. This is a test that I got working and I think this is what your idea was, right? I.e register any schema upon its occurrence and reference it.

This is definitely not in a completed state, but at least it showcases the API design that we can try and work towards.

[AGalabov]

@EmilIvanov @samchungy @fomin-alexander I just opened a PR where I've implemented pretty much all of the work for the suggested new API. I'd be more than happy to get some feedback from you guys. The most relevant change (API wise) that you should be looking at seems to be the auto-register.spec.ts file where I've added various test cases for how I imagined the API should work.

[fomin-alexander]

Thank you, @AGalabov, for your work on this! I noticed that your PR introduces the concept of automatic registration of request and response body schemas based on registered paths. However, as a library user, I would suggest leaving it up to the user to decide which schemas to register, for example, by only auto-registering schemas with refId specified. From my experience, not all schemas are useful and re-usable, and auto-registered schemas can sometimes clutter the specification and Swagger UI (I've had this issue with specs generated by tsoa before). So I think it would be better to provide flexibility similar to OpenAPI itself, allowing users to register or not register req/resp schemas as needed.

[samchungy]

Thank you, @AGalabov, for your work on this! I noticed that your PR introduces the concept of automatic registration of request and response body schemas based on registered paths. However, as a library user, I would suggest leaving it up to the user to decide which schemas to register, for example, by only auto-registering schemas with refId specified. From my experience, not all schemas are useful and re-usable, and auto-registered schemas can sometimes clutter the specification and Swagger UI (I've had this issue with specs generated by tsoa before). So I think it would be better to provide flexibility similar to OpenAPI itself, allowing users to register or not register req/resp schemas as needed.

Just curious about this, is there a scenario where you would add a refId but not want it to show in the doc as a ref?

Oh my, I just had a lightbulb moment 💡. I think I understand what you want and how to achieve it. Will come back in like a week 😄

[AGalabov]

@fomin-alexander thank you for the kind words. I think you've missed some parts. I totally agree with your statement and actually the library works exactly as you said in the PR 😄 . If you take a look at the can automatically register response body data test the Person schema has .openapi('Person') and this is passed to the response body. This result in the schema being registered and the route looking like:

{
    "application/json": {
        "schema": {
            "$ref": "#/components/schemas/Person"
        }
    }
}

If we remove the .openapi('Person') and basically pass a plain schema, then nothing would be registered and the "schema" would be just inlined into the response like that:

{
    application/json": {"schema": {"type": "object",
    "properties": {
        "name": {
            "type": "string"
        }
    },
    "required": [
        "name"
    ]
}

Was that what you were suggesting or is there more to it?

[fomin-alexander]

@AGalabov, ah now I see that your changes are exactly what I was hoping for, sorry for being slow to understand :)

@samchungy, I can't imagine a situation where we have a refId but wouldn't want to register it as a schema.

Thanks guys for the feedback, and I'm looking forward to the new release!

[jedwards1211]

@AGalabov I think as long as we have registry.register API the ref ids should live in the registry, not on metadata of the schemas themselves.

For example if you have const Post = z.object({ author: z.lazy(() => User) }) and you later do registry.register(User, 'User'), the User inside z.lazy() isn't going to have the ref id tacked onto its metadata. This is the obstacle we have right now with massaging recursive schemas into a format that zod-to-openapi can generate.
So I think the registry is a better place for that information to live than metadata on the schemas.

Either that or registry.register should be eliminted and everywhere would have to use .openapi(refId). But personally we prefer registry.register so that we can decouple declaring schemas from declaring which ones are refs.

[AGalabov]

@jedwards1211 although I do get your general idea that ideally you would fully separate the two I don't think I get how that would work. (I saw your comment here as well). Overall the idea was that wherever you use that schema you'd want it to have its reference to be reused => I don't think we would be changing that, sorry.

[EmilIvanov]

@AGalabov when are you planning on releasing this feature, or was it already released and we can close this issue?

[AGalabov]

@EmilIvanov I just merged the other PR I was waiting for and was writing release and migration notes. I should release this as 5.0.0 in the next 1-2 hours max 🤞 I'll close the issue with a comment then

[AGalabov]

Present in v5.0.0 🎉 . I'd be happy to hear your thoughts on how it works. Thank you for the idea and the patience on this.