fix: preserve operationId acronym casing#3809
fix: preserve operationId acronym casing#3809thomasvanlankveld wants to merge 10 commits intohey-api:mainfrom
Conversation
When `case: 'preserve'` is set on the typescript plugin, operation-derived types (`*Data`, `*Response`, `*Error`) should keep the spec's `operationId` verbatim. Add a minimal spec with operationId `RequestOperations_describeHTTPRequest` and a snapshot asserting the acronyms and underscore survive. Test fails on main: the IR pre-normalizes `operationId` to camelCase before the plugin's casing runs, so `preserve` preserves the already-flattened `requestOperationsDescribeHttpRequest*` form.
…ed types Add `operationBaseName(operation)` helper in the shared parser utils and route the `@hey-api/typescript` plugin's operation and webhook `buildSymbolIn` calls through it. The helper returns `operation.operationId` verbatim when `operation.id` is the plain camelCase normalization of it, and falls back to `operation.id` when disambiguation (a numeric suffix) was appended — so existing conflict-resolution behavior for duplicate operationIds is preserved. Makes the failing typescript snapshot pass.
Align the `OperationPath.id()` strategy with the helper so `operation.id`-based custom strategies benefit from operationId preservation. For all built-in SDK configurations this is a no-op: with `nesting: 'id'` the IR is path-derived (helper falls back to `operation.id`), and with the default `nesting: 'operationId'` the `fromOperationId` strategy is used instead. Also add a snapshot scenario that runs `@hey-api/sdk` + `@hey-api/typescript` against the acronym spec as an integration regression guard — the SDK method name and referenced types both preserve the raw `operationId` under preserve casing.
Mirror the typescript scenario for the valibot plugin: generate schemas against the acronym spec with `case: 'preserve'` and assert the operation schema names keep the raw operationId (`vRequestOperations_describeHTTPRequest*`). Test fails on main because valibot's `namingAnchor` is `operation.id`, which is pre-normalized to camelCase.
Swap `namingAnchor: operation.id` to `operationBaseName(operation)` across the valibot plugin's request body/headers/path/query, response, webhook, and composite request-schema entry points. Makes the failing valibot snapshot pass.
Mirror the valibot scenario for the zod plugin across v3, v4, and mini. The zod test harness iterates over all three compatibility versions from a single scenario list, so a single addition per test file covers all three zod outputs. Tests fail on main because zod's `namingAnchor` is `operation.id`, which is pre-normalized to camelCase.
Swap `namingAnchor: operation.id` to `operationBaseName(operation)` across the zod plugin's request/response/webhook entry points and the v3, v4, and mini composite request-schema entry points. Makes the failing zod snapshots pass.
|
|
|
@thomasvanlankveld is attempting to deploy a commit to the Hey API Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: bc74632 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 Key changes
Summary ｜ 45 files ｜ 10 commits ｜ base: Preserve acronym casing in operation-derived names
The core logic lives in a new
Every plugin that derives names from operations — The changeset documents the consumer-visible impact: under
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3809 +/- ##
==========================================
+ Coverage 39.80% 39.87% +0.06%
==========================================
Files 530 530
Lines 19467 19483 +16
Branches 5791 5806 +15
==========================================
+ Hits 7748 7768 +20
+ Misses 9487 9484 -3
+ Partials 2232 2231 -1
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/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: |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The core operationBaseName approach is sound and the test coverage via snapshots looks thorough for the case: 'preserve' + default output.case scenario. Two items to consider before merge.
Missed call sites outside the diff: packages/openapi-ts/src/plugins/nestjs/plugin.ts:91 and packages/openapi-ts/src/plugins/fastify/plugin.ts:120 still use name: operation.id directly. If those plugins support case: 'preserve' for operation-derived names, they should also use operationBaseName(operation) for consistency.
Task list (5/5 completed)
- Read the diff — identify major areas of change
- Read the core logic changes (packages/shared, plugins)
- Read the test spec and test config
- Verify correctness of the approach and edge cases
- Self-critique drafted comments and submit review
The old equality check hardcoded `'camelCase'` while `operation.id` is built from `output.case`. Under `output.case: 'PascalCase'` (or any non-camelCase), the comparison never matched and the helper silently fell back to `operation.id`, re-introducing the acronym-squash bug. Canonicalize both sides (strip non-alphanumerics, lowercase) so the match works across all `output.case` values. The canonicalization still catches disambiguation because appended numeric suffixes change the canonical form. Add unit tests covering all `output.case` values, missing operationId, and the disambiguation path.
|
Verified the canonical comparison fix in Task list (4/4 completed)
|
|
Verified the author's fixes in Task list (4/4 completed)
|
|
Re: the nestjs/fastify call sites flagged in the review — considered, deferred. A direct |
|
Review submitted. The Task list (5/5 completed)
|
dosubot
Bot
added
the
size:L
There was a problem hiding this comment.
Well-structured PR with clear problem analysis, thorough test coverage across all plugin surfaces, and a sound disambiguation strategy. Two minor items below — neither blocks merge.
Task list (5/5 completed)
- Read the diff to identify major areas of change
- Review core logic changes in packages/shared and packages/openapi-ts
- Review test spec and test changes
- Review snapshot files for correctness
- Self-critique and submit review
| * // Result: ['getUserById'] | ||
| */ | ||
| id: (): OperationPathStrategy => (operation) => [operation.id], | ||
| id: (): OperationPathStrategy => (operation) => [operationBaseName(operation)], |
There was a problem hiding this comment.
The JSDoc for OperationPath.id() (lines 197–203) still says "Uses operation.id as a single path segment" and the example shows operation.id: 'getUserById' → ['getUserById']. After this change the return value is operationBaseName(operation), which may be operation.operationId rather than operation.id.
Since OperationPath is part of the public API of @hey-api/shared, the doc should reflect the new semantics — something like "Uses the operation's base name (raw operationId when available, falling back to operation.id)." The example could show both cases.
| * // Result: ['getUserById'] | ||
| */ | ||
| id: (): OperationPathStrategy => (operation) => [operation.id], | ||
| id: (): OperationPathStrategy => (operation) => [operationBaseName(operation)], |
There was a problem hiding this comment.
This is a subtle public-API behavior change: anyone calling OperationPath.id() in a custom OperationStructureStrategy and not running the result through applyNaming/toCase will now get the raw operationId instead of the pre-cased operation.id. Built-in plugins are fine (they all apply naming downstream), but external consumers might not. Worth calling out in the changeset or release notes.
There was a problem hiding this comment.
Approving — the two inline comments from the prior review are minor and non-blocking.
Task list (5/5 completed)
- Read the diff to identify major areas of change
- Review core logic changes in packages/shared and packages/openapi-ts
- Review test spec and test changes
- Review snapshot files for correctness
- Self-critique and submit review
Replace the `parser` scope (no parser code changed) with three `plugin(name)` scopes matching where the user-visible changes land: @hey-api/typescript, zod, and valibot. Add a note describing the observable shift in operation-derived names for `case: 'preserve'` consumers. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 participant
Problem
When
output.case: 'preserve'is set and anoperationIdcontains acronyms (e.g.RequestOperations_describeHTTPRequest), operation-derived type and schema names lose the acronym casing during IR normalization.HTTPRequestbecomesHttpRequestin generated*Data/*Responsetypes and in Valibot/Zod validators.Fixes #3797
Root Cause
operationToIdin the parser pre-normalizes the raw operationId viatoCase(sanitizeNamespaceIdentifier(id), targetCase)withtargetCase = output.case ?? 'camelCase'. That's correct for disambiguation and name uniqueness, but downstream consumers that derived names fromoperation.idlost the acronym information the user askedcase: 'preserve'to keep.Fix
Add an
operationBaseName(operation)helper in@hey-api/shared. It returnsoperation.operationIdverbatim when its canonical form (strip non-alphanumerics + lowercase) matches the canonical form ofoperation.id, and falls back tooperation.idwhen they differ — that difference is the signal that the IR appended a disambiguation suffix, which must be kept. The canonical comparison is case- and separator-insensitive so it works across alloutput.casevalues.Route operation-name derivation through the helper:
*Data/*Response/*Errortypes and webhook equivalentsOperationPath.id()strategy (consistency + regression guard)Collision-disambiguation behavior is unchanged — verified against the existing
internal-name-conflictscenario, which still producesCreate2Data/Create3Datafor its three-waycreate/create_/create__operationId collision.Consumer impact
Under
case: 'preserve', operation-derived type and schema names (request/response/error types, zod/valibot schemas) that previously had acronym segments lowercased on re-concatenation (describeHttpRequest) now retain the original casing (describeHTTPRequest). Unaffected under othercasevalues. Downstream code importing these names directly will need to update — the changeset calls this out as well.This fix targets
case: 'preserve'only. Under non-preservecasings, downstreamtoCasestill squashes acronyms — the helper rescues them only where the final casing step is a no-op.Test Coverage
Added snapshot tests across the main/valibot/zod suites using a new spec
specs/3.1.x/acronym-operationid-preserve.yaml(operationId:RequestOperations_describeHTTPRequest). Undercase: 'preserve', generated names now keep the acronym casing (RequestOperations_describeHTTPRequestData,vRequestOperations_describeHTTPRequestPath, etc.).Unit tests for
operationBaseNamein@hey-api/sharedcover the three branches (normal path, missingoperationId, disambiguation) and alloutput.casevalues (camelCase,PascalCase,snake_case,SCREAMING_SNAKE_CASE,preserve).Known limitations / out of scope
Two operation-name sinks are intentionally left alone:
sdk.gen.ts). Flat-surface consumers want camelCase-joined with acronyms preserved (requestOperationsDescribeHTTPRequest); nomethodName.casingvalue delivers that.'preserve'keeps the raw operationId including separators (RequestOperations_describeHTTPRequest);'camelCase'runstoCase, which treats all-caps runs as word boundaries and squashes them (requestOperationsDescribeHttpRequest). The nested shape (nesting: 'operationId'+ customnestingDelimiters) recovers the acronym by splitting into segments, but flips the call site fromclient.requestOperationsDescribeHTTPRequest(...)toclient.RequestOperations.describeHTTPRequest(...)— a migration for any consumer on the flat shape. The real follow-up is a casing mode (or smartertoCase) that treats all-caps runs as a single word — a@hey-api/sharednaming-semantics change, larger than this PR's helper.name. These plugins emitoperation.iddirectly as a schema property. A naive swap tooperationBaseNamewould change behavior under defaultoutput.casetoo (operation.idis camelCased today;operationBaseNamereturns the raw operationId). The correct fix is a companiontoCase(operationBaseName(operation), output.case ?? 'camelCase')in each plugin — deferrable.Discussion
Two alternative approaches that were considered:
operation.idinoperationToId. Fix the root: emit the sanitized-but-uncased operationId into the IR, and let each consumer'sapplyNaming/pathToNamestep handle casing (same way schemas work, derived from the raw$refviapathToName). One change, every surface fixed. Rejected for this PR because it changesoperation.id's shape for user hooks / custom plugins, and becausefastify/nestjsemitoperation.idas a literal property name with no casing step after — they'd need companion casing to avoid a regression. (AlthoughoperationToIdis marked@deprecated)operation.baseNamealongsideoperation.id.idstays the cased lookup key;baseNameis the display base. Cleanest semantically, but a new public IR surface, bigger PR, and solves a design problem beyond the reported bug.Instead, this PR introduces the
operationBaseNamehelper. This should localize the fix, so that it would be relatively easy to clean up if you later want to go for one of the other two approaches.