fix: path parameters with same name as query parameters are always required in generated TypeScript interfaces#150
Merged
Merged
Conversation
…-named query parameter
Add a Kubernetes-style scenario to the path-parameter fixture where:
- Path parameters are defined at the PathItem level (not per-operation)
- A query parameter shares the same name as one of the path parameters (`path`)
- Multiple HTTP methods (GET, HEAD) inherit parameters from the PathItem
This reproduces the pattern found in `connectCoreV1HeadNodeProxyWithPath`
(/api/v1/nodes/{name}/proxy/{path}) from the Kubernetes OpenAPI spec, and
captures the current code generation behavior as snapshot baselines.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…enerated TypeScript interfaces
When a PathItem defines both a path parameter and a query parameter with the
same name (e.g. Kubernetes /api/v1/nodes/{name}/proxy/{path}), the previous
implementation used the parameter name alone as the deduplication key in
generatePropertySignatures. This caused the optional query parameter to
overwrite the required path parameter, producing `path?: string` instead of
the correct `path: string`.
Fix: sort parameters before reducing so that path parameters are processed last.
Since they win in the final reduce assignment, their required status is always
preserved in the generated TypeScript interface.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…chema The normalization that forced required: true on path parameters in generateValidRootSchema was not needed for correct TypeScript type generation. Parameter.ts already handles this via isPathProperty in generatePropertySignatureObject, which always produces a required property for path parameters regardless of the required field in the spec. Remove normalizePathParameters and its unit tests, keeping generateValidRootSchema minimal. Update parameter.json snapshots to reflect the absence of the injected required: true field (TypeScript interface output is unchanged). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… test files
- PathParameter-test.ts: extract p() and tpl() helpers to build expected
template literal strings without embedding raw \${...} in regular string literals
- factory.test.ts: add biome-ignore for the line that intentionally tests
escaping of template literal placeholders
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Himenon
changed the title
6 tasks
Himenon
added a commit
that referenced
this pull request
May 27, 2026
… same-named query parameters (#151) ## Summary - Migrated snapshot tests from `toMatchSnapshot` to `toMatchFileSnapshot` for better diff readability - Excluded snapshot files from TypeScript type checking to prevent false errors - Added Kubernetes v1.28.6 OpenAPI spec as a snapshot test target - Fixed a bug in `generatePropertySignatures` where same-named parameters were reordered ## Problem The previous fix for same-named path/query parameter conflicts (#150) used `.sort()` to push path parameters to the end of the array before reducing into a `Record`. This caused all path parameters to appear last in the generated TypeScript interface, regardless of their position in the OpenAPI spec. For example, when the spec defines: ```yaml parameters: - $ref: '#/components/parameters/zone_id' # path parameter (first) - name: bins in: query # query parameter (second) ``` The previous output was: ```typescript export interface Parameter$... { bins?: string; // ← wrong: appeared before zone_id zone_id: string; // ← moved to last by sort } ``` Additionally, the Kubernetes v1.28.6 spec exposes a second pattern: both the path parameter and the query parameter with the same name (`path`) are defined as `$ref` references. The previous sort only resolved `in` for inline parameters, so both references were treated as non-path and the query parameter reference (appearing last) won. ## Fix Replaced the sort + `Record` reduce approach with a `Map`-based iteration that preserves insertion order: - Parameters are processed in their original spec order - When two parameters share the same name, the path parameter's entry wins by overwriting the existing value at its **original position** in the Map (JavaScript `Map` preserves insertion order on `set()` of an existing key) - `resolveParameterIn` now resolves `$ref` references via `store.getParameter()` so both inline and referenced path parameters are correctly identified ```typescript // Before: sort-based (broke insertion order) const sorted = [...parameters].sort((a, b) => { /* path last */ }); const typeElementMap = sorted.reduce<Record<string, string>>(...); return Object.values(typeElementMap); // After: Map-based (preserves spec order) const typeElementMap = new Map<string, string>(); for (const parameter of parameters) { const { name, typeElement } = generatePropertySignatureObject(...); const isPath = resolveParameterIn(store, parameter) === "path"; if (!typeElementMap.has(name) || isPath) { typeElementMap.set(name, typeElement); } } return [...typeElementMap.values()]; ``` ## Test plan - [x] `pnpm test:code:gen:class` — regenerate code passes - [x] `pnpm test:code:gen:function` — regenerate code passes - [x] `pnpm test:code:gen:currying-function` — regenerate code passes - [x] `pnpm test:snapshot` — all 54 snapshot tests pass - [x] New `$ref`-based path/query conflict scenario added to `test/path-parameter/index.yml` - [x] Kubernetes v1.28.6 snapshot added 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test/path-parameter/index.ymlthat reproduces the pattern found inconnectCoreV1HeadNodeProxyWithPath(/api/v1/nodes/{name}/proxy/{path}) from the Kubernetes OpenAPI specgeneratePropertySignatureswhere a same-named query parameter would overwrite a path parameter when building the TypeScript interface, causingpath?: stringinstead of the correctpath: stringRoot Cause
generatePropertySignaturesinParameter.tsused the parameter name alone as the deduplication key when building atypeElementMap. When a PathItem defined both a path parameterpath(required) and a query parameterpath(optional) with the same name — as seen in the Kubernetes spec — the last parameter processed won, and the optional query parameter overwrote the required path parameter.Fix
Sort parameters before reducing so that path parameters are processed last. Since the reduce uses the parameter name as the key, path parameters now always overwrite same-named non-path parameters, preserving the required status in the generated TypeScript interface.
Test plan
pnpm test:code:gen:class— regenerate code passespnpm test:code:gen:function— regenerate code passespnpm test:code:gen:currying-function— regenerate code passespnpm test:snapshot— all 53 snapshot tests pass🤖 Generated with Claude Code