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

Allow overriding default .meta() and .getMeta() data types. #3

Closed
wants to merge 3 commits into from

Conversation

kozubsky
Copy link

Hello, Emil! Please consider approach that allows to define default metadata interface and also allows to override .meta() argument type and .getMeta() return type by using generics!

@IvanovES
Copy link
Owner

Hello, Emil! Please consider approach that allows to define default metadata interface and also allows to override .meta() argument type and .getMeta() return type by using generics!

Updated #2, please, check again.

Can you give me an example use case for overwriting IMeta? I want to try an keep it as open as possible. Typing specific keys is probably fine as long as they are not required and are possibly undefined in the return type.

@kozubsky
Copy link
Author

kozubsky commented Apr 25, 2023

Hello, Emil! Please consider approach that allows to define default metadata interface and also allows to override .meta() argument type and .getMeta() return type by using generics!

Updated #2, please, check again.

Can you give me an example use case for overwriting IMeta? I want to try an keep it as open as possible. Typing specific keys is probably fine as long as they are not required and are possibly undefined in the return type.

Ok, use case.

For example, I use zod for validation in controllers and also want to have some openApi related data stored inside every instance of ZodType:

const Input = z.object({
	page: z.number().meta({ example: 5 }),
	perPage: z.number().meta({ example: 50 }),
});

And I want to have autocompletion when I call .getMeta().<VALID_FIELDS_AUTOCOMPLETION>, or when I call .meta({<VALID_FIELDS_AUTOCOMPLETION>}).

In most of cases within a single project all metadata will have that type, so it is possible to be defined as a default data type.

But in some rare cases I will store in meta some data having another data structure, like { firstName?: string, lastName?: string }. In these cases I can do the following steps:

  1. Define interface ICustomMeta { firstName?: string, lastName?: string }

  2. Call z.object().meta<ICustomMeta>({ firstName: John, lastName: Doe })

  3. Call z.object().getMeta<ICustomMeta>()?.lastName

P.S. Of course, as an alternative, it is possible to define a pair of helper functions for each use case, like function getOpenapiMeta(value: ZodType): IOpenApiMeta and function setOpenapiMeta<T>(obj: T, value: IOpenApiMeta): T.

@IvanovES
Copy link
Owner

IvanovES commented Apr 25, 2023

I see. You may want to follow these issues then:
colinhacks/zod#2042
asteasolutions/zod-to-openapi#99

One problem with the example meta example is that the example value must be of the type of the schema output, e.g. string for z.string() and number for z.number. So, you'll end up typing it as example: string | number | any-other-type-you-need.
zod-to-openapi solves this and adds other utility, so you may want to check it out if you haven't.

To be honest, the only property I needed (that I didn't already have from zod-to-openapi) is the OpenAPI refId, so my only use case is probably something like

z.object({ firstName: ..., lastName: ... })
  .meta({ [Symbol.for('openpi:refId')]:  'User' })

Let me know if #2 is doing it for you.

@samchungy
Copy link

@IvanovES if you want to be my guinea pig I rewrote zod-to-openapi to support auto registering schemas: https://github.com/samchungy/zod-openapi. Very similar API but I went with a different approach

@IvanovES
Copy link
Owner

IvanovES commented Apr 29, 2023

@IvanovES if you want to be my guinea pig I rewrote zod-to-openapi to support auto registering schemas: https://github.com/samchungy/zod-openapi. Very similar API but I went with a different approach

@samchungy This is exactly what I hoped for when I first discovered zod-to-openapi!

Though, please remove createDocumentJson and createDocumentYaml. Without these your library does one thing and it does it well. Don't couple it with serialization. I don't need yaml, you don't need yaml, let's not have yaml in our dependencies. And whether or not it's json or yaml you don't want to pick the serialization library - yaml is not the only one nor is JSON.stringify - https://www.npmjs.com/package/fast-json-stable-stringify.

PS. I may have spoken too soon. You may still be able to have your yaml cake you can declare it as an optional peer dependency.

Anyway, good job!

@samchungy
Copy link

@IvanovES if you want to be my guinea pig I rewrote zod-to-openapi to support auto registering schemas: https://github.com/samchungy/zod-openapi. Very similar API but I went with a different approach

@samchungy This is exactly what I hoped for when I first discovered zod-to-openapi!

Though, please remove createDocumentJson and createDocumentYaml. Without these your library does one thing and it does it well. Don't couple it with serialization. I don't need yaml, you don't need yaml, let's not have yaml in our dependencies. And whether or not it's json or yaml you don't want to pick the serialization library - yaml is not the only one nor is JSON.stringify - https://www.npmjs.com/package/fast-json-stable-stringify.

PS. I may have spoken too soon. You may still be able to have your yaml cake you can declare it as an optional peer dependency.

Anyway, good job!

Good feedback. I'll have a look at removing it

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.

3 participants