Skip to content

Commit f1f65ec

Browse files
committed
fix(axios): eliminate module-level returnTypesToWrite map (#3685)
The axios generator used a module-level Map (returnTypesToWrite) to collect *Result type declarations during implementation generation, then read them during footer generation. In tags-split mode, all implementations are generated first (populating the map), then footers are generated per-tag. When two operations from different tags shared the same operationName (via override), the second overwrote the first's entry, causing the wrong *Result type to be emitted in the first tag's footer. Fix: eliminate the module-level map entirely. The return type generator is now returned from generateAxiosImplementation as part of GeneratorClient.returnType, stored on GeneratorOperation.types.result, and passed to the footer via the new optional operations param on ClientFooterBuilder/GeneratorClientFooter. The footer reads from operations[].types.result instead of the side-effect map, ensuring each tag's footer only emits types for its own operations.
1 parent 1134ba5 commit f1f65ec

7 files changed

Lines changed: 170 additions & 48 deletions

File tree

packages/axios/src/index.ts

Lines changed: 49 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ const PARAMS_SERIALIZER_DEPENDENCIES: GeneratorDependency[] = [
4949
},
5050
];
5151

52-
const returnTypesToWrite = new Map<string, (title?: string) => string>();
53-
5452
export const getAxiosDependencies: ClientDependenciesBuilder = (
5553
hasGlobalMutator,
5654
hasParamsSerializerOptions: boolean,
@@ -147,17 +145,14 @@ const generateAxiosImplementation = (
147145
)
148146
: '';
149147

150-
returnTypesToWrite.set(
151-
operationName,
152-
(title?: string) =>
153-
`export type ${pascal(
154-
operationName,
155-
)}Result = NonNullable<Awaited<ReturnType<${
156-
title
157-
? `ReturnType<typeof ${title}>['${operationName}']`
158-
: `typeof ${operationName}`
159-
}>>>`,
160-
);
148+
const returnType = (title?: string) =>
149+
`export type ${pascal(
150+
operationName,
151+
)}Result = NonNullable<Awaited<ReturnType<${
152+
title
153+
? `ReturnType<typeof ${title}>['${operationName}']`
154+
: `typeof ${operationName}`
155+
}>>>`;
161156

162157
const propsImplementation =
163158
mutator.bodyTypeName && body.definition
@@ -167,16 +162,19 @@ const generateAxiosImplementation = (
167162
)
168163
: toObjectString(props, 'implementation');
169164

170-
return `const ${operationName} = (\n ${propsImplementation}\n ${
171-
isRequestOptions && mutator.hasSecondArg
172-
? `options${context.output.optionsParamRequired ? '' : '?'}: SecondParameter<typeof ${mutator.name}<${response.definition.success || 'unknown'}>>,`
173-
: ''
174-
}) => {${bodyForm}
165+
return {
166+
implementation: `const ${operationName} = (\n ${propsImplementation}\n ${
167+
isRequestOptions && mutator.hasSecondArg
168+
? `options${context.output.optionsParamRequired ? '' : '?'}: SecondParameter<typeof ${mutator.name}<${response.definition.success || 'unknown'}>>,`
169+
: ''
170+
}) => {${bodyForm}
175171
return ${mutator.name}<${response.definition.success || 'unknown'}>(
176172
${mutatorConfig},
177173
${requestOptions});
178174
}
179-
`;
175+
`,
176+
returnType,
177+
};
180178
}
181179

182180
const options = generateOptions({
@@ -195,28 +193,28 @@ const generateAxiosImplementation = (
195193
hasSignal: false,
196194
});
197195

198-
returnTypesToWrite.set(
199-
operationName,
200-
() =>
201-
`export type ${pascal(operationName)}Result = AxiosResponse<${
202-
response.definition.success || 'unknown'
203-
}>`,
204-
);
196+
const returnType = () =>
197+
`export type ${pascal(operationName)}Result = AxiosResponse<${
198+
response.definition.success || 'unknown'
199+
}>`;
205200

206201
// In factory mode, use the axiosInstance parameter
207202
// In functions mode with global import, .default may be needed based on tsconfig
208203
const axiosRef = isFactoryMode
209204
? 'axiosInstance'
210205
: `axios${isSyntheticDefaultImportsAllowed ? '' : '.default'}`;
211206

212-
return `const ${operationName} = (\n ${toObjectString(props, 'implementation')} ${
213-
isRequestOptions
214-
? `options${context.output.optionsParamRequired ? '' : '?'}: AxiosRequestConfig\n`
215-
: ''
216-
} ): Promise<AxiosResponse<${response.definition.success || 'unknown'}>> => {${bodyForm}
207+
return {
208+
implementation: `const ${operationName} = (\n ${toObjectString(props, 'implementation')} ${
209+
isRequestOptions
210+
? `options${context.output.optionsParamRequired ? '' : '?'}: AxiosRequestConfig\n`
211+
: ''
212+
} ): Promise<AxiosResponse<${response.definition.success || 'unknown'}>> => {${bodyForm}
217213
return ${axiosRef}.${verb}(${options});
218214
}
219-
`;
215+
`,
216+
returnType,
217+
};
220218
};
221219

222220
export const generateAxiosTitle: ClientTitleBuilder = (title) => {
@@ -258,6 +256,7 @@ ${
258256

259257
export const generateAxiosFooter: ClientFooterBuilder = ({
260258
operationNames,
259+
operations,
261260
title,
262261
noFunction,
263262
hasMutator,
@@ -275,13 +274,11 @@ export const generateAxiosFooter: ClientFooterBuilder = ({
275274
\n`;
276275
}
277276

278-
for (const operationName of operationNames) {
279-
if (returnTypesToWrite.has(operationName)) {
280-
// Map.has ensures Map.get will not return undefined, but TS still complains
281-
// bug https://github.com/microsoft/TypeScript/issues/13086
282-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
283-
const func = returnTypesToWrite.get(operationName)!;
284-
footer += func(noFunction ? undefined : title) + '\n';
277+
if (operations) {
278+
for (const operation of operations) {
279+
if (operation.types?.result) {
280+
footer += operation.types.result(noFunction ? undefined : title) + '\n';
281+
}
285282
}
286283
}
287284

@@ -294,27 +291,35 @@ export const generateAxios = (
294291
isFactoryMode = false,
295292
) => {
296293
const imports = generateVerbImports(verbOptions);
297-
const implementation = generateAxiosImplementation(
294+
const { implementation, returnType } = generateAxiosImplementation(
298295
verbOptions,
299296
options,
300297
isFactoryMode,
301298
);
302299

303-
return { implementation, imports };
300+
return { implementation, imports, returnType };
304301
};
305302

306303
// Factory mode generator - axios is optional parameter
307304
export const generateAxiosFactory: ClientBuilder = (verbOptions, options) => {
308-
const { implementation, imports } = generateAxios(verbOptions, options, true);
309-
return { implementation, imports };
305+
const { implementation, imports, returnType } = generateAxios(
306+
verbOptions,
307+
options,
308+
true,
309+
);
310+
return { implementation, imports, returnType };
310311
};
311312

312313
export const generateAxiosFunctions: ClientBuilder = (verbOptions, options) => {
313-
const { implementation, imports } = generateAxios(verbOptions, options);
314+
const { implementation, imports, returnType } = generateAxios(
315+
verbOptions,
316+
options,
317+
);
314318

315319
return {
316320
implementation: 'export ' + implementation,
317321
imports,
322+
returnType,
318323
};
319324
};
320325

packages/core/src/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,6 +1596,7 @@ export interface GeneratorClient {
15961596
mutators?: GeneratorMutator[];
15971597
/** When set, overrides the default verbOption.doc prepended to the implementation */
15981598
docComment?: string;
1599+
returnType?: (title?: string) => string;
15991600
}
16001601

16011602
export interface GeneratorMutatorParsingInfo {
@@ -1660,6 +1661,7 @@ export type ClientHeaderBuilder = (params: {
16601661
export type ClientFooterBuilder = (params: {
16611662
noFunction?: boolean | undefined;
16621663
operationNames: string[];
1664+
operations?: GeneratorOperation[];
16631665
title?: string;
16641666
hasAwaitedType: boolean;
16651667
hasMutator: boolean;
@@ -1930,6 +1932,7 @@ export type GeneratorClientHeader = (data: {
19301932
export type GeneratorClientFooter = (data: {
19311933
outputClient: OutputClient | OutputClientFunc;
19321934
operationNames: string[];
1935+
operations?: GeneratorOperation[];
19331936
hasMutator: boolean;
19341937
hasAwaitedType: boolean;
19351938
titles: GeneratorClientExtra;

packages/core/src/writers/target-tags.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,7 @@ export function generateTargetForTags(
206206
// `isOperationInTagBucket` keeps this in lockstep with how the
207207
// buckets above were built, including untagged operations that were
208208
// routed into the implicit `default` bucket by `addDefaultTagIfEmpty`.
209-
.filter((operation) => isOperationInTagBucket(operation, tag))
210-
.map(({ operationName }) => operationName);
209+
.filter((operation) => isOperationInTagBucket(operation, tag));
211210

212211
const hasAwaitedType = hasTypeScriptAwaitedType(options.packageJson);
213212

@@ -220,7 +219,10 @@ export function generateTargetForTags(
220219

221220
const footer = builder.footer({
222221
outputClient: options.client,
223-
operationNames,
222+
operationNames: operationNames.map(
223+
({ operationName }) => operationName,
224+
),
225+
operations: operationNames,
224226
hasMutator: !!target.mutators?.length,
225227
hasAwaitedType,
226228
titles,

packages/core/src/writers/target.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ export function generateTarget(
156156
const footer = builder.footer({
157157
outputClient: options.client,
158158
operationNames,
159+
operations,
159160
hasMutator: target.mutators.length > 0,
160161
hasAwaitedType,
161162
titles,

packages/orval/src/api.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,14 @@ export async function getApiBuilder({
121121
acc.verbOptions[verbOption.operationId] = verbOption;
122122
}
123123
acc.schemas.push(...schemas);
124-
acc.operations = { ...acc.operations, ...pathOperations };
124+
for (const [key, value] of Object.entries(pathOperations)) {
125+
let operationKey = key;
126+
let counter = 1;
127+
while (Object.hasOwn(acc.operations, operationKey)) {
128+
operationKey = `${key}::${++counter}`;
129+
}
130+
acc.operations[operationKey] = value;
131+
}
125132

126133
return acc;
127134
},

packages/orval/src/client.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ export const generateClientHeader: GeneratorClientHeader = ({
159159
export const generateClientFooter: GeneratorClientFooter = ({
160160
outputClient,
161161
operationNames,
162+
operations,
162163
hasMutator,
163164
hasAwaitedType,
164165
titles,
@@ -186,6 +187,7 @@ export const generateClientFooter: GeneratorClientFooter = ({
186187
} else {
187188
implementation = footer({
188189
operationNames,
190+
operations,
189191
title: titles.implementation,
190192
hasMutator,
191193
hasAwaitedType,
@@ -194,6 +196,7 @@ export const generateClientFooter: GeneratorClientFooter = ({
194196
} catch {
195197
implementation = footer({
196198
operationNames,
199+
operations,
197200
title: titles.implementation,
198201
hasMutator,
199202
hasAwaitedType,
@@ -345,6 +348,9 @@ export const generateOperations = (
345348
paramsFilter: verbOption.paramsFilter,
346349
operationName: verbOption.operationName,
347350
fetchReviver: verbOption.fetchReviver,
351+
...(client.returnType
352+
? { types: { result: client.returnType } }
353+
: undefined),
348354
};
349355

350356
return acc;

packages/orval/src/generate-spec.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,3 +1261,101 @@ describe('generateSpec - schemas.splitByTags validation', () => {
12611261
}
12621262
});
12631263
});
1264+
1265+
describe('generateSpec - returnTypesToWrite isolation across tags (#3685)', () => {
1266+
it("each tag emits its own *Result type, not the other tag's", async () => {
1267+
const SPEC: OpenApiDocument = {
1268+
openapi: '3.1.0',
1269+
info: { title: 'Collision Demo', version: '1.0.0' },
1270+
paths: {
1271+
'/api/catalog/products': {
1272+
get: {
1273+
tags: ['catalog'],
1274+
operationId: 'getCatalogProducts',
1275+
responses: {
1276+
'200': {
1277+
description: 'ok',
1278+
content: {
1279+
'application/json': {
1280+
schema: { $ref: '#/components/schemas/Product' },
1281+
},
1282+
},
1283+
},
1284+
},
1285+
},
1286+
},
1287+
'/api/inventory/products': {
1288+
get: {
1289+
tags: ['inventory'],
1290+
operationId: 'getInventoryProducts',
1291+
responses: {
1292+
'200': {
1293+
description: 'ok',
1294+
content: {
1295+
'application/json': {
1296+
schema: { $ref: '#/components/schemas/Stock' },
1297+
},
1298+
},
1299+
},
1300+
},
1301+
},
1302+
},
1303+
},
1304+
components: {
1305+
schemas: {
1306+
Product: {
1307+
type: 'object',
1308+
properties: { id: { type: 'string' } },
1309+
},
1310+
Stock: {
1311+
type: 'object',
1312+
properties: { count: { type: 'integer' } },
1313+
},
1314+
},
1315+
},
1316+
};
1317+
1318+
const workspace = await createTempWorkspace();
1319+
1320+
try {
1321+
const options = await normalizeOptions(
1322+
{
1323+
input: { target: SPEC },
1324+
output: {
1325+
target: './endpoints.ts',
1326+
mode: 'tags-split',
1327+
schemas: './model',
1328+
client: 'axios',
1329+
override: {
1330+
operationName: () => 'getProducts',
1331+
},
1332+
},
1333+
},
1334+
workspace,
1335+
);
1336+
1337+
await generateSpec(workspace, options);
1338+
1339+
const catalogContent = await fs.readFile(
1340+
path.join(workspace, 'catalog', 'catalog.ts'),
1341+
'utf-8',
1342+
);
1343+
const inventoryContent = await fs.readFile(
1344+
path.join(workspace, 'inventory', 'inventory.ts'),
1345+
'utf-8',
1346+
);
1347+
1348+
// Both tags share the same operationName (getProducts) via override,
1349+
// but each must emit its own *Result type with the correct schema.
1350+
// Before #3685, the module-level returnTypesToWrite map would
1351+
// overwrite catalog's entry with inventory's.
1352+
expect(catalogContent).toContain('AxiosResponse<Product>');
1353+
expect(catalogContent).not.toContain('AxiosResponse<Stock>');
1354+
1355+
expect(inventoryContent).toContain('AxiosResponse<Stock>');
1356+
expect(inventoryContent).not.toContain('AxiosResponse<Product>');
1357+
} finally {
1358+
await rm(workspace, { recursive: true, force: true });
1359+
}
1360+
});
1361+
});

0 commit comments

Comments
 (0)