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

Typescript rewrite #27

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

jamiehaywood
Copy link

@jamiehaywood jamiehaywood commented Jan 27, 2022

This is a first pass at rewriting the whole library in TS.

If you want immediate utility out of this PR, you could cherry pick commit 59e19b0 which has 90% of all the type declarations in a types folder which mirror the existing JS structure. All you would need to do is add a "types" key to the package.json file and point at the types folder.

Otherwise, things still TODO:

  • rewrite exchange-code-for-token.js (w/ accompanying .d.ts file
  • rewrite get-auth-urls.js (w/ accompanying .d.ts file
  • rewrite tokens.js (w/ accompanying .d.ts file
  • Add Prettier
  • Add Husky v7
  • Iron out the build step to compile from TS (probably use rollup to make a ESM export as well)
  • rewrite tests to use jest
  • Add better return types for the requests (instead of Promise<unknown>)
  • Add eslint with TS configuration
  • Fix spelling mistakes in package.json

@davidgtonge
Copy link
Member

Wow, thanks @jamiehaywood

@ozamarripa
Copy link

This looks pretty good!
Out of curiosity, how come you changed the test to use Jest?

@jamiehaywood
Copy link
Author

@ozamarripa a couple of reasons for picking Jest, but the main ones are:

  • It has more weekly downloads on NPM
  • It has got more out of the box (less setup and extra stuff to maintain)
  • Syntax is nicer/clearer in Jest (subjective, but that's my feeling)
  • Coverage reporting out of the box (again, if you're not looking at coverage then it's a non point, but it can be useful)
  • Parallel test running

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.

3 participants