honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
20.39k stars 584 forks source link

Type inference not working with zod-openapi and path parameters #2525

Open stabildev opened 7 months ago

stabildev commented 7 months ago

What version of Hono are you using?

4.2.5

What runtime/platform is your app running on?

Wrangler

What steps can reproduce the bug?

Using path parameters (e. g. GET /customers/:id) with @hono/zod-openapi breaks type inference for the complete route.

This renders the Hono Client unusable with zod-openapi.

The following is a simplified example with only one route. If this is combined with another route, e. g. GET /customers (without parameter), type inference will break for this endpoint as well.

import { customerSchema, customersTable } from "@api/db/schema/customers";
import type { Env } from "@api/index";
import { OpenAPIHono, createRoute, z } from "@hono/zod-openapi";
import { and, asc, eq } from "drizzle-orm";
import { Hono } from "hono";
import { hc, type InferResponseType } from "hono/client";

const customersRoutes = new Hono<Env>().route(
  "/customers",
  new OpenAPIHono<Env>().openapi(
    createRoute({
      path: "/{id}",
      method: "get",
      request: {
        params: z.object({
          id: z.number().int().positive(),
        }),
      },
      responses: {
        200: {
          content: {
            "application/json": {
              schema: z.object({
                data: customerSchema,
              }),
            },
          },
          description: "The customer",
        },
        404: {
          content: {
            "application/json": {
              schema: z.object({
                error: z.string(),
              }),
            },
          },
          description: "Customer not found",
        },
      },
    }),
    async (c) => {
      const db = c.get("db");
      const userId = c.get("userId");

      const { id } = c.req.valid("param");

      const [customer] = await db
        .select()
        .from(customersTable)
        .where(and(eq(customersTable.userId, +userId), eq(customersTable.id, id)))
        .orderBy(asc(customersTable.name));

      if (!customer) {
        return c.json({ error: "Customer not found" }, 404);
      }

      return c.json({ data: customer });
    },
  ),
);

export default customersRoutes;

What is the expected behavior?

Type inference should return the correct response type:

type ResponseType = {
    error: string;
} | {
    data: {
        id: number;
        name: string;
        email: string | null;
        createdAt: string;
        updatedAt: string;
        deletedAt: string | null;
        userId: number;
        address: string;
        tel: string | null;
    };
}

What do you see instead?

The response type is inferred as any:

type ResponseType = any

Additional information

Working equivalent example without @hono/zod-openapi:

import { customersTable } from "@api/db/schema/customers";
import type { Env } from "@api/index";
import { and, asc, eq } from "drizzle-orm";
import { Hono } from "hono";
import { hc, type InferResponseType } from "hono/client";

const customersRoutes = new Hono<Env>().route(
  "/customers",
  new Hono<Env>().get("/:id", async (c) => {
    const db = c.get("db");
    const userId = c.get("userId");

    const id = c.req.param("id");

    const [customer] = await db
      .select()
      .from(customersTable)
      .where(and(eq(customersTable.userId, +userId), eq(customersTable.id, +id)))
      .orderBy(asc(customersTable.name));

    if (!customer) {
      return c.json({ error: "Customer not found" }, 404);
    }

    return c.json({ data: customer });
  }),
);

export default customersRoutes;

const client = hc<typeof customersRoutes>("localhost");
const get = client.customers[":id"].$get;
const response = await get({ param: { id: "1" } });
type ResponseType = InferResponseType<typeof get>;

Result:

type ResponseType = {
    error: string;
} | {
    data: {
        id: number;
        name: string;
        email: string | null;
        createdAt: string;
        updatedAt: string;
        deletedAt: string | null;
        userId: number;
        address: string;
        tel: string | null;
    };
}

The pattern works for routes without path parameters.

stabildev commented 7 months ago

The issue was here:

createRoute({
      path: "/{id}",
      method: "get",
      request: {
        params: z.object({
          id: z.number().int().positive(), // <--
        }),
      },

Apparently params must be of type string. Maybe this could create type error somewhere?

marceloverdijk commented 7 months ago

You could use coerce:

export const PetIdRequestParamSchema = z.object({
  petId: z.coerce
    .number()
    .int()
    .openapi({
      param: {
        name: 'petId',
        in: 'path',
      },
      description: 'The pet identifier.',
      example: 123,
    }),
});

app.openapi(
  createRoute({
    method: 'get',
    path: '/{petId}',
    summary: 'Get a pet',
    description: 'Returns a pet.',
    tags: ['pet'],
    operationId: 'pets/get',
    request: {
      params: PetIdRequestParamSchema,
    },
    ..
stabildev commented 7 months ago

@marceloverdijk

Even coerce breaks type inference:

image

What seems to be working as a workaround is this:

params: z.object({
  id: z
    .string()
    .transform((v) => Number.parseInt(v))
    .refine((v) => !Number.isNaN(v) && v > 0, { message: "Invalid ID" })
    .openapi({ type: "integer" }),
}),

However, the whole type inference is extremely slow. I have to wait 15-30 seconds every time for any kind of IntelliSense so I am basically writing the code blind, hoping it won't error later. Any ideas how to solve this?

I am using

marceloverdijk commented 7 months ago

hi @stabildev I must admit I found some issue with my suggested approach as well yesterday evening.

The earlier mentioned code:

export const PetIdRequestParamSchema = z.object({
  petId: z.coerce
    .number()
    .int()
    .openapi({
      param: {
        name: 'petId',
        in: 'path',
      },
      description: 'The pet identifier.',
      example: 123,
    }),
});

app.openapi(
  createRoute({
    method: 'get',
    path: '/{petId}',
    summary: 'Get a pet',
    description: 'Returns a pet.',
    tags: ['pet'],
    operationId: 'pets/get',
    request: {
      params: PetIdRequestParamSchema,
    },
    ..

seems correct:

image

however the generated OpenAPI spec looked like:

"/api/pets/{petId}": {
    "get": {
        "summary": "Get a pet",
        "description": "Returns a pet.",
        "tags": [
            "pet"
        ],
        "operationId": "pets/get",
        "parameters": [
            {
                "schema": {
                    "type": [
                        "integer",
                        "null"
                    ],
                    "description": "The pet identifier.",
                    "example": 123
                },
                "required": true,
                "in": "path",
                "name": "petId"
            }
        ],

note the the type being ["integer", "null"] which is incorrect as it is required and should not be null.

I solved it with:

export const PetIdRequestParamSchema = z
  .object({
    petId: z
      .preprocess((val) => {
        if (typeof val === 'string') {
          const num = Number(val);
          if (Number.isInteger(num)) {
            return num;
          } else {
            return NaN;
          }
        }
        return val;
      }, z.number().int())
      .openapi({
        param: {
          in: 'path',
          name: 'petId',
          required: true,
        },
        description: 'The pet identifier.',
        example: 123,
      }),
  })
  .strict();

so first I preprocess the received value, I manually convert it to number, and if that's not possible I return a NaN causing a validation error. It's a bit more code unfortunately but it works and the generated OpenAPI spec is correct as well:

"/api/pets/{petId}": {
    "get": {
        "summary": "Get a pet",
        "description": "Returns a pet.",
        "tags": [
            "pet"
        ],
        "operationId": "pets/get",
        "parameters": [
            {
                "schema": {
                    "type": "integer",
                    "description": "The pet identifier.",
                    "example": 123
                },
                "required": true,
                "in": "path",
                "name": "petId"
            }
        ],

PS: I'm not experiencing any slow type interference.. which IDE are you using?

stabildev commented 7 months ago

hi @marceloverdijk

I'm using VS Code. Are you using the Hono client?

My inference chain looks like this (in this example for the Customer type):

  1. Define Drizzle schema
  2. createSelectSchema from drizzle-zod
  3. Augment the schema using .pick and .extend in createRoute
  4. Group related routes using e. g. app.route("/customers", new OpenAPIHono<Env>().openapi(getCustomersRoute, getCustomersHandler)
  5. export type App = typeof app which is chained and includes all routers and middlewares
  6. Fetch data using HonoClient client = hc<App>("...");
  7. type CustomerResponse = Awaited<ReturnType<typeof client["customers"]["$get"]>
  8. type RawCustomer = ExtractData<CustomerResponse> (utility type to narrow the response to the one containing non-null data field
  9. type Customer = ReturnType<typeof transformCustomer> where transformCustomer: (in: RawCustomer) => Customer
marceloverdijk commented 7 months ago

I'm also using VC Code, but my chain is different. I'm using Prisma.

flipvh commented 7 months ago

@stabildev cool we are using almost the same setup! I also have a bit slow type interference - but still usable - and I think its on the radar of the main developer. We split our api in sections, I might help?

export type Workspace = Extract<InferResponseType<(typeof workspaceClient.workspaces)[':idOrSlug']['$get']>, { data: unknown }>['data'];

you can see how we split our api here: https://github.com/cellajs/cella/blob/development/backend/src/server.ts

that said, the type performance I think is an issue. I think we need an typescript performance expert to somehow look into it. Would be interested in sponsoring that partially.

jvcmanke commented 6 months ago

I was struggling with this as well, however I found a different workaround using with pipe operator form zod that may help you out:

z.string()
 .optional()
 .pipe(z.coerce.number().min(0).default(0))
 .openapi({ type: "integer" }),

This way you can still basically use your current schemas and zod goodies inside pipe, just make sure the initial check is z.string().

The caveat is that the RPC client typing is still string, but on your route it is parsed to number accordingly.

I'm not sure if this helps with type inference times, but it's worth a try.