Convert project to TypeScript

[danfinlay]

As a binary-heavy module with a lot of complicated parameters, this is a valuable module to add typescript to. In the past, we've failed to add a proper declaration file, so rather than take that half-measure, we should actually convert the module to TypeScript.

In case we add a bounty, we need well defined acceptance criteria. Those would be:

• The changes should obviously not change module behavior at all.
• All method parameters and return types should be typed.
• Should introduce an npm build script which builds the typescript file into an output file.
• the package.json main field should point to the built output.
• Anyone re-running build should be able to deterministically produce the same output.
• The typedef file should be generated programmatically from the input script.
• Migrating to yarn in the process is acceptable but not required.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

This issue now has a funding of 75.0 DAI (75.0 USD @ $1.0/DAI) attached to it.

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $101,189.03 more funded OSS Work available on the Gitcoin Issue Explorer

[developerfred]

I would like to start this work. ❤️

[mul53]

@lazaridiscom, I found this file eccrypto-lite.js under the folder /utils, it is not referenced in the main module. Should i refactor it?

[danfinlay]

Since it appears to be unused, you can ignore that file for your typescript migration, we can remove it in a separate change.

Please note that @lazaridiscom is not a MetaMask team member, and is not in a position to define the terms of this bounty, and is merely being helpful by offering his advice.

Lazaridiscom, please try to avoid situations where external contributors mistake you for a team member.

[mul53]

@lazaridiscom love the help!, your tips are very helpful.

[gitcoinbot]

@mul53 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• reminder (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[mul53]

Hey @lazaridiscom just realized with the work i currently have, I won't manage to finish the work, would be better to give someone else the bounty.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 266 years, 9 months from now.
Please review their action plans below:

1) alexandrumatei36 has applied to start work (Funders only: approve worker | reject worker).

• implement the quick changes necessary to migrate to TS (add tsc compiler, set up the tsconfig file, modify package.json file)
• make sure building and testing works as expected
  2) kichjang has been approved to start work.

I took an initial look at the repository and the preceding PR (#70), which contains pretty much the basic setup to get the ball rolling for any JS to TS conversion project.

The next and most important step there would be to add all the relevant typing information for every API and object that is used by the project. I can see that there's a special variable called TYPED_MESSAGE_SCHEMA, which would the prime candidate to convert to a proper TypeScript interface.

I expect this work to be done in 2 days. I say 2 instead of 1 just to pad in an extra day for unforeseen difficulties that I may encounter during implementation. To speed things up, can I ask where should I contact the maintainers to have a quicker response while I'm working on it?
3) hrithikgautham has applied to start work (Funders only: approve worker | reject worker).

I know JavaScript and Typescript. I can pull this off.
4) leinadpb has applied to start work (Funders only: approve worker | reject worker).

1. Examinate all source code.
2. Build models (and a service layer if required)
3. Refactor code using SOLID principles
   5) nttmai has applied to start work (Funders only: approve worker | reject worker).

I worked with ts for a blockchain project before, i want to do this job
6) nttmai has applied to start work (Funders only: approve worker | reject worker).

I worked with ts for a blockchain project before, i want to do this job
7) michalkotas has applied to start work (Funders only: approve worker | reject worker).

Converting each parts into properly typed (no implicit "any") blocks; define interfaces for data structures; verify if tests gives the same results after conversion.

Learn more on the Gitcoin Issue Details page.

[jellz]

Hey!

I saw this bounty to convert the JavaScript project to TypeScript.

Is this bounty still open? Is anyone working on it right now? I saw the message from the bot above but I'm not sure so if someone could let me know the status that'd be great.

Cheers.

[gitcoinbot]

💰 A crowdfund contribution worth 5.00000 DAI (5.0 USD @ $1.0/DAI) has been attached to this funded issue from @bakkdoor.💰

Want to chip in also? Add your own contribution here.

[tunnckoCore]

So, why #71 is closed? It almost looks good to me?

@mul53 ?

[KiChjang]

I should also add that it'd be nice if we can also convert the example into using TypeScript.

[mul53]

@tunnckoCore what @lazaridiscom said is true, it contains a lot of followup tasks. But it can be done, just time i didn't have on my hands.

[tcrowe]

Hi ppl 👋 You might benefit from enabling TS strict mode since the types, numbers, and binary have to be precise. In the CLI --strict or "strict": true, in tsconfig.json.

[NTP-996]

@danfinlay i just finished this issue, can you check it out ? #73

[danfinlay]

Hey there, back from a winter break. I see #73 has been submitted, but it did bypass the requested Gitcoin worker/approve process, which I suspect is why Chiro gave it 👎.

We're going to review that work since it's done and decide from there, will be back here soon, sorry for the delay and the approve/stopped bounty on Chiro, feel free to re-apply, we'll admit if we decide against this submission.

[danfinlay]

Okay, a quick review of #73, it seems to have merely moved the .js to .ts file. That's very funny, because JS is valid TS, but it isn't what is intended by this issue, I'll be clarifying the master post to clarify.

[tcrowe]

Hey guys I might also suggest you refactor and reuse all/most strings and numbers. You can convert everything to constants and you can also leverage Object.freeze({}) to ensure users don't overwrite values.

For example Object.freeze with things like this: https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L7

Reference a constant & don't keep writing and instantiating the same string:
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L44
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L50
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L59

Pull out weird values and describe them with JSDoc comments:
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L51
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L210

Does it really need to use this.? Is it really tracking internal state and should it? Right now it's kind of an unorthodox implicit class declaration. Maybe it can all be pure functions or classes if it really needs internal state.
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L34
https://github.com/MetaMask/eth-sig-util/blob/master/index.js#L219

If you think it's helpful you can @-ping me any time, kudos, tip or 👍 or 🤗. https://gitcoin.co/tcrowe

[KiChjang]

@danfinlay Do you mind doing the honors and sending in the payment for this bounty?

[gitcoinbot]

⚡️ A tip worth 7.50000 DAI (7.5 USD @ $1.0/DAI) has been granted to @KiChjang for this issue from @danfinlay. ⚡️

Nice work @KiChjang! Your tip has automatically been deposited in the ETH address we have on file.

• $75385.74 in Funded OSS Work Available at: https://gitcoin.co/explorer
• Incentivize contributions to your repo: Send a Tip or Fund a PR
• No Email? Get help on the Gitcoin Slack