astahmer / openapi-zod-client

Generate a zodios (typescript http client with zod validation) from an OpenAPI spec (json/yaml)
openapi-zod-client.vercel.app
823 stars 89 forks source link

Including passthrough() on schemas seems to mess up type inference #196

Closed robdodson closed 12 months ago

robdodson commented 1 year ago

Version: 1.10.2

When I hover over the generated types for a response object, they include wrapping generics. I mentioned this in the Zodios discord and @ecyrbe said that this looked wrong, and that the types should be inferred.

image

I think my issue is related to this change: https://github.com/astahmer/openapi-zod-client/pull/163, which added passthrough() to the end of all object schemas. If I manually remove the passthrough()s, or use the version of openapi-zod-client prior to this change, then the issue goes away.

image

Test case:

openapi: 3.0.3
info:
  title: Bills API
  description: "API spec for the Bill object"
  license:
    name: Apache 2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
  version: 1.0.0
tags:
  - name: list
    description: View many Bills
paths:
  /bills:
    get:
      tags:
        - list
      summary: Fetch all bills
      description: Index endpoint for bills. Can also be used for search
      operationId: getBills
      responses:
        "200":
          description: Success
          content:
            application/json:
              schema:
                type: object
                required:
                  - bills
                properties:
                  bills:
                    type: array
                    items:
                      $ref: "#/components/schemas/Bill"
components:
  schemas:
    Bill:
      type: object
      required:
        - id
        - schema_id
        - created_at
        - created_by
        - updated_at
        - updated_by
      properties:
        id:
          type: integer
          format: int64
        schema_id:
          type: string
          example: "bill_4328792"
        updated_at:
          type: string
          example: "2023-12-30 08:26:49.219717"
        updated_by:
          type: integer
          format: int64
        created_at:
          type: string
          example: "2020-07-27 08:26:49.219717"
        created_by:
          type: integer
          format: int64
import * as React from "react";
import { ZodiosHooks } from "@zodios/react";
import { api } from "./openapi-client";

function App() {
  const hooks = new ZodiosHooks("myAPI", api);
  const { data } = hooks.useGetBills();

  return <div className="App"></div>;
}

export default App;
astahmer commented 1 year ago

from the PR you linked, it looks like the OpenAPI spec behaves that way so I'd say it's correct to implement it this way and your OpenAPI might not be correct then (should have implicit additionalProperties: false) ?

But I also get that you might not be able to control the OpenAPI generation, which is kinda the point of this lib 😅 so, 2 solutions I can think of that should support everyone's needs:

ecyrbe commented 1 year ago

@astahmer the thing is, passthrough seems to break type inference somehow because zod add & [x: string]: unknown. zod can't resolve the types when there are multiple objects with passthrough embedded. i don't know a good solution here. zod could handle these case in their infer parser though. so maybe a better solution would be a fix on zod part ?

astahmer commented 1 year ago

yes, this would be the ideal solution, not sure how feasible that is tho

robdodson commented 1 year ago

from the PR you linked, it looks like the OpenAPI spec behaves that way so I'd say it's correct to implement it this way and your OpenAPI might not be correct then (should have implicit additionalProperties: false) ?

Ah yeah, it looks like the OpenAPI spec defaults additionalProperties to true

additionalProperties - Value can be boolean or object. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema. Consistent with JSON Schema, additionalProperties defaults to true.

Unfortunately it seems like Goa, the tool our backend team uses to write their endpoints, does not provide a way to set additionalProperties to false (stale issue)

either make a PR and reverse that change to additionalProperties when using a CLI/option flag #163 (files)

I would love if we could have a flag to force additionalProperties to false. I think that will be easier than traversing the spec.

i don't know a good solution here. zod could handle these case in their infer parser though. so maybe a better solution would be a fix on zod part ?

It looks like someone filed a similar issue in the Zod repo but it may be a TS problem ☹️

ecyrbe commented 1 year ago

@robdodson it seems that using a Simplify Helper fixes it. i pinged collin on twitter to check where it would be more valuable to add it. I would prefer Zod to add it rather than zodios. Because then if zod breaks it in another way, we'll have to change again inference on zodios.