Add typescript definitions for ethjs-util methods

[cgewecke]

Attempt to resolve #219 by defining types for ethjs-util locally. Not certain this is the right approach, perhaps it should be done by publishing an @types module.

We should also probably extend the tests to validate the types themselves.

For reference, the ethjs-utils methods are defined here.

[github-actions]

Coverage decreased (-0.001%) to 99.673% when pulling 7bb802f on ejs-util-typescript into 3141eda on master.

[cgewecke]

@holgerd77

This is still draft...do you have any advice about what direction this should go in, if any.. 🙂

[holgerd77]

I will leave this to @s1na to make a comment here since he started the issue. From my side this looks like a good start.

[holgerd77]

I've now excluded this from the v7.0.0 release #249. Then we can go here on a bit more relaxed pace and this can very well also be added within a subsequent patch release together with other changes.

[s1na]

Hm also not sure what's the best approach. I guess having the typedefs locally is okay since their repo hasn't changed much in the last 2 years. Ideally they'd merge these typedefs upstream and do a release. Or having these as under a types package as you suggested.

We could also consider "forking" their code (if licenses are compatible) locally into the repo.

[tomonari-t]

I think this is typo.

- "typings/**/*.js"
+ "typings/**/*.ts"

[tomonari-t]

I suggest that the another approach.

compileOptions.typeRoots's default value is @types.
So make below folders and put ethjs-util.d.ts to index.d.ts.
You don't need to modify tsconfig.json

- src
    - @types
        - ethjs-util
            - index.d.ts

[cgewecke]

@tomonari Thanks so much for catching this! Makes sense.

[cgewecke]

@tomonari Unfortunately, fixing the typo does not seem to resolve the requirement that ethjs-util be imported at the top of index.ts with a path reference comment.

Also had no luck with your suggestion about adding @types folder in src. Maybe something strange is going on with extending the ethereumjs config, idk.

[tomonari-t]

@cgewecke
Thanks for trying it.
I also tried and find out that it is not caused of ts but karma-typescript.
I'm dealing with it.

[tomonari-t]

@cgewecke
I made example code. Please check tomonari-t#2.

[cgewecke]

@tomonari-t

Yes that works! Have made your suggested changes in 40ecfe9 and added you as the commit author. Thank you for investigating this.

[tomonari-t]

@s1na
Your typedef is awesome!
I think it is best that creating PRs the adding typedef of ethjs-util to @types repository.

[lgtm-com]

This pull request introduces 1 alert when merging c5ee67f into 58c2476 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[holgerd77]

LGTM, thanks Chris

[holgerd77]

@cgewecke @tomonari-t just discovered on some experimentation around #257 that the changes on the tsconfig.json and tsconfig.prod.json files here broke the distribution build since source files are now placed in dist/src instead of dist, also the types from this PR are not transfered to the dist folder on build.

Can you have a look here and open a PR fixing this?

[cgewecke]

@holgerd77 Sorry about this. Have opened #260 which I think fixes.