Skip to content

Commit 4637eda

Browse files
Himenonclaude
andauthored
fix: preserve parameter order while ensuring path parameters win over 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>
1 parent 938bc6f commit 4637eda

99 files changed

Lines changed: 168043 additions & 19466 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

biome.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
},
4141
"files": {
4242
"ignoreUnknown": true,
43-
"includes": ["**", "!**/dist", "!coverage", "!examples", "!test/code"],
43+
"includes": ["**", "!**/dist", "!coverage", "!examples", "!test/code", "!**/__snapshots__"],
4444
"maxSize": 10485760
4545
},
4646
"javascript": {

scripts/testCodeGenWithClass.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ const main = () => {
4040
Writer.generateTypedefWithTemplateCode("test/kubernetes/openapi-v1.18.5.json", "test/code/class/kubernetes/client-v1.18.5.ts", false, {
4141
sync: false,
4242
});
43+
Writer.generateTypedefWithTemplateCode("test/kubernetes/openapi-v1.28.6.json", "test/code/class/kubernetes/client-v1.28.6.ts", false, {
44+
sync: false,
45+
});
4346
Writer.generateTypedefWithTemplateCode("test/argo-rollout/index.json", "test/code/class/argo-rollout/client.ts", false, {
4447
sync: false,
4548
});

src/code-templates/class-api-client/ApiClientClass/Method.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { EOL } from "node:os";
2-
31
import type { TsGenerator } from "../../../api";
42
import type { CodeGenerator } from "../../../types";
53
import * as MethodBody from "../../_shared/MethodBody";
@@ -161,7 +159,7 @@ export const create = (factory: TsGenerator.Factory.Type, params: CodeGenerator.
161159
comment: option.additionalMethodComment
162160
? [params.operationParams.comment, `operationId: ${params.operationId}`, `Request URI: ${params.operationParams.requestUri}`]
163161
.filter(t => !!t)
164-
.join(EOL)
162+
.join("\n")
165163
: params.operationParams.comment,
166164
deprecated: params.operationParams.deprecated,
167165
type: returnType,

src/code-templates/currying-functional-api-client/FunctionalApiClient/index.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { EOL } from "node:os";
2-
31
import type { TsGenerator } from "../../../api";
42
import type { CodeGenerator } from "../../../types";
53
import type { Option } from "../../_shared/types";
@@ -11,7 +9,7 @@ export const create = (factory: TsGenerator.Factory.Type, list: CodeGenerator.Pa
119
comment: option.additionalMethodComment
1210
? [params.operationParams.comment, `operationId: ${params.operationId}`, `Request URI: ${params.operationParams.requestUri}`]
1311
.filter(t => !!t)
14-
.join(EOL)
12+
.join("\n")
1513
: params.operationParams.comment,
1614
modifiers: ["export"],
1715
declarationList: factory.VariableDeclarationList.create({

src/code-templates/functional-api-client/FunctionalApiClient/index.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { EOL } from "node:os";
2-
31
import type { TsGenerator } from "../../../api";
42
import type { CodeGenerator } from "../../../types";
53
import type { Option } from "../../_shared/types";
@@ -13,7 +11,7 @@ export const create = (factory: TsGenerator.Factory.Type, list: CodeGenerator.Pa
1311
comment: option.additionalMethodComment
1412
? [params.operationParams.comment, `operationId: ${params.operationId}`, `Request URI: ${params.operationParams.requestUri}`]
1513
.filter(t => !!t)
16-
.join(EOL)
14+
.join("\n")
1715
: params.operationParams.comment,
1816
});
1917
});

src/index.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { EOL } from "node:os";
2-
31
import * as Api from "./api";
42
import { generateValidRootSchema } from "./generateValidRootSchema";
53
import type * as Types from "./types";
@@ -63,7 +61,7 @@ export class CodeGenerator {
6361
});
6462
return statements;
6563
};
66-
return [Api.OpenApiTools.Comment.generateLeading(this.resolvedReferenceDocument), Api.TsGenerator.generate(create)].join(EOL + EOL + EOL);
64+
return [Api.OpenApiTools.Comment.generateLeading(this.resolvedReferenceDocument), Api.TsGenerator.generate(create)].join("\n\n\n");
6765
}
6866

6967
/**
@@ -79,7 +77,7 @@ export class CodeGenerator {
7977
return generatorTemplate?.generator(payload, generatorTemplate.option);
8078
});
8179
};
82-
return [Api.OpenApiTools.Comment.generateLeading(this.resolvedReferenceDocument), Api.TsGenerator.generate(create)].join(EOL + EOL + EOL);
80+
return [Api.OpenApiTools.Comment.generateLeading(this.resolvedReferenceDocument), Api.TsGenerator.generate(create)].join("\n\n\n");
8381
}
8482

8583
/**

src/internal/OpenApiTools/Comment.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { EOL } from "node:os";
21
import { Name, Version } from "../../meta";
32
import type { OpenApi } from "../../types";
43

@@ -22,5 +21,5 @@ export const generateLeading = (schema: OpenApi.Document): string => {
2221
.map(message => {
2322
return `// ${message}`;
2423
})
25-
.join(EOL);
24+
.join("\n");
2625
};

src/internal/OpenApiTools/Walker/Operation.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import { EOL } from "node:os";
2-
31
import type { CodeGenerator, OpenApi } from "../../../types";
42

53
const httpMethodList = ["get", "put", "post", "delete", "options", "head", "patch", "trace"] as const;
@@ -37,7 +35,7 @@ export const create = (rootSchema: OpenApi.Document): State => {
3735
state[operation.operationId] = {
3836
httpMethod,
3937
requestUri,
40-
comment: [operation.summary, operation.description].filter(Boolean).join(EOL),
38+
comment: [operation.summary, operation.description].filter(Boolean).join("\n"),
4139
deprecated: !!operation.deprecated,
4240
requestBody: hasValidMediaType ? requestBody : undefined,
4341
parameters: uniqParameters(parameters),
Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
import { EOL } from "node:os";
2-
31
import type { OpenApi } from "../../../types";
42

53
export const addComment = (comment?: string, externalDocs?: OpenApi.ExternalDocumentation): string | undefined => {
64
if (!externalDocs) {
75
return comment;
86
}
9-
return [comment, "", `@see ${externalDocs.url}`, externalDocs.description].filter(Boolean).join(EOL);
7+
return [comment, "", `@see ${externalDocs.url}`, externalDocs.description].filter(Boolean).join("\n");
108
};

src/internal/OpenApiTools/components/Operation.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { EOL } from "node:os";
21
import * as path from "node:path";
32

43
import type { OpenApi } from "../../../types";
@@ -26,7 +25,7 @@ const generateComment = (operation: OpenApi.Operation): string => {
2625
if (operation.tags) {
2726
comments.push(`tags: ${operation.tags.join(", ")}`);
2827
}
29-
return comments.join(EOL);
28+
return comments.join("\n");
3029
};
3130

3231
export const generateNamespace = (

0 commit comments

Comments
 (0)