Typescript API Client Bugs and Improvements

[Baskezada 4]

Hello! Recently, I've been testing the Emby API in a TypeScript project and I've encountered some issues.

1. In postItemsByIdImagesByTypeByIndexUrl and postNotificationsAdmin uses a parameter called "url", which conflicts with the "url" library used for formatting strings. I had to manually change the name of this parameter.
2. In getPlaylistsByIdAddtoplaylistinfo, the return value is declared as Promise<ModelObject>, but ModelObject is not declared anywhere.
3. The interface ProviderIdDictionary has an extends null<String, string> which doesn't make sense.
4. And lastly, and most importantly, every single response interface has its properties in lowercase, but the Emby API provides these responses with uppercase properties, which makes the API practically unusable since all the types are incorrect.

Another problem I encountered is that there are functions with too many parameters. If I only want to use a few, it's practically impossible. For example, the following function:

public getUsersByUseridItemsResume(userId: string, artistType?: string, maxOfficialRating?: string, hasThemeSong?: boolean, hasThemeVideo?: boolean, hasSubtitles?: boolean, hasSpecialFeature?: boolean, hasTrailer?: boolean, adjacentTo?: string, minIndexNumber?: number, minStartDate?: string, maxStartDate?: string, minEndDate?: string, maxEndDate?: string, minPlayers?: number, maxPlayers?: number, parentIndexNumber?: number, hasParentalRating?: boolean, isHD?: boolean, isUnaired?: boolean, minCommunityRating?: number, minCriticRating?: number, airedDuringSeason?: number, minPremiereDate?: string, minDateLastSaved?: string, minDateLastSavedForUser?: string, maxPremiereDate?: string, hasOverview?: boolean, hasImdbId?: boolean, hasTmdbId?: boolean, hasTvdbId?: boolean, excludeItemIds?: string, startIndex?: number, limit?: number, recursive?: boolean, searchTerm?: string, sortOrder?: string, parentId?: string, fields?: string, excludeItemTypes?: string, includeItemTypes?: string, anyProviderIdEquals?: string, filters?: string, isFavorite?: boolean, isMovie?: boolean, isSeries?: boolean, isFolder?: boolean, isNews?: boolean, isKids?: boolean, isSports?: boolean, isNew?: boolean, isPremiere?: boolean, isNewOrPremiere?: boolean, isRepeat?: boolean, projectToMedia?: boolean, mediaTypes?: string, imageTypes?: string, sortBy?: string, isPlayed?: boolean, genres?: string, officialRatings?: string, tags?: string, excludeTags?: string, years?: string, enableImages?: boolean, enableUserData?: boolean, imageTypeLimit?: number, enableImageTypes?: string, person?: string, personIds?: string, personTypes?: string, studios?: string, studioIds?: string, artists?: string, artistIds?: string, albums?: string, ids?: string, videoTypes?: string, containers?: string, audioCodecs?: string, audioLayouts?: string, videoCodecs?: string, extendedVideoTypes?: string, subtitleCodecs?: string, path?: string, minOfficialRating?: string, isLocked?: boolean, isPlaceHolder?: boolean, hasOfficialRating?: boolean, groupItemsIntoCollections?: boolean, is3D?: boolean, seriesStatus?: string, nameStartsWithOrGreater?: string, artistStartsWithOrGreater?: string, albumArtistStartsWithOrGreater?: string, nameStartsWith?: string, nameLessThan?: string, options?: any) 

I had to transform it to request a single object as a parameter so I could use it:

 public getUsersByUseridItemsResume(params : {userId: string, artistType?: string, maxOfficialRating?: string, hasThemeSong?: boolean, hasThemeVideo?: boolean, hasSubtitles?: boolean, hasSpecialFeature?: boolean, hasTrailer?: boolean, adjacentTo?: string, minIndexNumber?: number, minStartDate?: string, maxStartDate?: string, minEndDate?: string, maxEndDate?: string, minPlayers?: number, maxPlayers?: number, parentIndexNumber?: number, hasParentalRating?: boolean, isHD?: boolean, isUnaired?: boolean, minCommunityRating?: number, minCriticRating?: number, airedDuringSeason?: number, minPremiereDate?: string, minDateLastSaved?: string, minDateLastSavedForUser?: string, maxPremiereDate?: string, hasOverview?: boolean, hasImdbId?: boolean, hasTmdbId?: boolean, hasTvdbId?: boolean, excludeItemIds?: string, startIndex?: number, limit?: number, recursive?: boolean, searchTerm?: string, sortOrder?: string, parentId?: string, fields?: string, excludeItemTypes?: string, includeItemTypes?: string, anyProviderIdEquals?: string, filters?: string, isFavorite?: boolean, isMovie?: boolean, isSeries?: boolean, isFolder?: boolean, isNews?: boolean, isKids?: boolean, isSports?: boolean, isNew?: boolean, isPremiere?: boolean, isNewOrPremiere?: boolean, isRepeat?: boolean, projectToMedia?: boolean, mediaTypes?: string, imageTypes?: string, sortBy?: string, isPlayed?: boolean, genres?: string, officialRatings?: string, tags?: string, excludeTags?: string, years?: string, enableImages?: boolean, enableUserData?: boolean, imageTypeLimit?: number, enableImageTypes?: string, person?: string, personIds?: string, personTypes?: string, studios?: string, studioIds?: string, artists?: string, artistIds?: string, albums?: string, ids?: string, videoTypes?: string, containers?: string, audioCodecs?: string, audioLayouts?: string, videoCodecs?: string, extendedVideoTypes?: string, subtitleCodecs?: string, path?: string, minOfficialRating?: string, isLocked?: boolean, isPlaceHolder?: boolean, hasOfficialRating?: boolean, groupItemsIntoCollections?: boolean, is3D?: boolean, seriesStatus?: string, nameStartsWithOrGreater?: string, artistStartsWithOrGreater?: string, albumArtistStartsWithOrGreater?: string, nameStartsWith?: string, nameLessThan?: string, options?: any}) {

        const {userId, artistType, maxOfficialRating, hasThemeSong, hasThemeVideo, hasSubtitles, hasSpecialFeature, hasTrailer, adjacentTo, minIndexNumber, minStartDate, maxStartDate, minEndDate, maxEndDate, minPlayers, maxPlayers, parentIndexNumber, hasParentalRating, isHD, isUnaired, minCommunityRating, minCriticRating, airedDuringSeason, minPremiereDate, minDateLastSaved, minDateLastSavedForUser, maxPremiereDate, hasOverview, hasImdbId, hasTmdbId, hasTvdbId, excludeItemIds, startIndex, limit, recursive, searchTerm, sortOrder, parentId, fields, excludeItemTypes, includeItemTypes, anyProviderIdEquals, filters, isFavorite, isMovie, isSeries, isFolder, isNews, isKids, isSports, isNew, isPremiere, isNewOrPremiere, isRepeat, projectToMedia, mediaTypes, imageTypes, sortBy, isPlayed, genres, officialRatings, tags, excludeTags, years, enableImages, enableUserData, imageTypeLimit, enableImageTypes, person, personIds, personTypes, studios, studioIds, artists, artistIds, albums, ids, videoTypes, containers, audioCodecs, audioLayouts, videoCodecs, extendedVideoTypes, subtitleCodecs, path, minOfficialRating, isLocked, isPlaceHolder, hasOfficialRating, groupItemsIntoCollections, is3D, seriesStatus, nameStartsWithOrGreater, artistStartsWithOrGreater, albumArtistStartsWithOrGreater, nameStartsWith, nameLessThan, options} = params
       
    }

It would be ideal if all functions were of this style.

I'm really excited about being able to create a project for the community, but these errors are holding me back (especially the last one, as the first ones are easy to fix)

Edited June 6, 2024 by Baskezada

[softworkz 5066]

7 hours ago, Baskezada said:

Another problem I encountered is that there are functions with too many parameters. If I only want to use a few, it's practically impossible

Why would that be impossible?

You just supply 'null' for all the parameters you don't want to specify. You can do that for all the parameters where the type is suffixed by a question mark '?'. This indicates that a parameter is optional. Those without a '?' MUST be supplied (i.e. non-null).

[Baskezada 4]

1 hour ago, softworkz said:

You just supply 'null' for all the parameters you don't want to specify. You can do that for all the parameters where the type is suffixed by a question mark '?'. This indicates that a parameter is optional. Those without a '?' MUST be supplied (i.e. non-null).

Yes, that can be done, but it adds a lot of difficulty. For example, if I wanted to call that endpoint with only mediaTypes: 'Video', I would have to do this:

getUsersByUseridItemsResume('userId', null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,null, null, null, null, null, null, null, null, null, null,null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, 'Video')

mediaTypes being the 57th parameter (I had to count it), when it could be done like this:

getUsersByUseridItemsResume({userId: 'userId', mediaTypes: 'Video'})

I don't know if there is another way to pass the function arguments without doing that.

 

[softworkz 5066]

22 minutes ago, Baskezada said:

mediaTypes being the 57th parameter (I had to count it)

Well, one of the advantages of using TypeScript is that you can see appropriate hints (and error indications while typing) in our development environment about the function definitions, and when this is working properly, you wouldn't need to count.

24 minutes ago, Baskezada said:

I don't know if there is another way to pass the function arguments without doing that.

You could call the prototype with the parameters as an array, but then you would really need to know the indexes.
Probably it would also possible to develop a wrapper function which examines the function prototype, extracting the parameter names and build an array from that. Finally invoke the function with this array.

But such approach would sacrifice both, type safety and parameter infos in the IDE.

 

I understand that functions having so many parameters are ugly and I wouldn't define a native api like this, but these functions are auto-generated from the REST API definitions for 9 different programming languages and it's beyond our capacity to make individual changes to each API definitions.
We're using the C# API client (Emby.ApiClient) ourselves in some new client apps and I haven't spent a millisecond of thought about it while pressing CTRL+D on a ', null' selection as many times as needed, until VS indicated that I've approached at the desired parameter.

[softworkz 5066]

9 hours ago, Baskezada said:

In getPlaylistsByIdAddtoplaylistinfo, the return value is declared as Promise<ModelObject>, but ModelObject is not declared anywhere.

I have fixed this, it was due to a wrong definition in our code.

 

9 hours ago, Baskezada said:

The interface ProviderIdDictionary has an extends null<String, string> which doesn't make sense.

Submitted a bug report in the generators repo: https://github.com/swagger-api/swagger-codegen-generators/issues/1297

[softworkz 5066]

10 hours ago, Baskezada said:

And lastly, and most importantly, every single response interface has its properties in lowercase, but the Emby API provides these responses with uppercase properties, which makes the API practically unusable since all the types are incorrect.

Fixed (for future versions)

10 hours ago, Baskezada said:

In postItemsByIdImagesByTypeByIndexUrl and postNotificationsAdmin uses a parameter called "url", which conflicts with the "url" library used for formatting strings. I had to manually change the name of this parameter.

This is getting accidentally fixed due to the above change to use the original casing and the original casing is 'Url'.

[Baskezada 4]

8 hours ago, softworkz said:

I understand that functions having so many parameters are ugly and I wouldn't define a native api like this, but these functions are auto-generated from the REST API definitions for 9 different programming languages and it's beyond our capacity to make individual changes to each API definitions.

Oh, I understand, I'll see if I take that alternative or try to wrap the function or even generate a script to transform it into something more convenient for me. If I find something, I'll share it.

By the way, thanks for the changes made, I'll be looking forward to that new version 

[softworkz 5066]

Can you try this? It's the new output:

Publish.zip

[softworkz 5066]

28 minutes ago, Baskezada said:

I'll see if I take that alternative or try to wrap the function or even generate a script to transform it into something more convenient for me. If I find something, I'll share it.

One possible way would be something like this (untested, just for the idea):

function getParamNames(func: AnyFunction): string[] {
    const fnStr = func.toString().replace(/[/][/].*$/mg, ''); // comments
    const result = fnStr.slice(fnStr.indexOf('(') + 1, fnStr.indexOf(')')).match(/([^\s,]+)/g);
    return result === null ? [] : result;
}

type AnyFunction = (...args: any[]) => any;

function callWithNamedParams<T extends AnyFunction>(fn: T, params: Record<string, any>): ReturnType<T> {
    const paramNames = getParamNames(fn);
    const args = paramNames.map(name => params[name]);
    return fn(...args);
}

This sacrifices type safety but it's safe against changes to the parameter list (on API  updates).

But it's also kind of a false safety, because when a parameter gets removed or a new mandatory parameter gets added, it still fails.

Updates are also an issue when you make any modifications to the generated code. I would rather stick to create just some wrappers for functions you need to use frequently.
Unfortunately this still doesn't save you from issues due to signature changes on API updates.

It's might be a good idea to create a unit-test in your code for each API function you are using and check its signature for changes (by comparing 'func.toString()' ). This is probably the only way to be 100% safe.

Edited June 7, 2024 by softworkz

[Baskezada 4]

1 hour ago, softworkz said:

Can you try this? It's the new output:

It's working perfectly!  I will try the wrapper later, but for now, I can work like this. Thanks!!

[softworkz 5066]

Great! Thanks for trying.

[adminExitium 355]

18 hours ago, softworkz said:

but these functions are auto-generated from the REST API definitions for 9 different programming languages and it's beyond our capacity to make individual changes to each API definitions.

Not sure if you have the budget for it, but you may want to check out Stainless or SpeakEasy for SDKs based on OpenAPI specs:

https://www.stainlessapi.com/

https://www.speakeasyapi.dev/

Stainless, in particular, is used by Cloudflare and their SDKs are very good across languages.

 

[softworkz 5066]

5 hours ago, adminExitium said:

Not sure if you have the budget for it, but you may want to check out Stainless or SpeakEasy for SDKs based on OpenAPI specs:

There are many offerings for generating API clients, not all of them even existed when we started the SDK generation two years ago.
Also, for these new ones, you can't take for granted that they would do any better than what we are using now, because swagger-codegen is being developed since many years - opposed to those you are suggesting. Speakeasy itself for example is just two years old and Stainless just 1 year (GitHub repo age).
I doubt that these are any more mature than swagger-codegen, especially with regards to our APIs, which are quite complex as they have grown over time. Providing better APIs would have to start by changing our API definitions (which is not going to happen) and it cannot be remedied by changing the client generation libraries.

Finally, you also need to consider that this is the first feedback topic about the TypeScript client lib after it existed for two years, so the audience is rather small.

But for that small audience, we now have two doc websites (separate for beta and stable) and eight API clients  (again separate for beta and stable) which are all and always up-to-date, which is quite an achievements over what we've had before (outdated dev Wiki and a few years-old client libs).