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.

Sign up for GitHub

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

fix: mark the ZodDate as string type in the OpenAPI schema #126

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: mark the ZodDate as string type in the OpenAPI schema #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/nestjs-zod/src/openapi/zod-to-openapi.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,16 @@ describe.each([

expect(openApiObject).toMatchSnapshot()
})


it('should serialize dates', () => {
const schema = z.date()
const openApiObject = zodToOpenAPI(schema)

expect(openApiObject).toEqual({
type: 'string',
})
});

it('should serialize objects', () => {
const schema = z.object({
prop1: z.string(),
Expand Down
4 changes: 4 additions & 0 deletions packages/nestjs-zod/src/openapi/zod-to-openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export function zodToOpenAPI(
}
}

if (is(zodType, z.ZodDate)) {
object.type = 'string'
Copy link
Preview

Copilot AI Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation should set the 'format' property to 'date-time' for the date type in the OpenAPI schema. Update the code to include 'object.format = "date-time"'.

Suggested change
object.type = 'string'
object.type = 'string'; object.format = 'date-time';

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenLorantfy I got this recommendation locally as well, but I wasn't sure that is necessary. Let me know.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gamote I don't think it's a bad idea, but it should probably be date, not date-time:

image image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenLorantfy In that case it might be a bit more common that people are sharing ISO strings rather than just the date. What is your take on it?

Copy link
Owner

@BenLorantfy BenLorantfy Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to support both format: "date" and format: "date-time". But we need a way to tell them apart (e.g. is this zod schema a z.string().date() schema or a z.string().datetime() schema). I can look into that later tonight or tmr if we're unsure how to do so.

Copy link
Author

@Gamote Gamote Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it might be the best approach, I can try to look into it tomorrow too, but I do agree that supporting both might be the way to go.

Copy link

@deweve deweve Dec 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, if it helps, I see you can check the format of a string with isDate or isDateTime.

You can do this when you know that the schema is a string

Edit: Sorry, the library already handles the string type with date and dateTime.

If it can helps my use case of z.date is to add a transform the date object to a specific format.

This is used in a serializer.

function serializeDate(date: Date): string {
    dayjs(date).format("YYYY-MM-DD");
}

z.date().transform(serializeDate)

In this case, you cannot guess the format

You will have a ZodEffect, the zodEffect schema contains a z.Date and not a z.String.

To conclude : The recommendation is good, z.Date will be transformed in string and it is always a datetime.

}

if (is(zodType, z.ZodPassword)) {
const { checks } = zodType._def
const regex = zodType.buildFullRegExp()
Expand Down
Loading