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

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

Open
stabildev opened this issue Apr 18, 2024 · 8 comments
Open

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

stabildev opened this issue Apr 18, 2024 · 8 comments
Labels

Comments

@stabildev
Copy link

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 stabildev added the bug label Apr 18, 2024
@stabildev stabildev changed the title OpenAPI breaks type inference for parameterized paths Type inference not working with zod-openapi and path parameters Apr 18, 2024
@stabildev
Copy link
Author

stabildev commented Apr 18, 2024

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
Copy link

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
Copy link
Author

stabildev commented Apr 18, 2024

@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

  • Drizzle with Zod Adapter
  • Hono with Zod Hono OpenAPI

@marceloverdijk
Copy link

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
Copy link
Author

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
Copy link

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

@flipvh
Copy link

flipvh commented Apr 21, 2024

@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
Copy link

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.

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

No branches or pull requests

4 participants