hey-api / openapi-ts

✨ Turn your OpenAPI specification into a beautiful TypeScript client
https://heyapi.vercel.app
MIT License
634 stars 44 forks source link

Transform generation fails if route has multiple success responses #705

Closed topical closed 5 days ago

topical commented 1 week ago

Description

If you have a route with multiple success responses, activating "date-time" transformer (see #670) in "client-fetch" client leads to

Route PUT /abc/def has more than 1 success response and will not currently have a transform generated. If you have a use case for this please open an issue https://github.com/hey-api/openapi-ts/issues

I understand that the general case is hard to implement (see #496 and #490) but there are simple cases that are easy to handle (see simplified example below):

Especially with "PUT" routes, there are by definition 2 scenarios with different success responses:

In my scenario, both responses are empty and thus can be handled the same.

As you may notice, modification returns 204 in my API (other examples return 200 and the new object) as this endpoint uploads a PDF, so I don't want embed it into response.

OpenAPI specification (optional)

/data/{id}/document:
    put:
      operationId: documentUpload
      requestBody:
        required: true
        content:
          application/pdf:
            schema:
              type: string
              format: binary
      responses:
        "201":
          description: Created
        "204":
          description: Modified

Configuration

import { defineConfig } from '@hey-api/openapi-ts'

export default defineConfig({
  input: 'my-api.yaml',
  output: {
    path: 'src/client',
    format: 'prettier',
    lint: 'eslint'
  },
  client: '@hey-api/client-fetch',
  debug: true,
  schemas: false,
  types: {
    enums: false,
    dates: 'types+transform'
  }
})

System information (optional)

No response

mrlubos commented 1 week ago

@Nick-Lucas want to take this on?

Nick-Lucas commented 1 week ago

Thanks @topical for raising this

As you may have picked up on it's an initial scope restriction so we could get this into the wild but should be possible to do. I built just enough for my use case at work

I will eventually get to it but if anyone needs it urgently then PRs are definitely welcome.

I propose the responseTransformer param on clients/requests will need changing to pass the status code back as a second param, and then response transformers emission (where this warning is emitted from) will need a bit of new codegen to check the status code and call the appropriate schema - so not actually a ton of work if you fancy giving it a shot

mrlubos commented 1 week ago

@topical can you create a reproducible StackBlitz example? I don't see a 200 response in your example but I assume that's what's missing?

topical commented 6 days ago

@mrlubos There is no 200 response, only 201 (creation successful) and 204 (modified successfully). Both have empty response.

mrlubos commented 6 days ago

@topical In that case I'm not understanding what's the proposed change? The warning message doesn't throw, and handling your example would mean doing nothing as there's nothing to transform, correct? I understand handling void would mean we get rid of the warning message, is there any benefit beyond that?

topical commented 6 days ago

@mrlubos You are right. It's only a warning message, but "a big red one" I would like to get rid of.

Nick-Lucas commented 6 days ago

It's certainly meant to make it clear that the types will be wrong for that endpoint if a transformer is needed. I'd much rather users get a warning they see and understand action is needed, than a warning they don't see and then raise bug tickets.

Please do open a PR if you feel strongly about lifting this limitation, I'll be happy to provide some guidance. At some point I'll work on it myself otherwise but I just can't commit to a timeline for that right now

mrlubos commented 5 days ago

@topical void responses will be taken care of in the next release. Warnings will be also hidden behind debug flag so they don't scream in your face