Conversation
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 86c0795 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
TL;DR — When an OpenAPI schema uses Key changes
Summary | 55 files | 7 commits | base: Discriminator propagation through the IR
A new optional
Zod
|
There was a problem hiding this comment.
This PR contains no code changes — the single "Initial plan" commit is empty (0 files, 0 additions, 0 deletions). There is nothing to review yet.
Once implementation begins, key areas to watch:
- How
discriminator.mappingis resolved during IR construction (especially for$refvalues vs. inline schemas) - Whether the changes correctly handle
oneOf+ discriminator vs.allOf+ discriminator patterns, which have different semantics - Impact on existing snapshot tests
- Zod plugin changes to emit
z.discriminatedUnion()instead of.and()chains
|
@copilot is The Lastly, add the same support to Valibot using |
Yes, For the IR: I've reverted the IR Valibot |
|
@copilot add discriminator to IR again. Clean up Zod, Valibot, and TypeScript plugins to not detect their own discriminators and use the IR instead. |
Done in commit
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3780 +/- ##
==========================================
- Coverage 40.07% 39.75% -0.32%
==========================================
Files 522 530 +8
Lines 19311 19462 +151
Branches 5766 5789 +23
==========================================
Hits 7738 7738
- Misses 9371 9491 +120
- Partials 2202 2233 +31
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:
|
|
Reviewed PR #3780. Submitted a review with 6 inline comments. Key finding: the PR description incorrectly claims Zod mini does not support Task list (9/9 completed)
|
There was a problem hiding this comment.
Important
The PR description claims "Zod mini continues to emit z.union() as .extend() is not available in that package," but the mini snapshots and code both show z.discriminatedUnion() with .extend() being generated. Both APIs are available in zod/v4-mini, so the generated code is correct at runtime — but the description is misleading and should be updated. The refactoring from inlined walker logic into shared intersectionToAst/unionToAst files is clean. One correctness issue below with the tryBuildDiscriminatedUnion null-skipping logic.
Task list (9/9 completed)
- Read PR description and diff overview
- Review core logic changes (shared IR types, OpenAPI parsers)
- Review Zod plugin changes (shared discriminated-union, mini/v3/v4 walkers and toAst)
- Review Valibot plugin changes
- Review test specs and snapshots
- Review changesets
- Deep dive: v4 intersection behavior change
- Deep dive: mini discriminatedUnion + extend correctness
- Submit review
ceead13 to
64c1d5e
Compare
64c1d5e to
59f1535
Compare
59f1535 to
86c0795
Compare
@hey-api/codegen-core
@hey-api/json-schema-ref-parser
@hey-api/nuxt
@hey-api/openapi-ts
@hey-api/shared
@hey-api/spec-types
@hey-api/types
@hey-api/vite-plugin
commit: |

When an OpenAPI schema uses
oneOf/anyOfwith adiscriminator+mapping, the Zod plugin was emittingz.union([z.object({...}).and(zRef), ...])and the Valibot plugin was emittingv.union([...])— both losing type-narrowing and runtime validation optimisation.Changes
packages/shared/src/ir/types.ts): Addeddiscriminator?: { propertyName: string }toIRSchemaObjectas the single source of truth for discriminated unions.parseAnyOf/parseOneOf): SetirSchema.discriminatorwhen the OpenAPI schema carries adiscriminatorand the resulting IR schema is a union.discriminatedUnionandextendidentifiers.zod/shared/discriminated-union.ts):tryBuildDiscriminatedUnion()readsparentSchema.discriminator?.propertyNamefrom the IR and emitsz.discriminatedUnion('key', [zRef.extend({ key: z.literal(val) }), ...]). Returnsnull(falls back toz.union()) when no discriminator is present or the pattern cannot be applied (lazy refs, inline schemas, etc.)..extend()is required becausez.discriminatedUnion()requiresZodObjectmembers —.and()produces aZodIntersectionwhich Zod rejects.union()attemptstryBuildDiscriminatedUnion()before falling back toz.union(). Zod mini is unchanged —.extend()is absent in that package.v1/toAst/union.ts: ReadsparentSchema.discriminator?.propertyNamefrom the IR and emitsv.variant('key', [...intersectItems]). Valibot'svariantacceptsintersectschemas directly so no.extend()equivalent is needed.Example
Zod:
Valibot:
Applies to Zod v3, Zod v4, and Valibot v1. Zod mini continues to emit
z.union()as.extend()is not available in that package. The@hey-api/typescriptplugin required no changes — TypeScript handles discriminated unions natively through structural typing.