Add linting, ES6 module and TypeScript support

HRKings commented 3 years ago

Hey! First of all, sorry if this PR is a little intrusive.

This PR aims to modernize the package as a whole, what it does:

Adds linting (provided by ESLint, using airbnb-base and custom rules)
Converts to ES6 modules, this enables using import/from
Adds pnpm support (a very fast and space efficient package manager, used by big packages like Vue)
Updates the README to reflect the changes and apply some best practices.
Add typescript support with JSDocs using the generator created by me and C-Garza based off mudkipme

The underlying code logic has not changed and all tests are passing.

Naramsim commented 3 years ago

Thanks for this huge work of art :)

Hahah, anyway, it will take me a bit of time to review everything.

One small question, by turning pokedex-promise-v2 into an ES6 module, everyone will still be able to use it? Or only those writing in ES6?

HRKings commented 3 years ago

Thanks!

Well, converting to a module requires that the person using also changes the type to module in the package.json, but all modern libraries that I looked up the source code uses the "type": "module, including node-fetch that is what the type generator uses

HRKings commented 3 years ago

@Naramsim, I've made a new branch on the fork including the types generated by the schemas, if you want to take a look too

Naramsim commented 3 years ago

Maybe it's better for me to start a review when everything is set and ready :)

HRKings commented 3 years ago

Yep! When we decide what to use in #51 I will update the PR if needed.

Naramsim commented 3 years ago

@C-Garza, feel free to comment and review this PR as well

HRKings commented 3 years ago

Based on the suggestions and requested changes, I've made big changes as I read the code and encountered some todos and some points that could be improved. So:

Updates

Migrated the entire codebase to TypeScript.
- Motivation: we already have ts-morph to generate the types, so I used the same package to generate the main class of the project instead of using prototypes, the underlying logic stays mostly the same
Deprecated some methods to maintain a naming convention:
- resource -> getResource
- cacheSize -> getCachedItemsCount
Fix typos on method names, all of the wrong ones still exists, but are marked as deprecated
- getBerriesFirmnesssList -> getBerriesFirmnessList
- getPokedexsList -> getPokedexList
getResource method now follows the logic of the other methods and uses pMap when passed an array of strings, this change also guarantees that the call receives a complete promise
Organized the tests, all tests are now in .specs.ts files and not in one giant file (still can be more split)
The generators require a node version with ES2021+ (Node LTS 16 supports it)
Swap memory-cache for node-cache
- Motivation: the latest release of memory-cache was 4 years ago, and it had a problem of any code using it would not exit (what happens to pokedex-promise-v2 today), swapping it for node-cache improves performance, fix this issue and on top of this all, it is a more updated package which brings more security and stability

The build process to generate a `dist/src/` folder with the transpiled JS version is:

pnpm run generate:types
pnpm run generate:main
pnpm run generate:jsdocs

This will create a fully working and complete index.js + other required files from the src (I've added the dist/src to the distrbuted files on the package.json)

Type generation

The types are generated with quicktype and cleaned up after. For that to occur, the PokeAPI/api-data must be present in the root of the project and all $ref inside the schemas need to be changed from /schema/v2/... to /api-data/data/schema/v2/... (e.g. /schema/v2/api_resource.json to api-data/data/schema/v2/api_resource.json), this is an limitation of quicktype as it resolves all the refs (so no string.replaceAll can be done)

A bash script could be provided if needed to, to clone the repo and replace all $refs, but I used only the global replace of VSCode and it worked wonders

Naramsim commented 3 years ago

Crazy! This will need a lot of time to review and understand! 😄

Thanks immensely for the time you've put into this! I've updated the docs of PokeAPI, so if I'm right, by re-running the generator, the types should update. Right?

HRKings commented 3 years ago

Thanks immensely for the time you've put into this!

No worries, it always a honor to contribute to such amazing project !

I've updated the docs of PokeAPI, so if I'm right, by re-running the generator, the types should update. Right?

Yep ! I've updated the PR to contain the latest docs too.

HRKings commented 3 years ago

When running npm run generate:main, PokeAPI class and export are appended and thus duplicated. In the index.d.ts file

The intention was to encourage regenerate the types file every time, so the main class would be always updated. But I've made it overwrite anyways now

Naramsim commented 3 years ago

Ok @HRKings, we are close to merging this PR :) It was hard and I still need to look into the code. But for now, I'm asking you to change the version to 4.0.0-beta.0 so we can push it on NPM.

Naramsim commented 3 years ago

Ah, another thing. Would you like to join PokeAPI library contributors so that you can support and maintain this package?

HRKings commented 3 years ago

Ok @HRKings, we are close to merging this PR :) It was hard and I still need to look into the code. But for now, I'm asking you to change the version to 4.0.0-beta.0 so we can push it on NPM.

Changed it! It's been a long road, take your time to look into the code

HRKings commented 3 years ago

Ah, another thing. Would you like to join PokeAPI library contributors so that you can support and maintain this package?

Sure! I would love to help and support the package

Naramsim commented 2 years ago

@HRKings , I'm terribly sorry for the delay.

I merged the PR, made some changes, you can check my commits if you want, and published an NPM beta version: https://www.npmjs.com/package/pokedex-promise-v2

Maybe you can test if including the module in a real typescript application works well. Or we could add a .ts test to this repo.

HRKings commented 2 years ago

@Naramsim, no worries about the delay! I saw the changes, thanks for picking them up. I've been testing the package in a TS app from the beginning and now that I transitioned for the npm beta everything is working as expected. Thanks for all the support!

Naramsim commented 2 years ago

Nice! Alright, so I added automated tests, saw that on Node12 Typescript transpiles well and the tests are all passing. In your opinion, can we set the .engine property in the package.json to >=12?

HRKings commented 2 years ago

I think we totally should set the minimum node version to 12, since it's the LTS.

PokeAPI / pokedex-promise-v2