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

Should we convert this to TypeScript #53

Open
hatched opened this issue Apr 21, 2020 · 1 comment
Open

Should we convert this to TypeScript #53

hatched opened this issue Apr 21, 2020 · 1 comment

Comments

@hatched
Copy link
Collaborator

hatched commented Apr 21, 2020

There has been interest from a couple parties recently to convert this project to TypeScript or at the very least provide type definitions to the library.

A PR has been created to add the later #50

This issue is to track a discussion around the advantages, disadvantages, and necessary tasks to complete the conversion.

@unional
Copy link

unional commented Apr 22, 2020

There are three things I would like to bring up and discuss.
None of these are planned for the initial PR when converting to TypeScript.
Just things that would like to figure out and work on afterwards.

null vs undefined

I saw some usage of null in the code.
I understand that it is probably from the fact the JSON uses null.
On the other hand the typical recommendation for bottom value is undefined.
Some of the code needs to handle both.
Should it be consolidated? Will there be any impact?
I don't see any downside to this but would like to get some feedback on this.

Macaroon class vs closure

newMacaroon() returns a new instance of Macaroon.
This pose a problem as all properties are accessible to the user in JavaScript.

An alternative approach is to use closure to keep private values really private.
Private property #prop is coming to class in new ECMAScript and is available to TypeScript,
but personally I don't see the need to as closure will work just fine and the especially Macaroon class should be considered final, not extensible to the user.

V1 vs V2

There are some slight design issue that V1 code and V2 code are mixed together.
Not too big a deal but would love to refactor that in the future.

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

No branches or pull requests

2 participants