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

feat(zod): add option to setup strict mode #1303

Merged
merged 9 commits into from
Apr 11, 2024
Merged

Conversation

anymaniax
Copy link
Collaborator

Status

READY

Description

Zod add strict option to each object

@anymaniax
Copy link
Collaborator Author

I don't understand why formatting doesn't work. Locally all seems ok

@anymaniax
Copy link
Collaborator Author

I needed to update deps 😅

@soartec-lab
Copy link
Member

soartec-lab commented Apr 11, 2024

Regarding the formatting failure, a formatting error occurred in my environment as well. Perhaps the version of prettier in your environment was outdated?
And now, this time the zod test is failing. This is an intended test failure, so it seems that the test case needs to be updated.

@anymaniax
Copy link
Collaborator Author

anymaniax commented Apr 11, 2024

should be good this time 😅.

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

@anymaniax

it's nice update 👍
CI seems to have passed all checks. And, please check as I have given some opinions.

packages/core/src/types.ts Show resolved Hide resolved
@@ -34,7 +34,11 @@ const queryParams: ZodValidationSchemaDefinitionInput = {
describe('parseZodValidationSchemaDefinition', () => {
describe('with `override.coerceTypes = false` (default)', () => {
it('does not emit coerced zod property schemas', () => {
const parseResult = parseZodValidationSchemaDefinition(queryParams);
const parseResult = parseZodValidationSchemaDefinition(
Copy link
Member

Choose a reason for hiding this comment

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

I am also updating the unit tests, but at the same time, I thought it would be a good idea to add a test that the options added this time work using the automatically generated source code of zod in the tests directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure to understand. I added a test for the new option too. And you want to test the generated files also right? If so indeed would be great

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is a good idea to continuously test that the automatically generated source code does not break when the options added this time are specified. And it's now in the tests directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a test with the option

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

LGTM!
If all CI is successful I'll merge this.

@soartec-lab soartec-lab merged commit a1fa843 into master Apr 11, 2024
2 checks passed
@melloware melloware modified the milestones: 6.28.0, 6.27.0 Apr 11, 2024
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