Add useNullish configuration option for Zod to use z.nullish instead of z.optional - closes #2323#3302
Add useNullish configuration option for Zod to use z.nullish instead of z.optional - closes #2323#3302hbmartin wants to merge 1 commit intohey-api:mainfrom
useNullish configuration option for Zod to use z.nullish instead of z.optional - closes #2323#3302Conversation
…ead of `z.optional` - closes hey-api#2323
|
|
|
@hbmartin is attempting to deploy a commit to the Hey API Team on Vercel. A member of the Team first needs to authorize it. |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46947192fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (optional) { | ||
| ast.expression = $(z).attr(identifiers.optional).call(ast.expression); | ||
| ast.typeName = identifiers.ZodOptional; | ||
| const method = plugin.config.useNullish ? identifiers.nullish : identifiers.optional; | ||
| ast.expression = $(z).attr(method).call(ast.expression); | ||
| ast.typeName = plugin.config.useNullish ? identifiers.ZodNullish : identifiers.ZodOptional; |
There was a problem hiding this comment.
Avoid referencing non-existent
z.ZodNullish type
When useNullish is enabled, this changes the type annotation to z.ZodNullish, but Zod doesn’t expose a ZodNullish type (nullish is a convenience for ZodOptional<ZodNullable<...>>). In v4 outputs that include type annotations, this will cause generated code to fail type checking because z.ZodNullish is undefined. Consider keeping ZodOptional (or omitting the type annotation) when generating .nullish().
Useful? React with 👍 / 👎.
| if (optional) { | ||
| ast.expression = $(z).attr(identifiers.optional).call(ast.expression); | ||
| ast.typeName = identifiers.ZodMiniOptional; | ||
| const method = plugin.config.useNullish ? identifiers.nullish : identifiers.optional; | ||
| ast.expression = $(z).attr(method).call(ast.expression); | ||
| ast.typeName = plugin.config.useNullish | ||
| ? identifiers.ZodMiniNullish | ||
| : identifiers.ZodMiniOptional; |
There was a problem hiding this comment.
Avoid referencing non-existent
z.ZodMiniNullish type
With useNullish enabled for the mini generator, this sets the type annotation to z.ZodMiniNullish, but Zod Mini doesn’t export a ZodMiniNullish type. That means the emitted TypeScript will reference an undefined type whenever optional properties are generated with .nullish(), breaking type checking in the mini output. It’s safer to keep the existing ZodMiniOptional type name (or skip the type annotation).
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3302 +/- ##
=======================================
Coverage 34.91% 34.91%
=======================================
Files 401 401
Lines 22237 22246 +9
Branches 1429 1429
=======================================
+ Hits 7763 7767 +4
- Misses 14470 14475 +5
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@hey-api/codegen-core
@hey-api/nuxt
@hey-api/openapi-ts
@hey-api/shared
@hey-api/types
@hey-api/vite-plugin
commit: |
|
@hbmartin Hey, appreciate the pull request, but if there's an issue with the spec, that should be fixed at the spec level. As such, this pull request isn't going to get merged. The linked issues are not related to the problem you're describing |
By default, non-required object properties generate
.optional(), which acceptsundefinedvalues. Some API's incorrectly use nullable in their specs, where required properties can also be setnull. By using this option, the generator produces.nullish()instead of.optional().See issues:
#2323
#3243