cloudinary / cloudinary_npm

Cloudinary NPM for node.js integration
629 stars 323 forks source link

Missing return types on v2.api.resources() #648

Closed rubenvar closed 10 months ago

rubenvar commented 10 months ago

Explain your use case

Hi, I am using the v2.api.resources() method in a TypeScript app, and this method returns a type of Promise<any>.

I found issue #315, which in turn linked to this PR, which added return types for all methods except for .resources(). I can't find the reason for this, sorry if it's already explained somewhere.

Describe the problem you’re trying to solve

I am trying to get a list of some of my latest uploaded resources, I thought api.resources() was the best method for this, but TS complains about the return value...

Do you have a proposed solution?

I think it would be possible for this method to return Promise<ResourceApiResponse> as the rest of the other methods?

skalahasti-cloudinary commented 10 months ago

Hi @rubenvar . Can you please provide a sample code that you are trying?

If you look at the specs.ts , you will see the following definitions: // $ExpectType Promise cloudinary.v2.api.resources( {type: 'facebook'}, function (error, result) { console.log(result, error); } );

// $ExpectType Promise cloudinary.v2.api.resources( { type: 'upload', prefix: 'sample' }, function (error, result) { console.log(result, error); } );

// $ExpectType Promise cloudinary.v2.api.resources( {type: 'upload'}, function (error, result) { console.log(result, error); } ); https://github.com/cloudinary/cloudinary_npm/blob/044e3279c6337379dffcc2e50d6a15c437bcce66/types/cloudinary_ts_spec.ts

Thanks Sree

rubenvar commented 10 months ago

Hi Sree, thanks for the quick response.

Can you please provide a sample code that you are trying?

Of course, but I'm not doing anything special:

const images = await cloudinary.api.resources({ max_results: 14 });

// This works fine, but Type of images is: any
// Because return type by cloudinary.api.resources() is Promise<any>

The specs file that you linked shows that the other .resources() methods (resources_by_context(), etc.) have a return Type of Promise<ResourceApiResponse>.

My request is that the .resources() method would also have the return type Promise<ResourceApiResponse>, instead of Promise<any>, if it's possible.

That way, images in my example would be of type ResourceApiResponse instead of any.

dannyv-cloudinary commented 10 months ago

Hi @rubenvar

Speaking to the development team, this was a conscious choice made to ensure them migration guide didn't get too long or complex, as the changes between v1 and v2 aren't that significant.

There is a project internally to address this for the upcoming v3 release of the SDK, so we can use stricter, more accurate types. For now though, we aren't going to make this change to v2 in order to ensure we're not pushing out breaking changes.

rubenvar commented 10 months ago

Thanks for the clarification @dannyv-cloudinary, understood, no problem 👌