Skip to content

fix: preserve parameter order while ensuring path parameters win over same-named query parameters#151

Merged
Himenon merged 8 commits into
mainfrom
fix/issue-148-3
May 27, 2026
Merged

fix: preserve parameter order while ensuring path parameters win over same-named query parameters#151
Himenon merged 8 commits into
mainfrom
fix/issue-148-3

Conversation

@Himenon
Copy link
Copy Markdown
Owner

@Himenon Himenon commented May 27, 2026

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:

parameters:
  - $ref: '#/components/parameters/zone_id'  # path parameter (first)
  - name: bins
    in: query                                 # query parameter (second)

The previous output was:

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
// 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

  • pnpm test:code:gen:class — regenerate code passes
  • pnpm test:code:gen:function — regenerate code passes
  • pnpm test:code:gen:currying-function — regenerate code passes
  • pnpm test:snapshot — all 54 snapshot tests pass
  • New $ref-based path/query conflict scenario added to test/path-parameter/index.yml
  • Kubernetes v1.28.6 snapshot added

🤖 Generated with Claude Code

Himenon and others added 8 commits May 27, 2026 09:43
…napshot

Replace vitest .snap files with plain .ts files per directory structure.
Each snapshot is now stored as a readable TypeScript file alongside the
generated code it verifies, making diffs easier to review in PRs.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Generated snapshot files in __snapshots__ directories contain TypeScript
output from the code generator that is not expected to be type-safe.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@Himenon Himenon merged commit 4637eda into main May 27, 2026
2 checks passed
@Himenon Himenon deleted the fix/issue-148-3 branch May 27, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant