Skip to content

fix: validators enum type#3884

Merged
mrlubos merged 1 commit into
mainfrom
fix/validators-enum-type
May 14, 2026
Merged

fix: validators enum type#3884
mrlubos merged 1 commit into
mainfrom
fix/validators-enum-type

Conversation

@mrlubos

@mrlubos mrlubos commented May 14, 2026

Copy link
Copy Markdown
Member

No description provided.

@bolt-new-by-stackblitz

Copy link
Copy Markdown

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

@changeset-bot

changeset-bot Bot commented May 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: abc8ceb

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

This PR includes changesets to release 3 packages
Name Type
@hey-api/openapi-ts Patch
@hey-api/shared Patch
@hey-api/openapi-python 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

vercel Bot commented May 14, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
web Ready Ready Preview, Comment May 14, 2026 7:22pm

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Debug code (console.log plus a hardcoded querySymbol lookup for #/components/schemas/PermissionAction) is left in packages/openapi-ts/src/plugins/zod/v4/toAst/enum.ts. Must be removed before this leaves draft — it will execute for every enum schema in every consumer's build.

TL;DR — Extracts the enum AST emitter out of the TypeScript plugin's export.ts into a dedicated enum.ts, simplifies the duplicate-key bookkeeping, and moves the empty-literalMembers short-circuit in the Zod v4 enum resolver into baseNode so custom resolvers also benefit.

Key changes

  • Extract exportEnumAst into its own module — moves ~185 lines of enum emission out of shared/export.ts into shared/enum.ts, leaving exportAst to just dispatch.
  • Collapse key resolution into a single pre-pass — replaces the two-step itemsWithAttempts + per-item resolveEnumKey callsite with a single resolveItemsWithKeys that returns { item, key } pairs.
  • Factor out createSymbolMeta — removes three copies of the same { resource: 'definition', tool: 'typescript', ... } literal inside buildSymbolIn calls.
  • Move Zod v4 enum empty-literals guard into baseNodebaseNode now returns unknownToAst({ type: 'unknown' }) when there are no literal members, so custom ~resolvers.enum get the same fallback as the default path.
  • Default validated preset to enums: 'javascript'dev/typescript/presets.ts now passes the typescript plugin as an object so the Zod validator side has matching enum constants to reference.

Summary | 4 files | 1 commit | base: mainfix/validators-enum-type


Debug code left in the Zod v4 enum resolver

Before: baseNode computed items once and emitted the Zod node.
After: baseNode also queries a hardcoded resource ID and console.logs the result on every call.

The new querySymbol({ resourceId: '#/components/schemas/PermissionAction' }) + console.log(a) block (and the commented-out console.log(ctx.schema) below it) clearly belong to a local debugging session — the schema name is repo-specific to whatever spec was being inspected, the result is never used, and console.log is otherwise absent from this plugin tree. Drop the block entirely before un-drafting; otherwise every generated Zod schema using z.enum will emit a console.log at codegen time.

packages/openapi-ts/src/plugins/zod/v4/toAst/enum.ts


TypeScript enum extraction and key resolution

Before: shared/export.ts carried the full enum emitter inline and computed each item's duplicateAttempt separately from its final cased key, re-running toCase + the leading-digit underscore prefix per-iteration.
After: shared/enum.ts owns the emitter end-to-end; resolveItemsWithKeys produces { item, key } pairs in one pass; createSymbolMeta deduplicates the resource: 'definition', tool: 'typescript' boilerplate.

The behavior is preserved: resolveEnumKey({ duplicateAttempt: 0 }) deterministically produces the same baseKey that the old inline expression produced, the nameConflictResolver still receives the post-prefix key, and $.member / $.prop consume the resolved key in the same order. The minor cost is calling resolveEnumKey twice for duplicates (once to derive the base, once with the real attempt), which is negligible and worth the readability win.

packages/openapi-ts/src/plugins/@hey-api/typescript/shared/enum.ts · packages/openapi-ts/src/plugins/@hey-api/typescript/shared/export.ts


Empty-literals fallback hoisted into baseNode

Before: enumToAst short-circuited to unknownToAst when itemsNode produced no literal members, and enumResolver separately returned the untouched ctx.chain.current on the same condition.
After: baseNode performs the check itself and returns unknownToAst({ type: 'unknown' }), so a user-supplied ~resolvers.enum that calls ctx.nodes.base(ctx) gets the same fallback the default path used to get only inside enumToAst.

Worth confirming that no existing custom resolver relied on ctx.nodes.base(ctx) returning an "enum chain shape" specifically (the unknown chain is structurally a Zod expression and should slot into normal chain composition, but it's a behavior change for that surface).

packages/openapi-ts/src/plugins/zod/v4/toAst/enum.ts

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment on lines +63 to +68
const a = ctx.plugin.querySymbol({
resource: 'definition',
resourceId: '#/components/schemas/PermissionAction',
});
console.log(a);
// console.log(ctx.schema)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Debug leftovers — querySymbol with a hardcoded #/components/schemas/PermissionAction resource ID, an unused a, a console.log(a), and a commented-out console.log(ctx.schema). This will run for every enum schema in every consumer's build. Remove the block before un-drafting.

Suggested change
const a = ctx.plugin.querySymbol({
resource: 'definition',
resourceId: '#/components/schemas/PermissionAction',
});
console.log(a);
// console.log(ctx.schema)
const { symbols } = ctx;
const { z } = symbols;
if (allStrings && enumMembers.length) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Correction: the suggestion above duplicates the symbols/z destructuring already on lines 60-61 — please ignore the suggested block and just delete lines 63-68. Apologies for the noise.

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus𝕏

@pkg-pr-new

pkg-pr-new Bot commented May 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

@hey-api/codegen-core

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

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

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

@hey-api/nuxt

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

@hey-api/openapi-ts

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

@hey-api/shared

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

@hey-api/spec-types

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

@hey-api/types

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

@hey-api/vite-plugin

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

commit: abc8ceb

@codecov

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 3.09735% with 219 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.48%. Comparing base (9e52e26) to head (abc8ceb).

Files with missing lines Patch % Lines
...-ts/src/plugins/@hey-api/typescript/shared/enum.ts 3.38% 42 Missing and 15 partials ⚠️
...es/openapi-ts/src/plugins/valibot/v1/toAst/enum.ts 0.00% 16 Missing and 5 partials ⚠️
...ckages/openapi-ts/src/plugins/zod/v3/toAst/enum.ts 0.00% 14 Missing and 5 partials ⚠️
...ages/openapi-ts/src/plugins/zod/mini/toAst/enum.ts 0.00% 12 Missing and 5 partials ⚠️
...ckages/openapi-ts/src/plugins/zod/v4/toAst/enum.ts 0.00% 12 Missing and 5 partials ⚠️
...kages/openapi-ts/src/plugins/valibot/v1/visitor.ts 0.00% 10 Missing and 1 partial ⚠️
...ackages/openapi-ts/src/plugins/zod/mini/visitor.ts 0.00% 10 Missing ⚠️
packages/openapi-ts/src/plugins/zod/v3/visitor.ts 0.00% 10 Missing ⚠️
packages/openapi-ts/src/plugins/zod/v4/visitor.ts 0.00% 10 Missing ⚠️
...s/openapi-ts/src/plugins/valibot/v1/toAst/array.ts 0.00% 7 Missing ⚠️
... and 14 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3884      +/-   ##
==========================================
+ Coverage   39.45%   39.48%   +0.02%     
==========================================
  Files         533      534       +1     
  Lines       19636    19627       -9     
  Branches     5840     5854      +14     
==========================================
+ Hits         7748     7749       +1     
+ Misses       9637     9624      -13     
- Partials     2251     2254       +3     
Flag Coverage Δ
unittests 39.48% <3.09%> (+0.02%) ⬆️

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.

@mrlubos mrlubos force-pushed the fix/validators-enum-type branch from 2b0b774 to 51d4a67 Compare May 14, 2026 16:18
@mrlubos mrlubos force-pushed the fix/validators-enum-type branch from 51d4a67 to e9b1591 Compare May 14, 2026 16:34
@mrlubos mrlubos marked this pull request as ready for review May 14, 2026 16:35
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug 🔥 Broken or incorrect behavior. labels May 14, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TL;DR — Teaches the Zod validator plugin (v3, v4, mini) to reuse a TypeScript-emitted enum/const symbol via z.nativeEnum(...) / z.enum(...) when one exists, instead of always inlining literal members. Threads SchemaVisitorContext.path through every toAst entrypoint so the lookup can compute a resourceId, retires the walkerCtx field, and extracts the existing TS enum codegen into its own shared/enum.ts.

Key changes

  • Reuse TS enum/const symbols in Zod validators — when @hey-api/typescript runs alongside Zod and emits a non-const enum or a const object for the same $ref, enumToAst now emits z.nativeEnum(Foo) (v3) or z.enum(Foo) (v4/mini) instead of repeating the literals.
  • path replaces walkerCtx — every Zod toAst function and its Pick<...> option type now takes path directly, and BaseContext extends SchemaVisitorContext. walkerCtx: ctx call sites become path: ctx.path.
  • exportEnumAst extracted — the TS enum emitter is split out of @hey-api/typescript/shared/export.ts into a new shared/enum.ts, with resolveItemsWithKeys consolidating the per-item duplicate-key bookkeeping into one pass.
  • querySymbols() added to PluginInstance — companion to querySymbol() returning the full array; the three existing internal call sites switch over.
  • dev:ts/dev:py watch scopes narrowedtsx watch now only re-runs on relevant input/config/preset files instead of the entire monorepo.

Summary | 57 files | 1 commit | base: mainfix/validators-enum-type


Test coverage gap for the new lookup path

Before: Zod plugin emitted inline z.enum([...]) / z.union([...]) literals regardless of co-plugins.
After: Zod plugin emits z.nativeEnum(TsEnum) (v3) or z.enum(TsConst) (v4/mini) when a TS symbol matches the same resourceId.

The Zod snapshot matrix under packages/openapi-ts-tests/zod/v{3,4}/__snapshots__/3.{0,1}.x/{mini,v3,v4}/ was not regenerated, and none of the existing scenarios co-load @hey-api/typescript. The new querySymbols branch is therefore unreachable from any test in this PR — existing snapshots stay green because the lookup always returns [] and the inline-literals fallback runs. Consider adding scenarios that combine @hey-api/typescript (with both enums: 'typescript-const' and enums: 'javascript' cells) and zod, so each of the three dialects produces a snapshot proving the z.nativeEnum(...) / z.enum(...) selection, mirroring the matrix symmetry the repo expects.

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


Ordering and SDK-validator path

The lookup assumes @hey-api/typescript has already registered its enum/var symbol by the time the Zod walker reaches the same $ref. With a user-supplied plugins array that lists zod before @hey-api/typescript, querySymbols returns [] and the inline literal fallback runs — silently — so the behavior degrades to today's output without any signal. Separately, createRequestSchemaContext and createResponseValidator* in all three api.ts files seed path: ref([]); nested walks inside an SDK validator layer therefore can never match a definition-scoped TS symbol. Both behaviors look intentional, but neither is documented, so future readers may chase phantom regressions.

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

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏


const def = ctx.plugin
.querySymbols({
resource: 'definition', // maybe we shouldn't hardcode definition

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Self-doubting TODO comment leaked into production code (same comment is duplicated in v4 and mini enum.ts). Either commit to 'definition' and drop the comment, or thread the resource type through the lookup. The 13-line querySymbols(...).filter(...)[0] block below is also identical across all three dialects — only identifiers.nativeEnum vs identifiers.enum differs — so extracting it to zod/shared/ would let one resolved version replace three copies.


const def = ctx.plugin
.querySymbols({
resource: 'definition', // maybe we shouldn't hardcode definition

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Third copy of the // maybe we shouldn't hardcode definition comment and the same lookup block as v3/mini. Worth resolving once in a shared helper.


const def = ctx.plugin
.querySymbols({
resource: 'definition', // maybe we shouldn't hardcode definition

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Third copy of the same TODO + lookup block as v3/v4. Extracting it to zod/shared/ would unify the dialect-only-differs-by-identifier logic.

return this.querySymbols(filter)[0];
}

querySymbols(filter: SymbolMeta): Array<Symbol<ResolvedNode>> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New public method is welcome — a JSDoc clarifying that querySymbols returns symbols in registration (insertion) order would lock that contract in, since callers like the new Zod enum lookup take [0]. Today it relies on gen.symbols.query ordering by convention.

@mrlubos

mrlubos commented May 14, 2026

Copy link
Copy Markdown
Member Author

Open in StackBlitz

@hey-api/codegen-core

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

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

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

@hey-api/nuxt

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

@hey-api/openapi-ts

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

@hey-api/shared

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

@hey-api/spec-types

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

@hey-api/types

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

@hey-api/vite-plugin

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

commit: 9a78bfd

@volkankalin feel free to try and let me know if there's anything missing!

@mrlubos mrlubos force-pushed the fix/validators-enum-type branch from c3a154e to abc8ceb Compare May 14, 2026 19:20
@mrlubos mrlubos merged commit 72f15a1 into main May 14, 2026
11 of 12 checks passed
@mrlubos mrlubos deleted the fix/validators-enum-type branch May 14, 2026 19:29
@hey-api hey-api Bot mentioned this pull request May 14, 2026
@volkankalin

Copy link
Copy Markdown

@mrlubos Thank you!
So as far as I understood from the changes, the zod plugin checks if there is a native enum already registered, if thats the case then it registers the validator by using this symbol, which means @hey-api/typescript plugin has to be included in the plugins.

I have tested and worked well, thank you!

Although, one issue, when I try to use single output file, somehow can't make it work. I assume this is out of context or maybe I have misunderstood how the config should be set.

Here is the config I used:

createClient({
  input: './enum-inline.yaml', // Example schema from the repo (specs\3.1.x\enum-inline.yaml)
  output: {
    fileName: 'EnumInlineOutput',
    path: './generated',
  },
  parser: {
    transforms: {
      enums: 'root'
    }
  },
  plugins: [
    {
      enums: 'typescript',
      name: '@hey-api/typescript',
    },
    {
      name: 'zod',
      // 👇 Tried both with and without infer
      types: {
        infer: true
      }
    },
  ],
});

@mrlubos

mrlubos commented May 14, 2026

Copy link
Copy Markdown
Member Author

@volkankalin you can use parser hooks to pipe every symbol into a single file, see https://heyapi.dev/docs/openapi/typescript/configuration/parser#example-alphabetic-sort

It's a low-level API so you get full control with it. I plan to abstract it one day, for now this is the only way

@mrlubos

mrlubos commented Jun 1, 2026

Copy link
Copy Markdown
Member Author

@volkankalin ugh, I'm going to have to revert this nice feature – there are too many scenarios to handle and I don't have the time to dedicate to them right now.

@volkankalin

Copy link
Copy Markdown

@mrlubos No worries at all. The current version we are using satisfies all the scenarios we need. So we will stick with this version for a while in that case. Thank you for the heads-up 🙏

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:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants