openapi-ts / openapi-typescript

Generate TypeScript types from OpenAPI 3 specs
https://openapi-ts.dev
MIT License
5.8k stars 461 forks source link

Error types cannot be used (expression of type '"..."' can't be used to index type 'NonNullable<FilterKeys<...>>) #1291

Closed EmilePerron closed 7 months ago

EmilePerron commented 1 year ago

Description

When making an API call using the library, using fairly simple code like below, the error object does not allow access to any of the keys that are supposed to be available.

Ex.:

const client = createClient<paths>({ baseUrl: apiBaseUrl, headers: { ...baseHeaders, Authorization: `Bearer ${jwt}` } });
const response = await client.post("/rtm/notes/create", { body: data });

if (response.error) {
  enqueueSnackbar({
    message: response.error["hydra:description"],
    variant: "error",
    autoHideDuration: 10000,
  });

  return;
}

Gives the following error when using response.error["hydra:description"] (or any other key/property on the error):

Element implicitly has an 'any' type because expression of type '"hydra:description"' can't be used to index type 'NonNullable<FilterKeys<{ "application/ld+json": { "@context": string; "@type": string; "hydra:title": string; "hydra:description": string; }; "application/json": { code?: string | undefined; message?: string | undefined; }; } | { ...; } | { ...; }, `${string}/${string}`>>'.
  Property 'hydra:description' does not exist on type 'NonNullable<FilterKeys<{ "application/ld+json": { "@context": string; "@type": string; "hydra:title": string; "hydra:description": string; }; "application/json": { code?: string | undefined; message?: string | undefined; }; } | { ...; } | { ...; }, `${string}/${string}`>>'.ts(7053)

Here is the Typescript error in VS Code:

image

My OpenAPI schema for the responses is the following:

400:
  description: 'Invalid input'
  content:
    application/ld+json:
      schema:
        type: object
        properties:
          '@context': { readOnly: true, type: string, nullable: false }
          '@type': { readOnly: true, type: string, nullable: false }
          'hydra:title': { readOnly: true, type: string, nullable: false }
          'hydra:description': { readOnly: true, type: string, nullable: false }
        required:
          - '@context'
          - '@type'
          - 'hydra:title'
          - 'hydra:description'
    application/json:
      schema:
        type: object
        properties:
          code: { readOnly: true, type: string, nullable: false }
          message: { readOnly: true, type: string, nullable: false }
        required:
          - '@context'
          - '@type'
          - 'hydra:title'
          - 'hydra:description'
422:
  description: 'Unprocessable entity'
  content:
    application/ld+json:
      schema:
        type: object
        properties:
          '@context': { readOnly: true, type: string, nullable: false }
          '@type': { readOnly: true, type: string, nullable: false }
          'hydra:title': { readOnly: true, type: string, nullable: false }
          'hydra:description': { readOnly: true, type: string, nullable: false }
          violations: { readOnly: true, type: array, items: { type: object, properties: { code: { type: string, readOnly: true, nullable: false }, message: { type: string, readOnly: true, nullable: false }, propertyPath: { type: string, readOnly: true, nullable: false } } } }
        required:
          - '@context'
          - '@type'
          - 'hydra:title'
          - 'hydra:description'
    application/json:
      schema:
        type: object
        properties:
          code: { readOnly: true, type: string, nullable: false }
          message: { readOnly: true, type: string, nullable: false }
          violations: { readOnly: true, type: array, items: { type: object, properties: { code: { type: string, readOnly: true, nullable: false }, message: { type: string, readOnly: true, nullable: false }, propertyPath: { type: string, readOnly: true, nullable: false } } } }
        required:
          - '@context'
          - '@type'
          - 'hydra:title'
          - 'hydra:description'

Perhaps it's related to #1128, but my understanding of it is that it would default the error's type to the 400's format - not cause the error above. Do let me know if I'm wrong about this!

Reproduction

This is a typescript error, and so I expect reproducing the code above using the provided API responses schema would trigger the same error.

Expected result

Until #1128 is resolved, my expected result would've been that error be typed as the schema/type for 400, and that its properties could be accessed without any errors.

Checklist

(I don't expect that'll be necessary as I feel I might just be doing something wrong, but if it's something I can fix in a PR, I'd be happy to!)

Thanks in advance! :)

EmilePerron commented 1 year ago

As it turns out this is not only related to errors, but successful responses as well.

Using a similar schema (posted below), the following code breaks with the same error as mentioned above:

const client = createClient<paths>({ baseUrl: apiBaseUrl, headers: { ...baseHeaders, Authorization: `Bearer ${jwt}` } });
const response = await client.get("/rtm/{userId}/notes", { params: { path: { userId: user.id } } });

if (!response.error) {
  console.log(response.data["hydra:member"]);
}

Relevant bit of the schema:

'/rtm/{userId}/notes':
    get:
      operationId: api_rtm_userIdnotes_get_collection
      tags:
        - RTM
      responses:
        200:
          description: 'GetUserNotesWithNames collection'
          content:
            application/ld+json:
              schema:
                type: object
                properties:
                  'hydra:member': { type: array, items: { $ref: '#/components/schemas/GetUserNotesWithNames.jsonld' } }
                  'hydra:totalItems': { type: integer, minimum: 0 }
                  'hydra:view': { type: object, properties: { '@id': { type: string, format: iri-reference }, '@type': { type: string }, 'hydra:first': { type: string, format: iri-reference }, 'hydra:last': { type: string, format: iri-reference }, 'hydra:previous': { type: string, format: iri-reference }, 'hydra:next': { type: string, format: iri-reference } }, example: { '@id': string, type: string, 'hydra:first': string, 'hydra:last': string, 'hydra:previous': string, 'hydra:next': string } }
                  'hydra:search': { type: object, properties: { '@type': { type: string }, 'hydra:template': { type: string }, 'hydra:variableRepresentation': { type: string }, 'hydra:mapping': { type: array, items: { type: object, properties: { '@type': { type: string }, variable: { type: string }, property: { type: string, nullable: true }, required: { type: boolean } } } } } }
                required:
                  - 'hydra:member'
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/GetUserNotesWithNames'
            text/html:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/GetUserNotesWithNames'
        401:
          description: Unauthorized
          content:
            application/json:
              schema:
                type: object
                properties:
                  code: { readOnly: true, type: string, nullable: false }
                  message: { readOnly: true, type: string, nullable: false }
                required:
                  - '@context'
                  - '@type'
                  - 'hydra:title'
                  - 'hydra:description'
            application/ld+json:
              schema:
                type: object
                properties:
                  '@context': { readOnly: true, type: string, nullable: false }
                  '@type': { readOnly: true, type: string, nullable: false }
                  'hydra:title': { readOnly: true, type: string, nullable: false }
                  'hydra:description': { readOnly: true, type: string, nullable: false }
                required:
                  - '@context'
                  - '@type'
                  - 'hydra:title'
                  - 'hydra:description'
      summary: 'Get all notes for a user.'
      description: 'Get all notes for a user.'
      parameters:
        -
          name: userId
          in: path
          description: 'User identifier'
          required: true
          deprecated: false
          allowEmptyValue: false
          schema:
            type: string
          style: simple
          explode: false
          allowReserved: false
      deprecated: false
    parameters: []

Relevant types generated by the lib:

/**
   * Get all notes for a user. 
   * @description Get all notes for a user.
   */
  api_rtm_userIdnotes_get_collection: {
    parameters: {
      path: {
        /** @description User identifier */
        userId: string;
      };
    };
    responses: {
      /** @description GetUserNotesWithNames collection */
      200: {
        content: {
          "application/ld+json": {
            "hydra:member": (components["schemas"]["GetUserNotesWithNames.jsonld"])[];
            "hydra:totalItems"?: number;
            /**
             * @example {
             *   "@id": "string",
             *   "type": "string",
             *   "hydra:first": "string",
             *   "hydra:last": "string",
             *   "hydra:previous": "string",
             *   "hydra:next": "string"
             * }
             */
            "hydra:view"?: {
              /** Format: iri-reference */
              "@id"?: string;
              "@type"?: string;
              /** Format: iri-reference */
              "hydra:first"?: string;
              /** Format: iri-reference */
              "hydra:last"?: string;
              /** Format: iri-reference */
              "hydra:previous"?: string;
              /** Format: iri-reference */
              "hydra:next"?: string;
            };
            "hydra:search"?: {
              "@type"?: string;
              "hydra:template"?: string;
              "hydra:variableRepresentation"?: string;
              "hydra:mapping"?: ({
                  "@type"?: string;
                  variable?: string;
                  property?: string | null;
                  required?: boolean;
                })[];
            };
          };
          "application/json": (components["schemas"]["GetUserNotesWithNames"])[];
          "text/html": (components["schemas"]["GetUserNotesWithNames"])[];
        };
      };
      /** @description Unauthorized */
      401: {
        content: {
          "application/json": {
            code?: string;
            message?: string;
          };
          "application/ld+json": {
            "@context": string;
            "@type": string;
            "hydra:title": string;
            "hydra:description": string;
          };
        };
      };
    };
  };

The error message:

Element implicitly has an 'any' type because expression of type '"hydra:member"' can't be used to index type 'NonNullable<FilterKeys<{ "application/ld+json": { "hydra:member": { "@id"?: string | undefined; "@type"?: string | undefined; id?: string | undefined; note?: string | undefined; isCall?: boolean | undefined; timeSpent?: number | undefined; createdAt?: string | undefined; userFirstName?: string | undefined; userLastN...'.
  Property 'hydra:member' does not exist on type 'NonNullable<FilterKeys<{ "application/ld+json": { "hydra:member": { "@id"?: string | undefined; "@type"?: string | undefined; id?: string | undefined; note?: string | undefined; isCall?: boolean | undefined; timeSpent?: number | undefined; createdAt?: string | undefined; userFirstName?: string | undefined; userLastN...'.ts(7053)

At this point I'm just about convinced I'm either doing something wrong, or multiple Accept types simply aren't supported.

Any clues as to what is happening here? And what could be done to fix it (either in the lib or in my use of it)?

Thanks :)

EmilePerron commented 1 year ago

Well, following my thoughts from the last comment, I updated the OpenAPI schema to document a single content type - application/ld+json. Removing the others, leaving just this one, and then regenerating the types using this library, everything started working correctly.

This seems to indicate that multiple content types aren't supported by this library. (the library is still very useful and well crafted - I merely add this here to help anyone else who might stumble onto this issue.)

Now, would supporting multiple content types be of interest in the context of the library? If so, perhaps I could eventually make a PR to try and make that work, if you think it can be done and would be useful.

drwpow commented 1 year ago

Thanks for providing more context—indeed it seems that divergent media type responses are too tricky for the TS inference to follow.

Just out of curiosity, how would you propose the API worked for inferring which content-type should be used? e.g. if you referenced data, what check in TS would you make to know for sure that it was the "application/json" typing and not "application/ld+json"? Or vice-versa?

Or, in a general sense, how do consumers of your API know which response will be served?

The major blocker to adding support for schemas with competing media types is mostly figuring out the setup itself. And whether or not it works for all people who want to use this.

EmilePerron commented 1 year ago

In all of my cases, my APIs are built with API Platform for Symfony, which uses the standard headers for content type negociation.

The Accept HTTP header defines which response type will be received, and the Content-Type defines the which content type is being sent in the request.

So my initial thoughts were that those headers should be defined on your client or on your individual requests to allow for proper type inference. That would follow HTTP standards and would be what most most people expect, I believe.

However, I also realized that in most use cases, you would use this library to build an API client in your frontend, and you very likely only care about one of the supported content types (because unless your goal is to demo the fact that your API supports multiple content types, there is no point in switching types: working with various types in one application only makes development more complicated with no upside).

Do perhaps a single « default content type » config that is used for type inference when multiple types are available could do the trick as well.

It’d be a bit further from standards, but would likely be easier to implement and be good enough for the majority of use cases. Just my thoughts 🤷‍♂️

drwpow commented 1 year ago

Yeah that makes total sense—being able to pass it in as an option, at least (of course it’d have to be a type option, not a runtime option, e.g. createClient<paths, "application/ld+json">()

The Accept header is also a good idea. Though "Accept": "*" is probably what most people want, you probably would not manually set this header unless there were multiple possible response formats and you needed to declare which you want.

I’ll look into both and see which is less annoying to use, and more doable with TypeScript. As a project goal of using as few generics as possible, the latter seems more preferable at first glance.

NikolaStojicic commented 1 year ago

Hi! 👋

I have made an generic extension createClient<paths, 'application/ld+json'>();

I am definitely not sure if this helps or is it correct and I haven't tested it enough yet, but if anyone needs it, use this patch-package diff.

Today I used patch-package to patch openapi-fetch@0.7.8 for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/openapi-fetch/dist/cjs/index.d.cts b/node_modules/openapi-fetch/dist/cjs/index.d.cts
index 5403066..55e7c71 100644
--- a/node_modules/openapi-fetch/dist/cjs/index.d.cts
+++ b/node_modules/openapi-fetch/dist/cjs/index.d.cts
@@ -1,4 +1,4 @@
-import type { ErrorResponse, SuccessResponse, FilterKeys, MediaType, PathsWithMethod, ResponseObjectMap, OperationRequestBodyContent } from "openapi-typescript-helpers";
+import type { ErrorResponse, SuccessResponse, FilterKeys, PathsWithMethod, ResponseObjectMap, OperationRequestBodyContent } from "openapi-typescript-helpers";
 /** options for each client instance */
 interface ClientOptions extends Omit<RequestInit, "headers"> {
     /** set the common root URL for all API requests */
@@ -35,7 +35,7 @@ export type RequestBodyOption<T> = OperationRequestBodyContent<T> extends never
     body: OperationRequestBodyContent<T>;
 };
 export type FetchOptions<T> = RequestOptions<T> & Omit<RequestInit, "body">;
-export type FetchResponse<T> = {
+export type FetchResponse<T, MediaType> = {
     data: FilterKeys<SuccessResponse<ResponseObjectMap<T>>, MediaType>;
     error?: never;
     response: Response;
@@ -49,23 +49,23 @@ export type RequestOptions<T> = ParamsOption<T> & RequestBodyOption<T> & {
     bodySerializer?: BodySerializer<T>;
     parseAs?: ParseAs;
 };
-export default function createClient<Paths extends {}>(clientOptions?: ClientOptions): {
+export default function createClient<Paths extends {}, MediaType>(clientOptions?: ClientOptions): {
     /** Call a GET endpoint */
-    GET<P extends PathsWithMethod<Paths, "get">>(url: P, init: FetchOptions<FilterKeys<Paths[P], "get">>): Promise<FetchResponse<"get" extends infer T ? T extends "get" ? T extends keyof Paths[P] ? Paths[P][T] : unknown : never : never>>;
+    GET<P extends PathsWithMethod<Paths, "get">>(url: P, init: FetchOptions<FilterKeys<Paths[P], "get">>): Promise<FetchResponse<"get" extends infer T ? T extends "get" ? T extends keyof Paths[P] ? Paths[P][T] : unknown : never : never, MediaType>>;
     /** Call a PUT endpoint */
-    PUT<P_1 extends PathsWithMethod<Paths, "put">>(url: P_1, init: FetchOptions<FilterKeys<Paths[P_1], "put">>): Promise<FetchResponse<"put" extends infer T_1 ? T_1 extends "put" ? T_1 extends keyof Paths[P_1] ? Paths[P_1][T_1] : unknown : never : never>>;
+    PUT<P_1 extends PathsWithMethod<Paths, "put">>(url: P_1, init: FetchOptions<FilterKeys<Paths[P_1], "put">>): Promise<FetchResponse<"put" extends infer T_1 ? T_1 extends "put" ? T_1 extends keyof Paths[P_1] ? Paths[P_1][T_1] : unknown : never : never, MediaType>>;
     /** Call a POST endpoint */
-    POST<P_2 extends PathsWithMethod<Paths, "post">>(url: P_2, init: FetchOptions<FilterKeys<Paths[P_2], "post">>): Promise<FetchResponse<"post" extends infer T_2 ? T_2 extends "post" ? T_2 extends keyof Paths[P_2] ? Paths[P_2][T_2] : unknown : never : never>>;
+    POST<P_2 extends PathsWithMethod<Paths, "post">>(url: P_2, init: FetchOptions<FilterKeys<Paths[P_2], "post">>): Promise<FetchResponse<"post" extends infer T_2 ? T_2 extends "post" ? T_2 extends keyof Paths[P_2] ? Paths[P_2][T_2] : unknown : never : never, MediaType>>;
     /** Call a DELETE endpoint */
-    DELETE<P_3 extends PathsWithMethod<Paths, "delete">>(url: P_3, init: FetchOptions<FilterKeys<Paths[P_3], "delete">>): Promise<FetchResponse<"delete" extends infer T_3 ? T_3 extends "delete" ? T_3 extends keyof Paths[P_3] ? Paths[P_3][T_3] : unknown : never : never>>;
+    DELETE<P_3 extends PathsWithMethod<Paths, "delete">>(url: P_3, init: FetchOptions<FilterKeys<Paths[P_3], "delete">>): Promise<FetchResponse<"delete" extends infer T_3 ? T_3 extends "delete" ? T_3 extends keyof Paths[P_3] ? Paths[P_3][T_3] : unknown : never : never, MediaType>>;
     /** Call a OPTIONS endpoint */
-    OPTIONS<P_4 extends PathsWithMethod<Paths, "options">>(url: P_4, init: FetchOptions<FilterKeys<Paths[P_4], "options">>): Promise<FetchResponse<"options" extends infer T_4 ? T_4 extends "options" ? T_4 extends keyof Paths[P_4] ? Paths[P_4][T_4] : unknown : never : never>>;
+    OPTIONS<P_4 extends PathsWithMethod<Paths, "options">>(url: P_4, init: FetchOptions<FilterKeys<Paths[P_4], "options">>): Promise<FetchResponse<"options" extends infer T_4 ? T_4 extends "options" ? T_4 extends keyof Paths[P_4] ? Paths[P_4][T_4] : unknown : never : never, MediaType>>;
     /** Call a HEAD endpoint */
-    HEAD<P_5 extends PathsWithMethod<Paths, "head">>(url: P_5, init: FetchOptions<FilterKeys<Paths[P_5], "head">>): Promise<FetchResponse<"head" extends infer T_5 ? T_5 extends "head" ? T_5 extends keyof Paths[P_5] ? Paths[P_5][T_5] : unknown : never : never>>;
+    HEAD<P_5 extends PathsWithMethod<Paths, "head">>(url: P_5, init: FetchOptions<FilterKeys<Paths[P_5], "head">>): Promise<FetchResponse<"head" extends infer T_5 ? T_5 extends "head" ? T_5 extends keyof Paths[P_5] ? Paths[P_5][T_5] : unknown : never : never, MediaType>>;
     /** Call a PATCH endpoint */
-    PATCH<P_6 extends PathsWithMethod<Paths, "patch">>(url: P_6, init: FetchOptions<FilterKeys<Paths[P_6], "patch">>): Promise<FetchResponse<"patch" extends infer T_6 ? T_6 extends "patch" ? T_6 extends keyof Paths[P_6] ? Paths[P_6][T_6] : unknown : never : never>>;
+    PATCH<P_6 extends PathsWithMethod<Paths, "patch">>(url: P_6, init: FetchOptions<FilterKeys<Paths[P_6], "patch">>): Promise<FetchResponse<"patch" extends infer T_6 ? T_6 extends "patch" ? T_6 extends keyof Paths[P_6] ? Paths[P_6][T_6] : unknown : never : never, MediaType>>;
     /** Call a TRACE endpoint */
-    TRACE<P_7 extends PathsWithMethod<Paths, "trace">>(url: P_7, init: FetchOptions<FilterKeys<Paths[P_7], "trace">>): Promise<FetchResponse<"trace" extends infer T_7 ? T_7 extends "trace" ? T_7 extends keyof Paths[P_7] ? Paths[P_7][T_7] : unknown : never : never>>;
+    TRACE<P_7 extends PathsWithMethod<Paths, "trace">>(url: P_7, init: FetchOptions<FilterKeys<Paths[P_7], "trace">>): Promise<FetchResponse<"trace" extends infer T_7 ? T_7 extends "trace" ? T_7 extends keyof Paths[P_7] ? Paths[P_7][T_7] : unknown : never : never, MediaType>>;
 };
 /** serialize query params to string */
 export declare function defaultQuerySerializer<T = unknown>(q: T): string;

This issue body was partially generated by patch-package.

simon511000 commented 9 months ago

Hi! Sorry for digging up this issue, but will this issue be fixed someday?

Currently, openapi-fetch is unusable with Api Platform, which generates both application/json and application/ld+json.

@NikolaStojicic's patch works well for most requests to force the use of application/ld+json, but it doesn't work for requests that are only in application/json, such as the login request.

NikolaStojicic commented 9 months ago

Hi!

Sorry for digging up this issue, but will this issue be fixed someday?

Currently, openapi-fetch is unusable with Api Platform, which generates both application/json and application/ld+json.

@NikolaStojicic's patch works well for most requests to force the use of application/ld+json, but it doesn't work for requests that are only in application/json, such as the login request.

I used 2 wrappers, 1 forced into ld+json other just into json using my patch.

Not very clean solution, but works good enough if you have small number of non ld requests.

armandabric commented 7 months ago

Hey there! We are having the same issue: our Open API schema contain multiple content types.

Is it plan to be fixed in the next version (v7)? What can we do to help?

drwpow commented 7 months ago

Is it plan to be fixed in the next version (v7)? What can we do to help?

PRs are welcome for this! Please see CONTRIBUTING.md to get started. The existing PRs should be sufficient to test types.