honojs / middleware

monorepo for Hono third-party middleware/helpers/wrappers
https://hono.dev
421 stars 144 forks source link

Alternative OpenAPI middleware - thoughts on including it in this repo? #758

Open paolostyle opened 2 days ago

paolostyle commented 2 days ago

Hey!

First of all, I'm not trying to promote my library. I'd like to include it in this repo as an official... not sure, either replacement or alternative to the existing zod-openapi middleware.

While I really love Hono and the ecosystem around it, I can't quite say the same about the OpenAPI middleware (zod-openapi). Perhaps I'm alone in this but I developed my apps with just zod-validator (which works great) and I was quite surprised that creating an OpenAPI documentation would require me to refactor pretty much the entire Hono code - you no longer can use the standard Hono class, .get/.post etc. methods (well I guess you can but they wouldn't be documented), I would have to use z object from the library, even gradual migration is difficult. I did not like that and I ended up not implementing OpenAPI in my app at all because of that.

Instead I spent some time to try and create a middleware that would be much simpler, something that would work similarly to zod-validator middleware and would be easy to gradually implement. I believe I succeeded in that and although it's a bit rough around the edges at the moment and I still need to test some edge cases, I do have my apps documented with that library and it works quite well.

A very simple example of how my middleware works:

import { Hono } from 'hono';
import { z } from 'zod';
import { createOpenApi, openApi } from 'hono-zod-openapi';

export const app = new Hono().get(
  '/user',
  openApi(
    // simple response type
    z.object({ hi: z.string() }),
   // request validators, uses zod-validator internally
    {
      query: z.object({ id: z.string() }),
    },
  ),
  (c) => {
    // this still works exactly as with `zod-validator`
    const { id } = c.req.valid('query');
    return c.json({ hi: id }, 200);
  },
)

// this will add a `GET /doc` route to the `app` router
// it's possible to do it manually, also possible to manually adjust the OpenAPI document
createOpenApi(app, {
  title: 'Example API',
  version: '1.0.0',
});

You can pass the response type also as a more complex object or array of them:

[
  {
    status: 200,
    description: '200 OK',
    schema: z
      .object({
        message: z.string(),
      })
      .openapi({
        examples: [
          {
            message: 'Hello, World!',
          },
        ],
      }),
  },
]

There is also support for validating the response but I'm on the fence whether it's actually useful. I think it would be more useful if c.json in the handler would be able to pick up that type and scream at you if it didn't match the schema.

OpenAPI functionality is handled by zod-openapi (so not the same library the official middleware is using, I found this one to be nicer to work with).

You can check out the repo here: https://github.com/paolostyle/hono-zod-openapi.

Now the main question - is there a place for a library like this here, in this repo, considering an official one already exists? Or should I just keep maintaining it in my repo? If it would be possible to include, how should I go about it? Just create a PR and we'd talk about the implementation details etc. there?

bennobuilder commented 2 days ago

@paolostyle I appreciate your work on zod-openapi, especially the use of middleware instead of wrapping routes.

However, I prefer a more explicit OpenAPI schema definition without excessive abstraction. A potential solution could be allowing middleware to take a specific zod-openapi object per route while providing a helper function for easy migration. For example:

// Abstract (with helper function 'fromValidators()')
export const app = new Hono().get(
  '/user',
  openApi(
    fromValidators([zValidator('json', JsonSchema), zValidator('query', QuerySchema)])
  ),
  (c) => {
    const { id } = c.req.valid('query');
    return c.json({ hi: id }, 200);
  },
);

// Specific (raw zod-openapi object)
export const app2 = new Hono().get(
  '/user',
  openApi({
    requestParams: { query: QuerySchema },
    requestBody: { content: { 'application/json': JsonSchema } },
  }),
  (c) => {
    const { id } = c.req.valid('query');
    return c.json({ hi: id }, 200);
  },
);

The idea is to let openApi() accept raw zod-openapi objects, with abstractions added via helpers like fromValidators(). This simplifies migration from zod-validator while offering a more explicit API for those who prefer less abstraction.

This flexibility would make me seriously consider switching to your library over defining OpenAPI schemas first and using my openapi-router for typesafe Hono routes.

cheers :)

paolostyle commented 2 days ago

@bennobuilder Thanks for the comment. I'm not sure if I'm following, though. So just to be clear, the second argument of my middleware defined like this: { json: someZodSchema, header: otherZodSchema } is equivalent to passing two zodValidator middlewares like this: zValidator('json', someZodSchema), zValidator('header', otherZodSchema). The added value of the middleware is that you get the OpenAPI spec for free, OpenAPI specific options are handled through .openapi method on the zod schema, just like in the current version.

For response schemas it is indeed slightly more abstracted but at the end of the day I wanted to do it mostly for myself and I hate the verbosity of the spec, I'd much rather pass a flat list of possible responses than defining a 6 levels deep json/yaml structure. I understand that the spec is somewhat of an industry standard, but my goal was to get an accurate Swagger docs page without needing to deal with yamls. The goal of my middleware is that the developer should spend as little time as possible on the OpenAPI quirks and get reasonable quality docs almost for free while writing idiomatic Hono code. createOpenApi function (the one that actually creates the document) accepts an overrides method where you can modify the doc however you want, and it receives a generated paths object which can be modified as required. This is obviously not the recommended way, I haven't figured out yet if there are things that are impossible to do in the middleware API, things like servers definitely need to be specified there but it's not really something that can be reliably inferred from the Hono object.

If my library was to replace the current one I would definitely have to support the existing createRoute API at least as a migration path, if that's what you meant by "specific zod-openapi object".

adeyemialameen04 commented 1 day ago

Does this have support for the security field in the createRoute yet?

maou-shonen commented 1 day ago

I mostly agree with your point that, as long as we avoid over-abstraction, less code usually leads to a better DX. I would prefer using an API similar to the one below. I might contribute to your library and use it, or perhaps build my own 😅.

new Hono().get(
  "/user",
  oapOperation({
    summary: "Get user",
    tags: ["User"],
  }),
  oapRequest("header", requestHeaderSchema),
  oapRequest("json", requestBodySchema),
  oapResponse("default", defaultResponseSchema),
  oapResponse(403, forbiddenResponseSchema),
  async (c) => {
    const { authorization } = c.req.valid("header")
    if (!authorization) {
      return c.text("Unauthorized", 403)
    }
    const { id } = c.req.valid("json")
    return c.json({ hi: id }, 200)
  },
)

I believe this would cover 95% of use cases, and for other scenarios, we could provide a usage pattern closer to the native implementation. The example below should be equivalent to the one above.


new Hono().get(
  "/user",
  oapOperation({
    summary: "Get user",
    tags: ["User"],
  }),
  oapRequest("header", [
    {
      name: "authorization",
      required: true,
      schema: {
        type: "string",
      },
    },
  ]),
  oapRequest("json", {
    description: "body description",
    required: true,
    content: {
      "application/json": {
        schema: requestBodySchema,
      },
    },
  }),
  oapResponse("default", {
    description: "default response description",
    content: {
      "application/json": {
        schema: defaultResponseSchema,
      },
    },
  }),
  oapResponse(403, {
    description: "forbidden response description",
    content: {
      "text/plain": {
        schema: {
          type: "string",
          example: "Forbidden",
        },
      },
    },
  }),
  async (c) => {
    const { authorization } = c.req.valid("header") // 
    if (!authorization) {
      return c.text("Unauthorized", 403)
    }
    const { id } = c.req.valid("json")
    return c.json({ hi: id }, 200)
  },
)
paolostyle commented 1 day ago

@adeyemialameen04 It is possible, openApi middleware accepts 3rd argument where you can pass any operation-level properties, you'd have to add the security component to createOpenApiDocs overrides (this name is probably not ideal) field, though. You can pass tags, summary etc. there, too. However for now I would not recommend using my library in production as it's still evolving and there might be breaking changes between the versions at least until 1.x version is released.

@maou-shonen Thanks for the feedback, I like this API a lot, just a couple of concerns, but I think they're all solvable:

adeyemialameen04 commented 1 day ago

@adeyemialameen04 It is possible, openApi middleware accepts 3rd argument where you can pass any operation-level properties, you'd have to add the security component to createOpenApiDocs overrides (this name is probably not ideal) field, though. You can pass tags, summary etc. there, too. However for now I would not recommend using my library in production as it's still evolving and there might be breaking changes between the versions at least until 1.x version is released.

@maou-shonen Thanks for the feedback, I like this API a lot, just a couple of concerns, but I think they're all solvable:

* we wouldn't be able to enforce usage of the `oapResponse` (I would change that name, `oap` is a strange abbreviation, it would be probably one of `openApiResponse`/`oapiResponse`/`openApiRes`/`oapiRes`) middleware and if it's missing, we won't be able to generate the document. We could probably just `console.error` it when calling `createOpenApiDocs`, though.

* `description` is a required field for responses, so we'd have to provide default ones based on the status code when used like this: `oapResponse(200, defaultResponseSchema)`. Not a huge issue, but I think `description`, `headers`, `links` and `summary` fields (all of them are part of the OpenAPI spec for response object type) should be passable through a 3rd argument.

* In your `403` example the content type is `text/plain`. It is not possible to provide the media type just through a zod schema. Currently I'm assuming it's always `application/json` unless specified through `mediaType` property. Even though it would be non-standard, I'd lean towards providing it through 3rd argument, like e.g. `description`, I think this would solve the issue for 95% of the cases and for the remaining 5% users would have the fallback to the standard spec definition.

Thanks for your feedback @paolostyle Anticipating v1

maou-shonen commented 1 day ago

@paolostyle

I haven’t thoroughly read every detail of the OpenAPI spec, so my example doesn’t fully account for how to comply with the spec. If you find any issues, you’re probably correct.