Description
TL;DR
Move from positional arguments to object arguments for the sake of readability and extendability
Details
The API that is currently generated is quite hard read if one has to fill in a property that is not the first or second argument.
The current function signature is like this (on the example of namespaces)
export class CoreV1Api {
public async createNamespace (body: V1Namespace, pretty?: string, dryRun?: string, fieldManager?: string, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: V1Namespace; }>{}
public async listNamespace (pretty?: string, allowWatchBookmarks?: boolean, _continue?: string, fieldSelector?: string, labelSelector?: string, limit?: number, resourceVersion?: string, timeoutSeconds?: number, watch?: boolean, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: V1NamespaceList; }> {}
}This leads to calls like this api.createNamespace(namespace, undefined, true, undefined, {headers: {TRACE_REQUEST: "1"}}), which are hard to understand / reason about without looking into the client itself.
I would like to propose a different format:
// These types would be exported by the package and imported in this file
type Options = {
pretty?: string
dryRun?: string
options?: {headers: {[name: string]: string}}
}
type Result<T> = Promise<{ response: http.IncomingMessage; body: T; }>
type CreateOptions<T> = Options & {
body: T
fieldManager?: string,
}
type CreateFunction<T> = (opts: CreateOptions<T>) => CreateResult<T>
type ListOptions = Options & {
allowWatchBookmarks?: boolean
_continue?: string
fieldSelector?: string
labelSelector?: string
limit?: number
resourceVersion?: string
timeoutSeconds?: number
watch?: boolean
}
type ListFunction<T> = (opts: ListOptions) => Result<V1NamespaceList>
// The generated file begins here
export class CoreV1Api {
public async createNamespace (opts: CreateOptions<T>): CreateResult<T>{}
public async listNamespace (opts: ListOptions) : Result<V1NamespaceList> {}
// ...
}This would allow us to call the api in a readable way: api.createNamespace({body: namespace, dryRun: true, options: {headers: {TRACE_REQUEST: "1"}}). Someone without intimate knowledge of the client library can instantly understand this.
Another benefit of having an object as configuration is that it easier to extend / change without breaking clients and that a user can define their own abstractions. For example defining a function that adds a header is a very different experience after we implement this:
type ListFn<T> = (pretty?: string, allowWatchBookmarks?: boolean, _continue?: string, fieldSelector?: string, labelSelector?: string, limit?: number, resourceVersion?: string, timeoutSeconds?: number, watch?: boolean, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: V1NamespaceList;) => T
function addHeader<T>(fn: ListFn<T> | CreateFn<T> | ...): T {
if (isListFn(fn)) {
return (pretty?: string, allowWatchBookmarks?: boolean, _continue?: string, fieldSelector?: string, labelSelector?: string, limit?: number, resourceVersion?: string, timeoutSeconds?: number, watch?: boolean, options: {headers: {[name: string]: string}} = {headers: {}}): T => fn(pretty, allowWatchBookmarks, _continue, fieldSelector, labelSelector, limit, resourceVersion, timeoutSeconds, watch, {...options, headers: {...options.headers, myHeader: "is there"})
}
if (isCreateFn(fn)){ .... }
}
// Versus after this change
function addHeader<T extends Options, R>(fn: (opts: T) => R): (opts: T) => R {
return (opts) => fn({...opts, options: {...opts.options, headers: {...opts.options.headers, myHeader: "is there"}})
}