Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[zod-openapi] Problem with strict type checking of return values #374

Open
WiktorKryzia opened this issue Feb 3, 2024 · 5 comments
Open

Comments

@WiktorKryzia
Copy link

Strict type checking was added in version 0.9.1 in such a way that if response type is declared, the handler of a given route can only return a response of the TypedResponse type, and of the methods available in Hono Context, only the .json() (and deprecated .jsonT()) method returns such a type, which forces that only JSON can be returned from this endpoint.

It's about this piece of code:

// If response type is defined, only TypedResponse is allowed.
  R extends {
    responses: {
      [statusCode: string]: {
        content: {
          [mediaType: string]: ZodMediaTypeObject
        }
      }
    }
  }
    ? HandlerTypedResponse<OutputType<R>>
    : HandlerAllResponse<OutputType<R>>

As a result, if I want to have a route that returns text or e.g. YAML, TS will report an error because the methods Context.text(), Context.body() etc. return the Response type, but TypedResponse is expected.

@yusukebe
Copy link
Member

yusukebe commented Feb 3, 2024

Hi @WiktorKryzia

Indeed, only TypeResponse is now allowed. But I think this is fine.

Since c.text() and c.body() cannot return a type, they cannot be defined in the ZodMediaTypeObject schema. So, if you want to return a response with c.text() or c.body(), write like this:

const route = createRoute({
  method: 'get',
  path: '/hello',
  responses: {
    200: {
      description: 'Say hello'
    }
  }
})

app.openapi(route, (c) => {
  return c.text('hello')
})

@WiktorKryzia
Copy link
Author

Hi @yusukebe

Thanks for the quick reply.
Ok, I understand, so I can only include important information in the description.

I think it's a bit of a pity that it can't be declared in such a way as to explicitly and precisely specify the returned Content-Type (at least) and some example of the returned response in dedicated fields.
Some OpenAPI UIs then nicely display this example, response schema type and add an 'Accept' header to the example query.

Some simple example: to describe an endpoint that returns the exact API version, I have to describe it like this:

  responses: {
    200: {
      description: 'Returns string (`text/plain` body) with API version number, e.g. `API v1.0.0`.',
    },
  }

Instead of, e.g.:

responses: {
    200: {
      description: 'Returns string with API version number.',
      content: {
        'text/plain': {
          schema: z.string().openapi({ example: 'API v1.0.0' }),
        },
      },
    },
  }

Better example: if the same endpoint can return data in YAML and JSON format for different status codes, then response for YAML declared only using description can't specified the Content-Type header, so then such a UI in Accept header won't add the appropriate header value for YAML. (on the one hand, this can be interpreted as an error of this UI, but on the other hand, the ability to specify a response header for each status code would be useful and more "transparent" for the reader of the documentation in my opinion)

@yusukebe
Copy link
Member

yusukebe commented Feb 8, 2024

@WiktorKryzia

I understand what you want to do. Thanks for the explanation.

Certainly it would be better to be able to specify content other than JSON. However, since shema supports only JSON, it would be better to make schema optional or undefined for text/plain.

const route = createRoute({
  method: 'get',
  path: '/info',
  responses: {
    200: {
      description: 'OK',
      content: {
        'text/plain': {
          example: 'API v1.0.0',
          schema: undefined
        }
      }
    }
  }
})

This feature can be added.

@WiktorKryzia
Copy link
Author

Yes exactly, I think it would be really nice to add such an option.

@yusukebe
Copy link
Member

yusukebe commented Feb 8, 2024

@WiktorKryzia

Thanks! I'll work on it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants