Skip to content

fix: zod optional default order#3759

Merged
mrlubos merged 1 commit intomainfrom
fix/zod-default-order
Apr 14, 2026
Merged

fix: zod optional default order#3759
mrlubos merged 1 commit intomainfrom
fix/zod-default-order

Conversation

@mrlubos
Copy link
Copy Markdown
Member

@mrlubos mrlubos commented Apr 14, 2026

Closes #3751

@mrlubos mrlubos marked this pull request as ready for review April 14, 2026 00:07
@bolt-new-by-stackblitz
Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 14, 2026

🦋 Changeset detected

Latest commit: cda5297

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hey-api/openapi-ts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hey-api-docs Ready Ready Preview, Comment Apr 14, 2026 0:13am

Request Review

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug 🔥 Broken or incorrect behavior. labels Apr 14, 2026
@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog bot commented Apr 14, 2026

TL;DR — Fixes Zod schema generation so .default() is emitted before .optional(), preventing z.infer from incorrectly marking fields with defaults as required. Also refactors the zod test infrastructure to use shared constants and globalTeardown for temp directory cleanup.

Key changes

  • Move .default() before .optional() in Zod walkers — Reorders the modifier chain across mini, v3, and v4 walkers so default values are applied to the inner schema, and .optional() wraps the result. This ensures z.infer correctly produces optional (not required) types for fields with defaults.
  • Extract getDefaultValue helper in Zod walkers — Deduplicates the inline meta.format ? maybeBigInt(...) : $.fromValue(...) logic into a typed getDefaultValue(meta: ZodMeta) function in each walker.
  • Drop .default() for non-optional, non-nullable required fields — Required fields (no optional, no nullable) with a default in the spec no longer emit .default() at all — the default is a spec-level concern, not a runtime validation one.
  • Rename hasDefaultneedsDefault — Aligns naming with the existing needsOptional / needsNullable convention across all walkers (including valibot and pydantic).
  • Refactor zod test utilities — Extracts snapshotsDir and tmpDir into a shared constants.ts, adds globalTeardown.ts to clean up .tmp after test runs, and simplifies .gitignore files.

Summary | 98 files | 1 commit | base: mainfix/zod-default-order


.default() before .optional() in Zod output

Before: z.string().optional().default('Hello World!')z.infer produces a required field because .default() wraps the optional schema.
After: z.string().default('Hello World!').optional()z.infer correctly produces an optional field with a default value.

The core issue is that Zod's z.infer resolves the outermost wrapper to determine whether a field is required. When .default() was the outermost call, Zod inferred the field as required (since a default guarantees a value). By placing .optional() on the outside, the inferred type correctly reflects that the field can be omitted from input — while still falling back to the default at runtime.

The fix applies uniformly to three branches in each walker: optional && nullable (nullish), optional-only, and nullable-only. In the optional-only branch, .default() is now applied before .optional() rather than unconditionally after the entire if/else chain. A hasDefault flag tracks whether .default() was already emitted so the fallback block (needsDefault && !hasDefault) only fires for non-optional, non-nullable cases.

Why are required fields with defaults no longer wrapped in .default()?

For required, non-nullable fields the .default() call was redundant — the field must be provided by the caller regardless. The default value is a schema-level documentation concern (e.g. for codegen clients), not a Zod runtime validation concern. Removing it keeps the generated schemas minimal and avoids confusing z.infer output.

packages/openapi-ts/src/plugins/zod/mini/walker.ts · packages/openapi-ts/src/plugins/zod/v3/walker.ts · packages/openapi-ts/src/plugins/zod/v4/walker.ts


Valibot and Pydantic hasDefaultneedsDefault rename

Before: Variable named hasDefault.
After: Variable named needsDefault, consistent with needsOptional and needsNullable.

A pure rename with no behavioral change. The valibot walker already had the correct .default() placement (inside valibot's optional()/nullish() wrappers), so only the variable name changed.

packages/openapi-ts/src/plugins/valibot/v1/walker.ts


Zod test infrastructure cleanup

Before: Each test file computed snapshotsDir and outputDir via helper functions; temp output went to .gen/snapshots; no automatic cleanup.
After: Shared constants.ts exports snapshotsDir and tmpDir; globalTeardown.ts removes .tmp after each test run; .gitignore simplified.

Both zod/v3 and zod/v4 test suites get the same treatment — extracting path constants, wiring up globalSetup in vitest.config.ts, and building snapshot paths inline in the test files instead of pre-computing them.

packages/openapi-ts-tests/zod/v3/test/constants.ts · packages/openapi-ts-tests/zod/v3/test/globalTeardown.ts · vitest.config.ts

Pullfrog  | View workflow run | Triggered by Pullfrog𝕏

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 0% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.08%. Comparing base (fe08b45) to head (cda5297).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
packages/openapi-ts/src/plugins/zod/mini/walker.ts 0.00% 6 Missing and 3 partials ⚠️
packages/openapi-ts/src/plugins/zod/v3/walker.ts 0.00% 6 Missing and 3 partials ⚠️
packages/openapi-ts/src/plugins/zod/v4/walker.ts 0.00% 6 Missing and 3 partials ⚠️
...ckages/openapi-ts/src/plugins/valibot/v1/walker.ts 0.00% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3759      +/-   ##
==========================================
- Coverage   40.11%   40.08%   -0.03%     
==========================================
  Files         521      521              
  Lines       19261    19273      +12     
  Branches     5733     5727       -6     
==========================================
  Hits         7726     7726              
- Misses       9339     9345       +6     
- Partials     2196     2202       +6     
Flag Coverage Δ
unittests 40.08% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 14, 2026

Open in StackBlitz

@hey-api/codegen-core

npm i https://pkg.pr.new/@hey-api/codegen-core@3759

@hey-api/json-schema-ref-parser

npm i https://pkg.pr.new/@hey-api/json-schema-ref-parser@3759

@hey-api/nuxt

npm i https://pkg.pr.new/@hey-api/nuxt@3759

@hey-api/openapi-ts

npm i https://pkg.pr.new/@hey-api/openapi-ts@3759

@hey-api/shared

npm i https://pkg.pr.new/@hey-api/shared@3759

@hey-api/spec-types

npm i https://pkg.pr.new/@hey-api/spec-types@3759

@hey-api/types

npm i https://pkg.pr.new/@hey-api/types@3759

@hey-api/vite-plugin

npm i https://pkg.pr.new/@hey-api/vite-plugin@3759

commit: cda5297

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog bot left a comment

Choose a reason for hiding this comment

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

Reviewed — no issues found.

The core fix is correct: for optional fields with defaults, .default(value).optional() produces the right z.infer type (T | undefined), whereas the old .optional().default(value) collapsed to just T (required). The nullish branch correctly keeps .default() after .nullish() since that wraps both null and undefined in one call.

Two secondary changes worth noting (both seem intentional):

  • .default() dropped for non-optional, non-nullable fields — required fields no longer get a Zod .default(), so validation will reject undefined. This is more correct for client schemas where the server applies the default, but it's a behavioral change beyond what the changeset title describes.
  • Test infra refactoring — output dir changed from .gen/snapshots to .tmp, with globalTeardown.ts for cleanup and shared constants.ts. The root .gitignore already covers .tmp, so the local .gitignore cleanup is safe.
Task list (4/4 completed)
  • Read the diff: changeset, core walker changes, and test infrastructure
  • Read snapshot changes to understand the behavioral impact
  • Trace data flow in walker changes for correctness
  • Self-critique and submit review

Pullfrog  | View workflow run𝕏

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog bot left a comment

Choose a reason for hiding this comment

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

Reviewed — no issues found.

The core fix is correct: for optional fields with defaults, .default(value).optional() produces the right z.infer type (T | undefined), whereas the old .optional().default(value) collapsed to just T (required). The nullish branch correctly keeps .default() after .nullish() since that wraps both null and undefined in one call.

Two secondary changes worth noting (both seem intentional):

  • .default() dropped for non-optional, non-nullable fields — required fields no longer get a Zod .default(), so validation will reject undefined. This is more correct for client schemas where the server applies the default, but it's a behavioral change beyond what the changeset title describes.
  • Test infra refactoring — output dir changed from .gen/snapshots to .tmp, with globalTeardown.ts for cleanup and shared constants.ts. The root .gitignore already covers .tmp, so the local .gitignore cleanup is safe.
Task list (4/4 completed)
  • Read the diff: changeset, core walker changes, and test infrastructure
  • Read snapshot changes to understand the behavioral impact
  • Trace data flow in walker changes for correctness
  • Self-critique and submit review

Pullfrog  | View workflow run𝕏

@mrlubos
Copy link
Copy Markdown
Member Author

mrlubos commented Apr 14, 2026

@pullfrog happy with the latest changes?

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog bot left a comment

Choose a reason for hiding this comment

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

Reviewed — no issues found.

Task list (5/5 completed)
  • Checkout PR and read the diff
  • Review walker changes (zod mini, v3, v4, valibot)
  • Review test infrastructure changes
  • Review snapshot changes
  • Submit review

Pullfrog  | View workflow run𝕏

@mrlubos mrlubos merged commit 7374708 into main Apr 14, 2026
10 of 12 checks passed
@mrlubos mrlubos deleted the fix/zod-default-order branch April 14, 2026 00:21
@hey-api hey-api bot mentioned this pull request Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🔥 Broken or incorrect behavior. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Zod] Default and Optional Interaction

1 participant