Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

Conversation

bitomic
Copy link

@bitomic bitomic commented Dec 5, 2022

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
Copy link
Member

Naramsim commented Dec 5, 2022

@HRKings can you review?

@Naramsim Naramsim requested a review from HRKings December 5, 2022 09:27
@HRKings
Copy link
Member

HRKings commented Dec 5, 2022

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

@bitomic
Copy link
Author

bitomic commented Dec 8, 2022

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

@HRKings
Copy link
Member

HRKings commented Dec 9, 2022

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
Copy link
Author

bitomic commented Dec 9, 2022

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
Copy link
Member

HRKings commented Dec 9, 2022

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
Copy link
Member

HRKings commented Dec 9, 2022

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
Copy link
Author

bitomic commented Dec 9, 2022

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.

@HRKings HRKings mentioned this pull request Dec 9, 2022
@bitomic bitomic closed this by deleting the head repository Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants