bittrex-orderbook
bittrex-orderbook copied to clipboard
gtklocker
[WIP] Typescript migration
Let's not go crazy with style changes yet, these will only complicate and delay the reviewing process. Also let's do the work of converting to TypeScript while keeping the .js file type to start with so that the diffs can be reviewed easily. Then there can be a separate commit which just moves files to their .ts counterparts if need be.
Also I'm not seeing changes to package.json for making this publishable. I'm guessing we need to add a build/prepare hook and somehow include the compiled JS and type definitions. I haven't done this before though.
This is also a good opportunity to get rid of babel as a dependency. I think we're using the polyfill for some Array prototype function, but we can probably rely on the TS compiler for that now. If you do that please find the offending function call in the original JS source and make sure its TS compiled counterpart works.
I'm seeing lots of any on the types. Not filling in all the types is OK but prefer to not type the thing at all instead of using any, as this disables the type checking entirely instead of allowing the compiler to infer the type. Make sure we don't have any option enabled in tsconfig which forces us to type everything.
I got rid of the babel compiler and polyfill and use the TS compiler myself as of https://github.com/gtklocker/bittrex-orderbook/commit/1823ba8035cebd7bdf870016b5d40630225e2b99
Also I'm not seeing changes to
package.jsonfor making this publishable. I'm guessing we need to add a build/prepare hook and somehow include the compiled JS and type definitions. I haven't done this before though.
Maybe it also makes sense to create a PR here: https://github.com/DefinitelyTyped/DefinitelyTyped
@themicp definitelytyped is for when you type a JS package you don't control. Seeing as this will be in Typescript we'll provide our type definitions in-package. I think it may be as simple as creating the .d.ts file with tsc and adding a types key on package.json like ccxt has https://github.com/ccxt/ccxt/blob/master/package.json#L46
@pkakelas please look at the previous comments before doing more work and rebase