fix(hono): some configs generate wrong imports #2448
Conversation
`single` mode generates these files:
```text
.
├── endpoints.context.ts
├── endpoints.handlers.ts
├── endpoints.ts
├── endpoints.validator.ts
└── endpoints.zod.ts
```
But the contexts file has this import:
```typescript
import { ListPetsParams,
CreatePetsParams,
CreatePetsBody } from './endpoints.schemas';
```
When in reality the TypeScript types live in `endpoints.ts`.
`tags` mode generates these files:
```text
.
├── endpoints.schemas.ts
├── endpoints.validator.ts
├── health.context.ts
├── health.handlers.ts
├── health.ts
├── pets.context.ts
├── pets.handlers.ts
├── pets.ts
└── pets.zod.ts
```
But strangely, an absolute import:
```typescript
import {
ListPetsParams,
CreatePetsParams,
CreatePetsBody
} from 'endpoints.schemas';
```
Refactor the hono client to prevent these sorts of issues.
First, introduce a helper that generates import paths. Prefer computing the relative path between
absolute paths. Callers must know the importer and importee paths but otherwise shouldn't perform
path manipulation even if the relationship between the two is supposedly known.
With path handling abstracted, this lets us remove some duplicated code. Much of the duplication
relates to mode handling, with the difference being the import paths in the code templates.
Split `generateHandlers` into `generateHandlerFiles` and `generateHandlerFile`.
`generateHandlerFiles` decides how to group verbs into files and defers code generation to
`generateHandlerFile`. split `generateContext` and `generateHandlers` similarly.
Remove some duplicate path computation. `generateZvalidator` reports its output path, feed that to
`generateHandlerFiles`. Compute the path to the schemas in `generateExtraFiles` because its location
doesn't vary per tag split.
Externalize the zValidator implemention. It's static and outlining it makes the code easier to read.
Use type imports in the orval code to satisfy the TypeScript compiler.
Generate type imports where we can; it's a good practice.
Use a Set to avoid a quadratic unique algorithm. As a side effect, generate sorted imports.
I notice there are a lot of uses `.reduce` that mutate the accumulator and `.flatMap` that `.push`
into an array; these are usually clearer as a traditional for-loop.
Signed-off-by: Alexandra Parker <alexandra.i.parker.civ@us.navy.mil>
|
7.14.0 has been released if you want to verify all your changes! |
|
thank you! i'll try it with our project at work tomorrow :) |
|
@melloware looks good! i just ran my first two api calls through it and already found some spots where we don't conform to the API spec one oddity i noticed at runtime is if a response body fails validation the server still returns 400 bad request. imo response validation failure should be 500 what do you think |
|
Hmmm 500 is usually only for unexpected errors. 4xx is usually for validation errors I typically use 422 |
|
@melloware I wasn't familiar with 422 so I looked it up https://www.rfc-editor.org/rfc/rfc9110#name-422-unprocessable-content
This makes sense for schema validation–it's valid JSON syntax but it doesn't conform to the schema semantics. If response validation fails, though, the client didn't do anything wrong. There may be no request body at all, and modifying the request is unlikely to fix the issue. The server's response generation is wrong and it requires the server to be reconfigured or updated. In general, the 4XX family should be for client errors, but responses are something the server generates so it doesn't make sense to me to return a 4XX in that case. |
|
Yep when you said validation I thought you meant like they submitted but "last name greater than 50 characters" etc is when I throw 422 If it's bad content like malformed JSON I would throw a 400 bad request. I only throw 500 for unhandled database errors or disk space is full something a client can't do anything about or recover from. Truly an "internal server error" At least that is what I have always done |
`single` mode generates these files:
```text
.
├── endpoints.context.ts
├── endpoints.handlers.ts
├── endpoints.ts
├── endpoints.validator.ts
└── endpoints.zod.ts
```
But the contexts file has this import:
```typescript
import { ListPetsParams,
CreatePetsParams,
CreatePetsBody } from './endpoints.schemas';
```
When in reality the TypeScript types live in `endpoints.ts`.
`tags` mode generates these files:
```text
.
├── endpoints.schemas.ts
├── endpoints.validator.ts
├── health.context.ts
├── health.handlers.ts
├── health.ts
├── pets.context.ts
├── pets.handlers.ts
├── pets.ts
└── pets.zod.ts
```
But strangely, an absolute import:
```typescript
import {
ListPetsParams,
CreatePetsParams,
CreatePetsBody
} from 'endpoints.schemas';
```
Refactor the hono client to prevent these sorts of issues.
First, introduce a helper that generates import paths. Prefer computing the relative path between
absolute paths. Callers must know the importer and importee paths but otherwise shouldn't perform
path manipulation even if the relationship between the two is supposedly known.
With path handling abstracted, this lets us remove some duplicated code. Much of the duplication
relates to mode handling, with the difference being the import paths in the code templates.
Split `generateHandlers` into `generateHandlerFiles` and `generateHandlerFile`.
`generateHandlerFiles` decides how to group verbs into files and defers code generation to
`generateHandlerFile`. split `generateContext` and `generateHandlers` similarly.
Remove some duplicate path computation. `generateZvalidator` reports its output path, feed that to
`generateHandlerFiles`. Compute the path to the schemas in `generateExtraFiles` because its location
doesn't vary per tag split.
Externalize the zValidator implemention. It's static and outlining it makes the code easier to read.
Use type imports in the orval code to satisfy the TypeScript compiler.
Generate type imports where we can; it's a good practice.
Use a Set to avoid a quadratic unique algorithm. As a side effect, generate sorted imports.
I notice there are a lot of uses `.reduce` that mutate the accumulator and `.flatMap` that `.push`
into an array; these are usually clearer as a traditional for-loop.
Signed-off-by: Alexandra Parker <alexandra.i.parker.civ@us.navy.mil>
I completely messed up my commit history somehow. Take two...
singlemode generates these files:But the contexts file has this import:
When in reality the TypeScript types live in
endpoints.ts.tagsmode generates these files:But strangely, an absolute import:
Refactor the hono client to prevent these sorts of issues.
First, introduce a helper that generates import paths. Prefer computing the relative path between absolute paths. Callers must know the importer and importee paths but otherwise shouldn't perform path manipulation even if the relationship between the two is supposedly known.
With path handling abstracted, this lets us remove some duplicated code. Much of the duplication relates to mode handling, with the difference being the import paths in the code templates. Split
generateHandlersintogenerateHandlerFilesandgenerateHandlerFile.generateHandlerFilesdecides how to group verbs into files and defers code generation togenerateHandlerFile. splitgenerateContextandgenerateHandlerssimilarly.Remove some duplicate path computation.
generateZvalidatorreports its output path, feed that togenerateHandlerFiles. Compute the path to the schemas ingenerateExtraFilesbecause its location doesn't vary per tag split.Externalize the zValidator implemention. It's static and outlining it makes the code easier to read.
Use type imports in the orval code to satisfy the TypeScript compiler.
Generate type imports where we can; it's a good practice.
Use a Set to avoid a quadratic unique algorithm. As a side effect, generate sorted imports.
I notice there are a lot of uses
.reducethat mutate the accumulator and.flatMapthat.pushinto an array; these are usually clearer as a traditional for-loop.