createClient returning a single function

[SandroMaglione]

Description

The current return type for createClient returns different functions for all possible methods (GET, POST, etc.):

https://github.com/drwpow/openapi-typescript/blob/ea65d72f602c013c8fba45836d68e48c37cc7480/packages/openapi-fetch/src/index.ts#L188-L195

The type exported is the following:

export default function createClient<Paths extends {}>(clientOptions?: ClientOptions): {
    /** Call a GET endpoint */
    GET<P extends PathsWithMethod<Paths, "get">>(url: P, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P], "get">>> extends never ? [(FetchOptions<FilterKeys<Paths[P], "get">> | undefined)?] : [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>>;
    /** Call a PUT endpoint */
    PUT<P_1 extends PathsWithMethod<Paths, "put">>(url: P_1, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_1], "put">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_1], "put">> | undefined)?] : [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>>;
    /** Call a POST endpoint */
    POST<P_2 extends PathsWithMethod<Paths, "post">>(url: P_2, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_2], "post">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_2], "post">> | undefined)?] : [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>>;
    /** Call a DELETE endpoint */
    DELETE<P_3 extends PathsWithMethod<Paths, "delete">>(url: P_3, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_3], "delete">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_3], "delete">> | undefined)?] : [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>>;
    /** Call a OPTIONS endpoint */
    OPTIONS<P_4 extends PathsWithMethod<Paths, "options">>(url: P_4, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_4], "options">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_4], "options">> | undefined)?] : [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>>;
    /** Call a HEAD endpoint */
    HEAD<P_5 extends PathsWithMethod<Paths, "head">>(url: P_5, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_5], "head">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_5], "head">> | undefined)?] : [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>>;
    /** Call a PATCH endpoint */
    PATCH<P_6 extends PathsWithMethod<Paths, "patch">>(url: P_6, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_6], "patch">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_6], "patch">> | undefined)?] : [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>>;
    /** Call a TRACE endpoint */
    TRACE<P_7 extends PathsWithMethod<Paths, "trace">>(url: P_7, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P_7], "trace">>> extends never ? [(FetchOptions<FilterKeys<Paths[P_7], "trace">> | undefined)?] : [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>>;
};

These methods have all the same definition. The only difference is the HttpMethod applied:

https://github.com/drwpow/openapi-typescript/blob/ea65d72f602c013c8fba45836d68e48c37cc7480/packages/openapi-typescript-helpers/index.d.ts#L5-L13

Proposal

Is there a reason for not combining all these methods in one? Something like the following:

export default function createClient<Paths extends {}>(clientOptions?: ClientOptions): <Method extends HttpMethod, P extends PathsWithMethod<Paths, Method>>(method: Method, url: P, ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P], Method>>> extends never ? [(FetchOptions<FilterKeys<Paths[P], Method>> | undefined)?] : [FetchOptions<FilterKeys<Paths[P], Method>>]): Promise<FetchResponse<Method extends infer T ? T extends Method ? T extends keyof Paths[P] ? Paths[P][T] : unknown : never : never>>;

A single function (instead of GET, POST, etc.) that accepts a method parameter of type HttpMethod.

openapiClient("get", "/carts/{cartId}", {
  params: {
    path: { cartId: "" },
    query: { sortKey: "SOME_KEY" },
  },
});

openapiClient("post", "/orders", {
  params: { query: {} },
});

openapiClient("delete", "/carts/{itemId}", {
  params: { path: { itemId: "" } },
});

Wouldn’t this be easier to implement and use? Or is there something that I am not aware of regarding the current implementation?

Checklist

• I’m willing to open a PR for this (see CONTRIBUTING.md)

[drwpow]

There’s not a fundamental difference in the proposal vs. the current form. If anything, the proposal is more verbose because in the current version you could just call GET(…) (if exploded) whereas in the proposal it must always be someFunction("get", "…").

At the end of the day it just comes down to an opinion, and I personally find the former to be simpler to remember and consume, that’s all. Originally, the separate functions were also easier for TypeScript to introspect, because functions inferring their signatures based on what the previous parameter was is just more difficult. Not impossible, just more work. Today that’s less of a concern as the types have improved. But it’s also worth pointing out the difficulty of certain type structures while the source file is composed in TypeScript, which means the code has to be written in a way that the correct definitions can be inferred (as opposed to a separate TypeScript definition file which is allowed to diverge from the runtime code, something I think we’ll change in the not-so-distant future).

As far as the proposal goes I’m not open to revisiting the API format at this time. Or at least not without pointing out some clear advantage to switching beyond personal preference. I’ve converted this into a discussion for others to weigh in on if they’d like.

[nicu-chiciuc]

One benefit would be to ease of adding some kind of wrapper around the methods.
Currently I have a big api.ts file with things like:

  async getSomething(query: PaginationQuery) {
    const response = await client.GET('/api/something/', {
      params: {
        query,
      },
      headers: defaultHeaders(),
    })

    return afterResponse(response)
  },

The:

      headers: defaultHeaders(),

and

return afterResponse(response)

are present in all the functions in my file, and I would like to somehow create a custom client for this.

My first iteration (~30 mins) looks like this:

const getClient = () => {
  const client = openapiFetch<paths>({
    baseUrl: baseURLWithoutSuffix,

    headers: defaultHeaders(),
  })

  const GET = async <P>(...args: Parameters<typeof client.GET<P>>) => {
    const directResponse = await client.GET(...args)

    return afterResponse(directResponse)
  }

  const POST = async <P>(...args: Parameters<typeof client.POST>) => {
    const directResponse = await client.POST(...args)

    return afterResponse(directResponse)
  }

  const PUT = async <P>(...args: Parameters<typeof client.PUT>) => {
    const directResponse = await client.PUT(...args)

    return afterResponse(directResponse)
  }

  const DELETE = async <P>(...args: Parameters<typeof client.DELETE>) => {
    const directResponse = await client.DELETE(...args)

    return afterResponse(directResponse)
  }

  const PATCH = async <P>(...args: Parameters<typeof client.PATCH>) => {
    const directResponse = await client.PATCH(...args)

    return afterResponse(directResponse)
  }

  return {
    GET,
    POST,
    PUT,
    DELETE,
    PATCH,
  }
}

having just 1 generic function would ease such cases, I think.

The headers are being received from the localStorage, so I can't set them up as a value.

[nicu-chiciuc]

This seems to work pretty well:

    async core<TMethod extends HttpMethod, P extends PathsWithMethod<Paths, TMethod>>(
      method: TMethod,
      url: P,
      ...init: HasRequiredKeys<FetchOptions<FilterKeys<Paths[P], TMethod>>> extends never
        ? [FetchOptions<FilterKeys<Paths[P], TMethod>>?]
        : [FetchOptions<FilterKeys<Paths[P], TMethod>>]
    ) {
      return coreFetch<P, TMethod>(url, { ...init[0], method } as any)
    },
    /** Call a GET endpoint */
    async GET<P extends GetPaths>(
      url: P,
      ...init: HasRequiredKeys<GetFetchOptions<P>> extends never ? [GetFetchOptions<P>?] : [GetFetchOptions<P>]
    ) {
      return coreFetch<P, 'get'>(url, { ...init[0], method: 'GET' } as any)
    },

[nicu-chiciuc]

So I've played with the code some days ago and got to this solution: nicu-chiciuc@a703748

The direction I went to was simplifying as much as possible without changing the interface too much.
I've replaced the returned object containing multiple function with a single function

All the existing tests pass, except for the proxy (since, I'm not sure that's an expected usage for a function) and the test that checks the existence for properties on the returned objects.

Some things I figured I noticed:

• The

export type HttpMethod =
  | "get"
  | "put"
...

would be nice to be changed to uppercase so that it wouldn't be necessary to use method: method.toUpperCase()
I assume this was done since the generated typescript definitions use 'get' and 'post', but it would be nice to rely on the correct methods.

• The whole coreFetch function can be split in 3 parts:
  1: Creation of the Request object (or [baseUrl, RequestInit])
  2: fetch!
  3: Parsing the Response object

I have a feeling that making these separate distinctions would allow more flexibility.

The switch is pretty simple, I've done a replacement of client.GET( to client('get',.
I've tested it in our internal project, and there don't seem to be any issues.

I'm not really sure where I should go with these changes.

I think that it would be nice to separate the create Request and parse Request parts into separate functions, so the JS logic remains the same, and then have strong-typed wrappers around that functionality.
That would allow more experimentation into how the TS interface should look, while leaving the nitty-gritty details safe.

[drwpow]

Thanks for providing additional context on all that. I wouldn’t mind adding a .request('method', …options) function to createClient() that calls the appropriate method.

Again, part of the internal structure was a limitation of TypeScript and composing the runtime and types in the same file. In an upcoming change, I’d like to separate the type definitions from the runtime. Which will clean up a lot of code that was written just to appease TypeScript. But in that change, we could combine all the functions together internally for simplicity without a breaking change, and we could also add that additional .request() method, if that works.