honojs / middleware

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

Zod-OpenAPI returns ZodError - when query param field is `z.array()` and only passing 1 value #660

Open patrick-kw-chiu opened 3 months ago

patrick-kw-chiu commented 3 months ago

Hi there 👋 . I face the following situation where Zod-OpenAPI would return a ZodError - saying it expects a string, instead of an array when I only query by 1 value.

(Note: I did have a work around. I put it in the last of this issue.)

I understand technically "query by 1 value" would map to c.req.query(). The appropriate type is string. However, to support a field that is meant to be a string[], but the consumer may pass only 1 value (query by c.req.queries()), should the z.array() definition of Zod-OpenAPI still work?

One more supporting reason is that, even when I pass 1 value only, that field would also exist in c.req.queries().

?days=mon => console.log(c.req.queries()) => { days: ['mon'] }

Anyway, here are the steps to reproduce:


Steps to reproduce

1. I define a query param field as z.array()

export const GetMerchantsParamsSchema = z.object({
  days: z
    .enum(days)
    .array()
    .optional()
    .openapi({
      param: { name: 'days', in: 'query' },
      example: ['mon'],
    }),
})

2. I try to get the value by:

export const getMerchants = (c) => {
  // ...
  const { days } = c.req.queries() 
  // ...
}

3. Situation:

When I query only one value e.g. ?days=mon, Zod-OpenAPI would return error saying it expects a string, instead of an array

{
  "expected": "array",
  "received": "string",
  "code": "invalid_type",
  "path": [
    "days"
  ],
  "message": "Expected array, received string"
}

When I query by 2 or more values e.v. ?days=mon&days=tue, it works perfectly 👍


Here's my workaround

export const GetMerchantsParamsSchema = z.object({
  days: z
    .enum(days)
    .array()
    .or(z.enum(days)) // <- Add this line :(
    // Make sure to add it after .enum().array(),
    // so that when you pass the OpenAPI schema to swagger editor,
    // it gives you a great selection box
    .optional()
    .openapi({
      param: { name: 'days', in: 'query' },
      example: ['mon'],
    }),
})

Screenshot 2024-07-21 at 5 52 47 PM

QzCurious commented 2 months ago

To maintainers:

Understanding Hono for Purposing Solution

I've spent some time to trace down how a query is shaped before it be parsed by a zod schema.

  1. Zod-OpenAPI uses zod-validator under the hood
  2. And zod-validator uses hono/validator under the hood https://github.com/honojs/middleware/blob/c5fb51f78321617eec9d3bd9004242480fd99bc3/packages/zod-validator/src/index.ts#L44
  3. The value parameter is what a shaped query be passed to
  4. Finally, query is construct (shaped) here https://github.com/honojs/hono/blob/350040470d5d4ffa27e7307f0d71ffeaf5f37acc/src/validator/validator.ts#L123-L129

I think the right spot for solving this issue is to "detect" when zod-validator parsing value under hono/validator. https://github.com/honojs/middleware/blob/c5fb51f78321617eec9d3bd9004242480fd99bc3/packages/zod-validator/src/index.ts#L45

How I do the "Detection" in Next.js

I'm recently facing the same issue that Next.js app router pass down a shaped query same way as hono. To detect a field that should be treat as an array before being parsed by zod, I had this snippet:

export function parseSearchParams<T extends ZodTypeAny>(
  schema: UnwrapInnerType<T> extends SomeZodObject ? T : never,
  q:
    | URLSearchParams // for client side
    | Record<string, string | string[] | undefined>, // for Next.js
): z.output<T> {
  // undefined instead of null for convenience of setting default value for destructing assignment
  type Shaped = Record<keyof z.output<T>, string | undefined | string[]>

  const unwrappedSchema = unwrapInnerType(schema) as SomeZodObject
  if (!(unwrappedSchema instanceof ZodObject))
    throw new Error('schema should compatible with ZodObject/SomeZodObject')

  if (q instanceof URLSearchParams) {
    const obj = {} as Shaped
    for (const [k, s] of Object.entries(unwrappedSchema.shape)) {
      // !!! detecting a field is asking for array !!!
      if (unwrapInnerType(s) instanceof ZodArray) obj[k as keyof Shaped] = q.getAll(k)
      else obj[k as keyof Shaped] = q.get(k) ?? undefined
    }
    return schema.parse(obj)
  }
  else {
    const obj = {} as Shaped
    for (const [k, s] of Object.entries(unwrappedSchema.shape)) {
      // !!! detecting a field is asking for array !!!
      if (unwrapInnerType(s) instanceof ZodArray) {
        obj[k as keyof Shaped] =
          q[k] == null ? [] : Array.isArray(q[k]) ? q[k] : [q[k]]
      } else obj[k as keyof Shaped] = q[k] ?? undefined
    }
    return schema.parse(obj)
  }
}

type UnwrapInnerType<
  T extends
    | ZodEffects<ZodTypeAny>
    | ZodOptional<ZodTypeAny>
    | ZodNullable<ZodTypeAny>
    | ZodReadonly<ZodTypeAny>
    | ZodDefault<ZodTypeAny>
    | ZodCatch<ZodTypeAny>
    | SomeZodObject
    | ZodTypeAny,
> =
  T extends ZodEffects<infer U, infer _, infer __>
    ? UnwrapInnerType<U>
    : T extends ZodOptional<infer U>
      ? UnwrapInnerType<U>
      : T extends ZodNullable<infer U>
        ? UnwrapInnerType<U>
        : T extends ZodReadonly<infer U>
          ? UnwrapInnerType<U>
          : T extends ZodDefault<infer U>
            ? UnwrapInnerType<U>
            : T extends ZodCatch<infer U>
              ? UnwrapInnerType<U>
              : T extends SomeZodObject | ZodTypeAny
                ? T
                : never

function unwrapInnerType<
  T extends
    | ZodEffects<ZodTypeAny>
    | ZodOptional<ZodTypeAny>
    | ZodNullable<ZodTypeAny>
    | ZodReadonly<ZodTypeAny>
    | ZodDefault<ZodTypeAny>
    | ZodCatch<ZodTypeAny>
    | ZodTypeAny,
>(schema: T): UnwrapInnerType<T> {
  if (schema instanceof ZodEffects) {
    return unwrapInnerType(schema.innerType())
  }
  if (schema instanceof ZodOptional) {
    return unwrapInnerType(schema._def.innerType)
  }
  if (schema instanceof ZodNullable) {
    return unwrapInnerType(schema._def.innerType)
  }
  if (schema instanceof ZodReadonly) {
    return unwrapInnerType(schema._def.innerType)
  }
  if (schema instanceof ZodDefault) {
    return unwrapInnerType(schema._def.innerType)
  }
  if (schema instanceof ZodCatch) {
    return unwrapInnerType(schema._def.innerType)
  }
  return schema as any
}

To use it:

interface PageProps {
  searchParams: { [key: string]: string | string[] | undefined }
}

export default async function Page({ searchParams }: PageProps) {
  const query = parseSearchParams(
    z.object({
      status: z.coerce
        .number()
        .array()
        .default([])  // empty array instead of throwing or optional
        .catch([])  // prevent state=not-a-number throwing
    }),
    searchParams,
  )
}

query.status
//         ^? (property) status: number[]

PS: A ZodCatch (made from z.number().catch()) would make Zod-OpenAPI crash. I'm not sure if it's a issue on Zod-OpenAPI or zod-to-openapi.

After All

Though it's not a perfect solution, it touches implementation detail of zod, I thought this snippet might save you sometime while implementing this.

If it look fine to you, I'm also willing to do a PR.

QzCurious commented 2 months ago

@patrick-kw-chiu Never thought I can trick OpenAPI to do that, cool workaround. But days field is now string | string[] | undefined instead of just string[] still make me not much satisfied.

QzCurious commented 2 months ago

Astro recently mark Astro Actions stable. It also supports validating payload for both JSON FormData. It would be a great reference for this issue.

https://github.com/withastro/astro/blob/1c64ae304d3c008a776e98e48fd3ece8be7b1fb5/packages/astro/src/actions/runtime/virtual/server.ts#L130-L164

yusukebe commented 2 months ago

Hi @patrick-kw-chiu

Please keep using your workaround. That method is best. And, you can get the validated value with the c.req.valid() method. The type definition may be correct.

const data = c.req.valid('query')