search

JorrinKievit / tmdb-js

A typesafe API wrapper for the TheMovieDatabase API for Node and the Web
9 stars 1 forks source link

append_to_response is not typesafe #2

Closed paolodelfino closed 1 year ago

paolodelfino commented 1 year ago

It would be kind of cool if append_to_response would be typesafe. I came out with the "solution" below (a cast), that is not actually a solution because for example, MoviesGetVideosResponse and MoviesGetImagesResponse are types that have id as one of the properties but, when you append videos or images, you don't get the id and also, MoviesGetImagesResponse doesn't have logos property.

api.v3.movies.getDetails(12, {append_to_response: "videos,images"}).then(movie => {
    movie.videos; // <- Property 'videos' does not exist on type 'MoviesGetDetailsResponse'.
    (movie as MoviesGetDetailsResponse & {videos: MoviesGetVideosResponse}).videos

    movie.images; // <- Property 'images' does not exist on type 'MoviesGetDetailsResponse'.
    (movie as MoviesGetDetailsResponse & {images: MoviesGetImagesResponse}).images
})

A solution would at least be to provide an interface specifically for append_to_response responses that you can use to cast, in the case, movie variable. But at the end it's really a nice api, good job!

JorrinKievit commented 1 year ago

Hey, I'm on vacation till wednesday so I'll take a look then!

JorrinKievit commented 1 year ago

Hmm this was an interesting one to solve to get append_to_responses type safe.

I had it partially working that based on the append_to_response field, other types get added. But that meant that I had to get append_to_response typesafe instead of type "string". So this results into something like this: image

This however is not scalable, and will also make the typescript server immensely slow since you get crazy big types because there are a lot of possibilities: image

So the solution would be to make append_to_response an array. This way its easier to pinpoint the needed types. I have this working now.

Still need to cleanup, and create a pipeline for automatic deployments 😅 . So expect an update this weekend

https://github.com/JorrinKievit/tmdb-js/assets/43169049/ee3b42f0-b2a7-4aeb-b643-837f608970de

JorrinKievit commented 1 year ago

Was easier than expected. Should be working in 1.1.0 :D