Skip to content

Commit c6da855

Browse files
xandrisAlexandra Parker
xandris
and
Alexandra Parker
authored
fix(hono): zValidator not imported consistently (#2444)
The branches in generateHandlers independently compute whether the generated code requires the zValidator import. This code hasn't been kept in sync, however, notably for the `tags` and `tags-split` modes. In this case, `generateHandlers` assess that `zValidator` isn't needed, but `getHonoHandlers` produces code that validates the response body anyway, resulting in a Typescript compilation error for first-time users of orval. I don't prefer code generation with string manipulation but don't want to make any drastic changes to someone else's codebase. Instead, have `getHonoHandlers` itself report whether `zValidator` is required by bundling this flag alongside the generated code in a tuple. Also, handle the one verb and many verbs per file cases uniformly by making `getHonoHandlers` variadic. It performs the same kind of reduction that was used before, but also reduces over the zValidator flag. Add tests for this condition to the hono configs. Tag the verbs in `examples.yaml` to trigger the behavior. Signed-off-by: Alexandra Parker <alexandra.i.parker.civ@us.navy.mil> Co-authored-by: Alexandra Parker <alexandra.i.parker.civ@us.navy.mil>
1 parent 2db0724 commit c6da855

File tree

3 files changed

+95
-111
lines changed

3 files changed

+95
-111
lines changed

‎packages/hono/src/index.ts‎

Lines changed: 85 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -131,47 +131,64 @@ export const generateHono: ClientBuilder = async (verbOptions, options) => {
131131
};
132132
};
133133

134-
const getHonoHandlers = ({
135-
handlerName,
136-
contextTypeName,
137-
verbOption,
138-
validator,
139-
}: {
140-
handlerName: string;
141-
contextTypeName: string;
142-
verbOption: GeneratorVerbOptions;
143-
validator: boolean | 'hono' | NormalizedMutator;
144-
}) => {
145-
let currentValidator = '';
146-
147-
if (validator) {
148-
if (verbOption.headers) {
149-
currentValidator += `zValidator('header', ${verbOption.operationName}Header),\n`;
150-
}
151-
if (verbOption.params.length > 0) {
152-
currentValidator += `zValidator('param', ${verbOption.operationName}Params),\n`;
153-
}
154-
if (verbOption.queryParams) {
155-
currentValidator += `zValidator('query', ${verbOption.operationName}QueryParams),\n`;
156-
}
157-
if (verbOption.body.definition) {
158-
currentValidator += `zValidator('json', ${verbOption.operationName}Body),\n`;
159-
}
160-
if (
161-
validator !== 'hono' &&
162-
verbOption.response.originalSchema?.['200']?.content?.['application/json']
163-
) {
164-
currentValidator += `zValidator('response', ${verbOption.operationName}Response),\n`;
165-
}
166-
}
134+
/**
135+
* getHonoHandlers generates TypeScript code for the given verbs and reports
136+
* whether the code requires zValidator.
137+
*/
138+
const getHonoHandlers = (
139+
...opts: Array<{
140+
handlerName: string;
141+
contextTypeName: string;
142+
verbOption: GeneratorVerbOptions;
143+
validator: boolean | 'hono' | NormalizedMutator;
144+
}>
145+
): [
146+
/** The combined TypeScript handler code snippets. */
147+
handlerCode: string,
148+
/** Whether any of the handler code snippets requires importing zValidator. */
149+
hasZValidator: boolean,
150+
] =>
151+
opts.reduce(
152+
([code, hasZValidator], opts) => {
153+
const { handlerName, contextTypeName, verbOption, validator } = opts;
154+
155+
let currentValidator = '';
156+
157+
if (validator) {
158+
if (verbOption.headers) {
159+
currentValidator += `zValidator('header', ${verbOption.operationName}Header),\n`;
160+
}
161+
if (verbOption.params.length > 0) {
162+
currentValidator += `zValidator('param', ${verbOption.operationName}Params),\n`;
163+
}
164+
if (verbOption.queryParams) {
165+
currentValidator += `zValidator('query', ${verbOption.operationName}QueryParams),\n`;
166+
}
167+
if (verbOption.body.definition) {
168+
currentValidator += `zValidator('json', ${verbOption.operationName}Body),\n`;
169+
}
170+
if (
171+
validator !== 'hono' &&
172+
verbOption.response.originalSchema?.['200']?.content?.[
173+
'application/json'
174+
]
175+
) {
176+
currentValidator += `zValidator('response', ${verbOption.operationName}Response),\n`;
177+
}
178+
}
167179

168-
return `
180+
code += `
169181
export const ${handlerName} = factory.createHandlers(
170182
${currentValidator}async (c: ${contextTypeName}) => {
171183
172184
},
173185
);`;
174-
};
186+
hasZValidator ||= currentValidator !== '';
187+
188+
return [code, hasZValidator];
189+
},
190+
['', false] as [string, boolean],
191+
);
175192

176193
const getValidatorOutputRelativePath = (
177194
validatorOutputPath: string,
@@ -264,28 +281,22 @@ const generateHandlers = async (
264281
`./${verbOption.operationName}` + extension,
265282
);
266283

267-
const hasZValidator =
268-
!!verbOption.headers ||
269-
verbOption.params.length > 0 ||
270-
!!verbOption.queryParams ||
271-
!!verbOption.body.definition;
272-
273-
const isExist = fs.existsSync(handlerPath);
274-
275284
const handlerName = `${verbOption.operationName}Handlers`;
276285
const contextTypeName = `${pascal(verbOption.operationName)}Context`;
277286

278-
if (isExist) {
287+
const [handlerCode, hasZValidator] = getHonoHandlers({
288+
handlerName,
289+
contextTypeName,
290+
verbOption,
291+
validator: output.override.hono.validator,
292+
});
293+
294+
if (fs.existsSync(handlerPath)) {
279295
const rawFile = await fs.readFile(handlerPath, 'utf8');
280296
let content = rawFile;
281297

282298
if (!rawFile.includes(handlerName)) {
283-
content += getHonoHandlers({
284-
handlerName,
285-
contextTypeName,
286-
verbOption,
287-
validator: output.override.hono.validator,
288-
});
299+
content += handlerCode;
289300
}
290301

291302
return {
@@ -323,14 +334,7 @@ const generateHandlers = async (
323334
import { ${contextTypeName} } from '${outputPath}.context';
324335
${zodImports}
325336
326-
const factory = createFactory();
327-
328-
${getHonoHandlers({
329-
handlerName,
330-
contextTypeName,
331-
verbOption,
332-
validator: output.override.hono.validator,
333-
})}
337+
const factory = createFactory();${handlerCode}
334338
`;
335339

336340
return {
@@ -351,14 +355,6 @@ ${getHonoHandlers({
351355
? upath.join(dirname, `${kebab(tag)}.handlers${extension}`)
352356
: upath.join(dirname, tag, tag + '.handlers' + extension);
353357

354-
const hasZValidator = verbs.some(
355-
(verb) =>
356-
!!verb.headers ||
357-
verb.params.length > 0 ||
358-
!!verb.queryParams ||
359-
!!verb.body.definition,
360-
);
361-
362358
const isExist = fs.existsSync(handlerPath);
363359

364360
if (isExist) {
@@ -367,17 +363,15 @@ ${getHonoHandlers({
367363

368364
content += Object.values(verbs).reduce((acc, verbOption) => {
369365
const handlerName = `${verbOption.operationName}Handlers`;
370-
const contextTypeName = `${pascal(
371-
verbOption.operationName,
372-
)}Context`;
366+
const contextTypeName = `${pascal(verbOption.operationName)}Context`;
373367

374368
if (!rawFile.includes(handlerName)) {
375369
acc += getHonoHandlers({
376370
handlerName,
377371
contextTypeName,
378372
verbOption,
379373
validator: output.override.hono.validator,
380-
});
374+
})[0];
381375
}
382376

383377
return acc;
@@ -389,6 +383,15 @@ ${getHonoHandlers({
389383
};
390384
}
391385

386+
const [handlerCode, hasZValidator] = getHonoHandlers(
387+
...Object.values(verbs).map((verbOption) => ({
388+
handlerName: `${verbOption.operationName}Handlers`,
389+
contextTypeName: `${pascal(verbOption.operationName)}Context`,
390+
verbOption,
391+
validator: output.override.hono.validator,
392+
})),
393+
);
394+
392395
let validatorImport = '';
393396
if (hasZValidator) {
394397
if (output.override.hono.validator === true) {
@@ -422,21 +425,7 @@ import { ${Object.values(verbs)
422425
.join(',\n')} } from '${outputRelativePath}.context';
423426
${zodImports}
424427
425-
const factory = createFactory();`;
426-
427-
content += Object.values(verbs).reduce((acc, verbOption) => {
428-
const handlerName = `${verbOption.operationName}Handlers`;
429-
const contextTypeName = `${pascal(verbOption.operationName)}Context`;
430-
431-
acc += getHonoHandlers({
432-
handlerName,
433-
contextTypeName,
434-
verbOption,
435-
validator: output.override.hono.validator,
436-
});
437-
438-
return acc;
439-
}, '');
428+
const factory = createFactory();${handlerCode}`;
440429

441430
return {
442431
content,
@@ -446,16 +435,6 @@ const factory = createFactory();`;
446435
);
447436
}
448437

449-
const hasZValidator = Object.values(verbOptions).some(
450-
(verb) =>
451-
!!verb.headers ||
452-
verb.params.length > 0 ||
453-
!!verb.queryParams ||
454-
!!verb.body.definition ||
455-
(verb.response.contentTypes.length === 1 &&
456-
verb.response.contentTypes[0] === 'application/json'),
457-
);
458-
459438
const handlerPath = upath.join(dirname, `${filename}.handlers${extension}`);
460439

461440
const isExist = fs.existsSync(handlerPath);
@@ -474,7 +453,7 @@ const factory = createFactory();`;
474453
contextTypeName,
475454
verbOption,
476455
validator: output.override.hono.validator,
477-
});
456+
})[0];
478457
}
479458

480459
return acc;
@@ -490,6 +469,15 @@ const factory = createFactory();`;
490469

491470
const outputRelativePath = `./${filename}`;
492471

472+
const [handlerCode, hasZValidator] = getHonoHandlers(
473+
...Object.values(verbOptions).map((verbOption) => ({
474+
handlerName: `${verbOption.operationName}Handlers`,
475+
contextTypeName: `${pascal(verbOption.operationName)}Context`,
476+
verbOption,
477+
validator: output.override.hono.validator,
478+
})),
479+
);
480+
493481
let validatorImport = '';
494482

495483
if (hasZValidator) {
@@ -521,21 +509,7 @@ import { ${Object.values(verbOptions)
521509
.join(',\n')} } from '${outputRelativePath}.context';
522510
${zodImports}
523511
524-
const factory = createFactory();`;
525-
526-
content += Object.values(verbOptions).reduce((acc, verbOption) => {
527-
const handlerName = `${verbOption.operationName}Handlers`;
528-
const contextTypeName = `${pascal(verbOption.operationName)}Context`;
529-
530-
acc += getHonoHandlers({
531-
handlerName,
532-
contextTypeName,
533-
verbOption,
534-
validator: output.override.hono.validator,
535-
});
536-
537-
return acc;
538-
}, '');
512+
const factory = createFactory();${handlerCode}`;
539513

540514
return [
541515
{

‎tests/configs/hono.config.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,12 @@ export default defineConfig({
1010
client: 'hono',
1111
},
1212
},
13+
examples: {
14+
input: '../specifications/examples.yaml',
15+
output: {
16+
target: '../generated/hono/examples/endpoints.ts',
17+
mode: 'tags-split',
18+
client: 'hono',
19+
},
20+
},
1321
});

‎tests/specifications/examples.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ info:
99
paths:
1010
/users:
1111
get:
12+
tags: [users]
1213
operationId: 'get-users'
1314
responses:
1415
'200':
@@ -24,6 +25,7 @@ paths:
2425
$ref: '#/components/examples/GetUsersResponseExample'
2526
/users2:
2627
get:
28+
tags: [users2]
2729
operationId: 'get-users-2'
2830
responses:
2931
'200':

0 commit comments

Comments
 (0)