vankasteelj / opensubtitles-api

nodejs opensubtitles.org api wrapper for downloading and uploading subtitles in multiple langs
110 stars 33 forks source link

Suggestion: TypeScript #48

Closed SubJunk closed 4 years ago

SubJunk commented 4 years ago

I saw https://github.com/vankasteelj/opensubtitles-api/issues/42 but it seems the user didn't follow up with reasoning for the suggestion when asked, and I am willing to.

I really value the simple features that TypeScript offers, I find they help me write more accurate code, and spend less time learning the code.

To use a basic example, if we take https://github.com/vankasteelj/opensubtitles-api/blob/master/index.js#L144

hash(path) {
  if (!path) throw Error('Missing path')
  return libhash.computeHash(path)
}

There is nothing here that tells me what type of data I expect path to contain, and more importantly I don't know what to expect it to return. It turns out that it returns an object like:

{
  moviehash: string,
  moviebytesize: string
}

but in order to know that, you need to go into computeHash itself https://github.com/vankasteelj/opensubtitles-api/blob/master/lib/hash.js#L51 and find the resolve part.

Instead it could be like this:

interface ComputedFileInfo {
  moviehash: string;
  moviebytesize: string;
}

hash(path: string): ComputedFileInfo {
  if (!path) throw Error('Missing path')
  return libhash.computeHash(path)
}

and in IDEs we can just hover over that custom type ComputedFileInfo to see what we can expect to be returned.

Happy to rant more about it if it still doesn't seem like a good choice

vankasteelj commented 4 years ago

well, it could be a good choice. But I can't maintain that, and I use this npm module almost daily so I need to be able to update it and fix it if needed.

SubJunk commented 4 years ago

@vankasteelj no problem :)