fix(zod): add type field to generated enums for openapi compatibility#1478
fix(zod): add type field to generated enums for openapi compatibility#1478Sigmabrogz wants to merge 5 commits intomiddleapi:mainfrom
Conversation
dosubot
bot
added
the
size:S
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEmit explicit JSON Schema primitive Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where ZodEnum and ZodNativeEnum schemas, when converted to JSON schema, were missing the crucial 'type' field. This omission led to compatibility problems with various OpenAPI tools, such as hey-api. The changes introduce the 'type' field, setting it to 'string' for standard Zod enums and dynamically inferring 'string' or 'number' for native TypeScript enums, thereby improving the robustness and interoperability of generated schemas. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
dosubot
bot
added
bug
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to add a type field to generated schemas for Zod enums to improve OpenAPI compatibility. While the intent is good and the changes for string-based enums are correct, the implementation introduces bugs for numeric enums in both the Zod v3 and Zod v4 converters. For Zod v3's ZodNativeEnum, the logic incorrectly handles TypeScript's numeric enum representation. For Zod v4's z.enum, the type is incorrectly hardcoded to string, failing to account for numeric enums. I've added specific comments detailing these issues.
packages/zod/src/converter.ts
Outdated
| const schema_ = schema as ZodNativeEnum<EnumLike> | ||
|
|
||
| return [true, { enum: Object.values(schema_._def.values) }] | ||
| return [true, { type: typeof Object.values(schema_._def.values)[0] === 'number' ? 'number' : 'string', enum: Object.values(schema_._def.values) }] |
There was a problem hiding this comment.
This implementation for ZodNativeEnum has two issues:
- Inefficiency: It calls
Object.values(schema_._def.values)twice, which can be optimized by storing the result in a variable. - Bug with numeric enums: For a numeric TypeScript enum like
enum MyEnum { A },Object.values(MyEnum)returns['A', 0]. Your logic will check the type of the first element ('A'), incorrectly infer the type as'string', and include the string key in the finalenumarray for the OpenAPI schema. This is incorrect and will generate invalid schemas for numeric enums.
A more robust implementation is needed to correctly handle native numeric enums by filtering out the reverse-mapped keys that TypeScript generates.
packages/zod/src/zod4/converter.ts
Outdated
| case 'enum': { | ||
| const enum_ = schema as $ZodEnum | ||
| return [true, { enum: Object.values(enum_._zod.def.entries) }] | ||
| return [true, { type: 'string', enum: Object.values(enum_._zod.def.entries) }] |
There was a problem hiding this comment.
Hardcoding the type as 'string' is incorrect for numeric enums, which are supported by z.enum() in Zod v4 (e.g., by passing a numeric native enum). This will generate an invalid OpenAPI schema for such cases.
You should infer the type from the enum values. For example:
const values = Object.values(enum_._zod.def.entries);
const type = values.every(v => typeof v === 'number') ? 'number' : 'string';
return [true, { type, enum: values }];Since Zod v4's z.enum does not support mixed-type enums, checking if all values are numbers is a safe way to determine the type.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/zod/src/converter.ts`:
- Line 352: Add a converter unit test in converter.test.ts that calls
z.nativeEnum(TestEnum) where TestEnum is mixed (e.g., A = 123, B = 'string') to
reproduce the bug, then change the inference in converter.ts so it doesn't use
typeof on the array itself; instead inspect each entry of schema_._def.values
(e.g., values = Object.values(schema_._def.values)) and determine if all entries
are numbers, all are strings, or mixed, then set the returned type to 'number'
if all numbers, 'string' if all strings, or omit/mark as mixed (or use a neutral
representation) and always return the full values array as the enum; update the
logic around schema_._def.values accordingly to ensure mixed enums produce the
correct enum array and no incorrect single-type tag.
In `@packages/zod/src/zod4/converter.ts`:
- Line 433: The return for z.enum handling currently hardcodes type: 'string'
which breaks JSON Schema for numeric or mixed enums; update the logic around the
branch returning [true, { type: 'string', enum:
Object.values(enum_._zod.def.entries) }] to inspect the enum values
(Object.values(enum_._zod.def.entries)), determine their primitive types (all
'string' -> type: 'string', all 'number' -> type: 'number', all 'boolean' ->
type: 'boolean'), and if the enum is mixed types omit the type property entirely
and only emit the enum array; keep the same return shape but conditionally
include the type field so functions handling z.enum in converter.ts produce
valid JSON Schema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92c762e1-845e-4bb7-a349-0957c764426d
📒 Files selected for processing (5)
packages/zod/src/converter.test.tspackages/zod/src/converter.tspackages/zod/src/zod4/converter.native.test.tspackages/zod/src/zod4/converter.structure.test.tspackages/zod/src/zod4/converter.ts
packages/zod/src/zod4/converter.ts
Outdated
| case 'enum': { | ||
| const enum_ = schema as $ZodEnum | ||
| return [true, { enum: Object.values(enum_._zod.def.entries) }] | ||
| return [true, { type: 'string', enum: Object.values(enum_._zod.def.entries) }] |
There was a problem hiding this comment.
enum can be number, ... or mix of them so I don't think your approach is acceptable now.
dosubot
bot
added
size:L
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/zod/src/zod4/coercer.native.test.ts (1)
9-17: Unused enum declarations.
NumericEnumandMixedEnumare declared but not used in any test cases in this file. The existing tests only useTestEnum. Either add test cases that exercise these enums or remove the unused declarations.Option 1: Add test cases for numeric and mixed enums
{ name: 'nativeEnum - 123n', schema: z.enum(TestEnum), input: '123n', }, + { + name: 'nativeEnum(NumericEnum) - 1', + schema: z.enum(NumericEnum), + input: '1', + expected: 1, + }, + { + name: 'nativeEnum(MixedEnum) - 1', + schema: z.enum(MixedEnum), + input: '1', + expected: 1, + }, { name: 'enum - 123',Option 2: Remove unused declarations
-enum NumericEnum { - A = 1, - B = 2, -} - -enum MixedEnum { - A = 1, - B = 'b', -} -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod/src/zod4/coercer.native.test.ts` around lines 9 - 17, The file declares NumericEnum and MixedEnum but never uses them; either remove those unused enum declarations or add tests that exercise them analogous to the existing TestEnum cases—specifically add unit tests that import/define NumericEnum and MixedEnum and assert coercion/validation behavior using the same test patterns as for TestEnum (copy the structure of the TestEnum test blocks and substitute NumericEnum and MixedEnum to cover numeric-only and mixed string/numeric enum coercion scenarios).packages/zod/src/coercer.ts (1)
31-38: Consider extracting shared helper to avoid duplication.This
getValidEnumValuesfunction is identical to the one inpackages/zod/src/converter.ts(lines 77-84). Consider extracting it to a shared utility module to adhere to DRY principles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod/src/coercer.ts` around lines 31 - 38, The getValidEnumValues helper is duplicated; extract it into a shared utility (e.g., create a new exported function getValidEnumValues in a common util module) and replace the local implementations in packages/zod/src/coercer.ts (function getValidEnumValues) and packages/zod/src/converter.ts with imports from that new module, ensure the new util is exported, import paths are updated, and run tests/typechecks to confirm no breaking changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/zod/src/coercer.ts`:
- Around line 31-38: The getValidEnumValues helper is duplicated; extract it
into a shared utility (e.g., create a new exported function getValidEnumValues
in a common util module) and replace the local implementations in
packages/zod/src/coercer.ts (function getValidEnumValues) and
packages/zod/src/converter.ts with imports from that new module, ensure the new
util is exported, import paths are updated, and run tests/typechecks to confirm
no breaking changes.
In `@packages/zod/src/zod4/coercer.native.test.ts`:
- Around line 9-17: The file declares NumericEnum and MixedEnum but never uses
them; either remove those unused enum declarations or add tests that exercise
them analogous to the existing TestEnum cases—specifically add unit tests that
import/define NumericEnum and MixedEnum and assert coercion/validation behavior
using the same test patterns as for TestEnum (copy the structure of the TestEnum
test blocks and substitute NumericEnum and MixedEnum to cover numeric-only and
mixed string/numeric enum coercion scenarios).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8a3517c-2928-46e9-a376-6c5f8e7a500d
📒 Files selected for processing (7)
packages/zod/src/coercer.test.tspackages/zod/src/coercer.tspackages/zod/src/converter.test.tspackages/zod/src/converter.tspackages/zod/src/zod4/coercer.native.test.tspackages/zod/src/zod4/converter.native.test.tspackages/zod/src/zod4/converter.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/zod/src/zod4/converter.ts
- packages/zod/src/converter.ts
- Extracted getValidEnumValues to shared util module - Added coercion tests for NumericEnum and MixedEnum to complete coverage Fixes nitpicks from CodeRabbit review.
|
Updated based on feedback. Changes made:
Ready for re-review! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/zod/src/util.ts (1)
1-10: KeepgetValidEnumValuesbehavior aligned with the zod4 helper.
packages/zod/src/zod4/converter.tsstill has an earlyArray.isArray(obj)return, but this shared helper does not. Adding that guard here keeps behavior consistent and prevents drift if callers pass array-backed enum-like values.Suggested patch
export function getValidEnumValues(obj: any): any[] { + if (Array.isArray(obj)) + return obj + const validKeys = Object.keys(obj).filter( (k: any) => typeof obj[obj[k]] !== 'number', ) const filtered: any = {} for (const k of validKeys) { filtered[k] = obj[k] } return Object.values(filtered) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zod/src/util.ts` around lines 1 - 10, The getValidEnumValues function should early-return when passed an array to match the zod4 helper behavior: add an initial guard in getValidEnumValues that checks Array.isArray(obj) and returns obj (as any[]) immediately; keep the existing enum-object filtering logic (validKeys filter, building filtered, and returning Object.values(filtered)) for non-array inputs so callers that pass array-backed enum-like values behave identically to zod4/converter.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/zod/src/util.ts`:
- Around line 1-10: The getValidEnumValues function should early-return when
passed an array to match the zod4 helper behavior: add an initial guard in
getValidEnumValues that checks Array.isArray(obj) and returns obj (as any[])
immediately; keep the existing enum-object filtering logic (validKeys filter,
building filtered, and returning Object.values(filtered)) for non-array inputs
so callers that pass array-backed enum-like values behave identically to
zod4/converter.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3516f53-3733-4e54-93d2-3d4a2fff0f5e
📒 Files selected for processing (4)
packages/zod/src/coercer.tspackages/zod/src/converter.tspackages/zod/src/util.tspackages/zod/src/zod4/coercer.native.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/zod/src/converter.ts
- packages/zod/src/zod4/coercer.native.test.ts
|
I've added the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/zod/src/zod4/converter.ts`:
- Around line 88-97: The file defines a duplicate getValidEnumValues function;
remove this local definition and import the canonical implementation from the
shared utility (packages/zod/src/util.ts) instead. Specifically, delete the
local getValidEnumValues declaration in converter.ts and add an import for
getValidEnumValues from the shared util at the top of the file, then ensure any
usages in converter.ts reference that imported symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5b5f1e2-8344-4830-8a4b-b49cefb64fe8
📒 Files selected for processing (3)
packages/zod/src/converter.tspackages/zod/src/util.tspackages/zod/src/zod4/converter.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zod/src/util.ts
| function getValidEnumValues(obj: any): any[] { | ||
| if (Array.isArray(obj)) | ||
| return obj | ||
| const validKeys = Object.keys(obj).filter((k: any) => typeof obj[obj[k]] !== 'number') | ||
| const filtered: any = {} | ||
| for (const k of validKeys) { | ||
| filtered[k] = obj[k] | ||
| } | ||
| return Object.values(filtered) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate function definition — import from shared utility instead.
This local getValidEnumValues function duplicates the canonical implementation in packages/zod/src/util.ts. The PR objectives state this was extracted to a shared utility module to remove duplication, but this file defines a local copy instead of importing it.
♻️ Proposed fix to import from shared utility
+import { getValidEnumValues } from '../util'
+
export interface ZodToJsonSchemaConverterOptions {Then remove the local function definition at lines 88-97:
-function getValidEnumValues(obj: any): any[] {
- if (Array.isArray(obj))
- return obj
- const validKeys = Object.keys(obj).filter((k: any) => typeof obj[obj[k]] !== 'number')
- const filtered: any = {}
- for (const k of validKeys) {
- filtered[k] = obj[k]
- }
- return Object.values(filtered)
-}
-
export class ZodToJsonSchemaConverter implements ConditionalSchemaConverter {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getValidEnumValues(obj: any): any[] { | |
| if (Array.isArray(obj)) | |
| return obj | |
| const validKeys = Object.keys(obj).filter((k: any) => typeof obj[obj[k]] !== 'number') | |
| const filtered: any = {} | |
| for (const k of validKeys) { | |
| filtered[k] = obj[k] | |
| } | |
| return Object.values(filtered) | |
| } | |
| import { getValidEnumValues } from '../util' | |
| export interface ZodToJsonSchemaConverterOptions { | |
| // ... interface definition ... | |
| } | |
| export class ZodToJsonSchemaConverter implements ConditionalSchemaConverter { | |
| // ... class definition ... | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/zod/src/zod4/converter.ts` around lines 88 - 97, The file defines a
duplicate getValidEnumValues function; remove this local definition and import
the canonical implementation from the shared utility (packages/zod/src/util.ts)
instead. Specifically, delete the local getValidEnumValues declaration in
converter.ts and add an import for getValidEnumValues from the shared util at
the top of the file, then ensure any usages in converter.ts reference that
imported symbol.
|
I don't think you should add filter enum values here, it's outside of this topic. If you just want add 'type' field into enum why just check if every values is string set as string, if every value is finite number set as number, otherwise set undefined |
2 participants
Fixes #1453
Currently enums created with ZodEnum do not generate a
typefield which leads to issues for OpenAPI tooling like hey-api that expects it. Addedtype: "string"(or inferred type) alongside theenumarray to ensure broader compatibility.Summary by CodeRabbit
Improvements
Bug Fixes
Tests