PokeAPI / pokedex-promise-v2

An easy way to use pokeapi v2 with promises in node.js
MIT License
516 stars 91 forks source link

fix: overload methods to know when it should return an array or not #66

Closed bitomic closed 1 year ago

bitomic commented 1 year ago

Currently, using most methods like:

const berry = await pokeapi.getBerryByName( 1 )

will make berry be of type PokeAPITypes.Berry | PokeAPITypes.Berry[], so you must guard it with something like:

let berry = await pokeapi.getBerryByName( 1 )
berry = Array.isArray( berry ) ? berry.at( 0 ) : berry // or berry.pop()
if ( !berry ) return

Typescript allows to overload the declaration of a method so, depending on the parameters, it can infer the return type. This PR makes it so passing number | string allows typescript to know the result won't be an array, and passing Array<number | string> will make it know the result will also be an array.

This only works with the async/await usage of the package. The callback form is untouched.

Naramsim commented 1 year ago

@HRKings can you review?

HRKings commented 1 year ago

@Naramsim, sure thing. I will review it later today

bitomic commented 1 year ago

Dropping a comment to make sure it isn't forgotten and the issue goes stale.

HRKings commented 1 year ago

Hey @bitomic sorry for the delay, got a bit busy. So, this is a manual fix, but this files are generated automagically, this means that in a future update we would have apply the changes again. The best way would be to modify the generator, want to give it a try ?

bitomic commented 1 year ago

I can take a look, but no idea how do you generate the code then. :o Please let me know so I can give it a try! ^^

HRKings commented 1 year ago

You can take a look at the generator folder to see how it's make. I can see that you didn't make a overloard for the callbacks too, I think that this will be important as well

HRKings commented 1 year ago

I have made some changes to the generator to accomodate everything as I had to update the code to the latest version of the API, mind if I close this PR @bitomic ?

bitomic commented 1 year ago

Absolutely no problem, but I am somewhat lost haha. Let me know if I have to do something.

edit: didn't notice there were two comments, only read the lastone.