middleware icon indicating copy to clipboard operation
middleware copied to clipboard

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

Open WiktorKryzia opened this issue 2 years ago • 8 comments

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.

WiktorKryzia avatar Feb 03 '24 09:02 WiktorKryzia

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')
})

yusukebe avatar Feb 03 '24 12:02 yusukebe

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)

WiktorKryzia avatar Feb 07 '24 17:02 WiktorKryzia

@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.

yusukebe avatar Feb 08 '24 08:02 yusukebe

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

WiktorKryzia avatar Feb 08 '24 12:02 WiktorKryzia

@WiktorKryzia

Thanks! I'll work on it later.

yusukebe avatar Feb 08 '24 16:02 yusukebe

Related: #641

@yusukebe Allowing schema: undefined seems to be a nice solution. Will this be implemented soon?

marvinruder avatar Aug 15 '24 18:08 marvinruder

+1

micheledicosmo avatar Nov 07 '24 14:11 micheledicosmo

any updates? would love to have optional defined schema here

abielzulio avatar Nov 18 '24 12:11 abielzulio