PokeAPI / pokedex-promise-v2

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

How about Typescript support? #51

Closed maykon-oliveira closed 2 years ago

Naramsim commented 2 years ago

I never used Typescript, so I'm reluctant to work on a possible extension. If there is someone already experienced, he could open a PR.

C-Garza commented 2 years ago

I was actually working on making a type definition file for the pokeapi-js-wrapper as practice for some typescript I was learning lol.

This repo has a very similar codebase, so I could work on it after if @maykon-oliveira isn't already working on it.

Naramsim commented 2 years ago

That would be perfect

Naramsim commented 2 years ago

Types for pokeapi-js-wrapper should be already available somewhere.

Here: https://github.com/mudkipme/pokeapi-v2-typescript

HRKings commented 2 years ago

Hey ! I've made a PR #53 that adds TS support too, using the generator above

C-Garza commented 2 years ago

Types for pokeapi-js-wrapper should be already available somewhere.

Here: https://github.com/mudkipme/pokeapi-v2-typescript

Oh wow, that declaration file is great! They parsed the pokeapi docs and and created the interfaces straight from them. They should have uploaded it to the DefinitelyTyped repo.

Naramsim commented 2 years ago

@C-Garza, do you think they scraped the docs? If so we shouldn't really rely on it. Our docs are outdated and they might as well report wrong resource types.

C-Garza commented 2 years ago

@Naramsim Yeah, from what it looks like it generates types from the docs JSON files at the pokeapi.co repo. So you're saying parsing them is a bad approach then? How out of date are they?

Naramsim commented 2 years ago

I guess they are 1-2 years outdated. Some of the values are probably also wrong. I don't remember how we created those JSON files. I think those were extracted from this file https://github.com/PokeAPI/pokeapi/tree/master/pokemon_v2#item-attributes by @cmmartti. And I don't think that file can be generated.

Naramsim commented 2 years ago

Maybe the best would be to use the schema that @sargunv came up with: https://github.com/PokeAPI/api-data/tree/master/data/schema/v2

C-Garza commented 2 years ago

Oh, the JSON schema would probably work really well with generating a declaration file. I'm pretty sure there's a tool that automatically does that. I'll have a look into testing that out sometime tomorrow.

HRKings commented 2 years ago

Yeah, I was looking into that. The generator uses the docs because they provide descriptions to the fields in the interfaces. The schemas only really provide typing, but definitely they will be more updated.

HRKings commented 2 years ago

I've made a little test repo here, the package to generate the types is a little complicate to use with the paths on the $ref of the schemas, but it works.

C-Garza commented 2 years ago

I've made a little test repo here, the package to generate the types is a little complicate to use with the paths on the $ref of the schemas, but it works.

Cool, I'll fork it and play around with it. I'm thinking also maybe just scraping the docs for the descriptions they use.

HRKings commented 2 years ago

I've made a little test repo here, the package to generate the types is a little complicate to use with the paths on the $ref of the schemas, but it works.

Cool, I'll fork it and play around with it. I'm thinking also maybe just scraping the docs for the descriptions they use.

Nice! I just implemented full TS support to the methods too, so its basically complete. The only thing left is to pick the schemas directly from the main repo, but there is a bug in the package that makes parsing refs correctly a pain.

It would be nice if the schemas had the description. Scraping the docs would require another pass of the ts-morph to add the JSDocs

C-Garza commented 2 years ago

Hey @HRKings nice work! I kind of went off in a tangent when you said you were having trouble with the refs using that library, so I tried it out with another one called quicktype. Here is the output on my fork.

It still has a couple of issues, but it works with the refs pretty well. It'll even fetch from the schemas repo online if you change the refs to the github url. Let me know if you think quicktype's output is good or if you solved the ref problem on your end.

I also agree that it would be nice to have descriptions on the schema. It would avoid having to scrape the docs and also make the schema itself work like self-documentation. @Naramsim what do you think about adding some description keys to the schema? Is that doable?

Naramsim commented 2 years ago

I was thinking we could add a .meta.description property to the API itself. I quickly looked at it, but I couldn't come up with a proper way of doing it. I guess one could modify our serializers and statically add a description for each resource. But I don't know if that way will work.

Other than that, the schema itself is auto-generated by Ditto. So no easy/clean way to add descriptions.

HRKings commented 2 years ago

@C-Garza, the quicktype code that you wrote is awesome! The output is way better. I've updated my test repo and the pokedex-promise fork to use quicktype. It has the same problem of having to rename the refs, but it's cleaner and easier to integrate into the ts-morph

@Naramsim, I saw the Ditto code, it only craws the API, correct ? So the description need to be put manually or come from another source that would map into the properties of the schema. Getting this from the docs would be no benefit as it is outdated, right ?

Naramsim commented 2 years ago

Yep, correct. The docs are outdated regarding the sprites resources and the following 4 Pull Requests.

https://github.com/PokeAPI/pokeapi/pull/634 https://github.com/PokeAPI/pokeapi/pull/571 https://github.com/PokeAPI/pokeapi/pull/457 https://github.com/PokeAPI/pokeapi/pull/564

I guess that updating the docs with the new data should be an easy task. Moreover, @phrasmotica always updated the README.md file when creating his Pull Requests, so half the work is done. I'll try in the next few days.

Probably the best option to retrieve the descriptions and types is just to look at the documentation and assume it's correct. If it's not correct someone will file an issue.

https://github.com/PokeAPI/pokeapi.co/tree/master/src/docs Here there are the JSON files

HRKings commented 2 years ago

@Naramsim the Ideia is to generate from the schemas and then generate the JSDocs from the docs?

Naramsim commented 2 years ago

Could be, yes! I think that's the easiest way.

HRKings commented 2 years ago

Nice! I've already implememented It. It works really well and it will be always updated when we run from the command line with minimal manual adjustments. @Naramsim, Should I merge into the master of my fork to update the repo ?

Naramsim commented 2 years ago

Yes yes. If you think your PR should contain also those changes, yes.

HRKings commented 2 years ago

I've updated it, the type definitions should be the most updated and already with the JSDocs

verbumfeit commented 2 years ago

Would it be possible to merge the types from this project with the pokeapi-js-wrapper?

Thank you for your hard work on this!

HRKings commented 2 years ago

With Naramsim permission, I could adapt only the generators without any problems

Naramsim commented 2 years ago

Yeah, I would like that package to remain pure JS. But adding types it's fine, I guess.

HRKings commented 2 years ago

I could create a separated package like @types/pokeapi-js-wrapper too, so the codebase will remain 100% vanilla JS

verbumfeit commented 2 years ago

That would be the way to go, imho

Naramsim commented 2 years ago

Hello @maykon-oliveira, would you be so kind to test the Typescript package just created by @HRKings?

You would need to npm i -s pokedex-promise-v2@beta

Naramsim commented 2 years ago

Typescript support has been added. Thanks immensely @HRKings