Added proper types for Helper Object and its methods. #14
Conversation
| /** | ||
| * @typedef {Object} HelperType | ||
| * @prop {() => string} getTSCCommand | ||
| * @prop {() => string[]} resolveTSFiles | ||
| * @prop {() => Command} getOptions | ||
| */ |
There was a problem hiding this comment.
I would personally favor adding this to the beginning, all the way before the "@typedef {import('commander').Command}" JSDoc comment (and in its own comment block).
| * @prop {() => string[]} resolveTSFiles | ||
| * @prop {() => Command} getOptions | ||
| */ | ||
| /** @type {HelperType} */ |
There was a problem hiding this comment.
Correct but I will likely move it when I do the cleanup that I proposed in PR #9.
| var Helper = {}; | ||
|
|
||
| /** | ||
| * Find tsc file executable path | ||
| * | ||
| * @returns {string} |
There was a problem hiding this comment.
I am not (yet) convinced to remove the return types. Yes my understanding is that return types can be "implicit" and that they are declared in the Helper type above. But I continue to really like the idea of making the types in JSDoc comments explicit, even for internal functions, like I would do this with JavaDoc comments. This remark applies to multiple places.
| @@ -79,8 +83,6 @@ Helper.getTSCCommand = function () { | |||
|
|
|||
| /** | |||
| * Resolve ts file paths | |||
| * | |||
| * @returns {Array} | |||
There was a problem hiding this comment.
+1 for not using semi-useless "Array" type here
Here is what I would do:
| * @returns {Array} | |
| * | |
| * @returns {string[]} |
| @@ -96,7 +98,6 @@ Helper.resolveTSFiles = function () { | |||
|
|
|||
| /** | |||
| * Get command options | |||
| * @returns {Command} | |||
There was a problem hiding this comment.
not (yet) convinced to get rid of this, as I said before
This is a simpler fix for you checkjs problem. Just needed to define the HelperType object with its methods and return types and then cast the Helper object to that type.