Address issues from TypeScript checking of javascript

[shadowspawn]

Pull Request

Having another look at TypeScript checks now we have modernised our code.

Previous work: #1132

Reference: https://www.typescriptlang.org/docs/handbook/type-checking-javascript-files.html

Problem

Running TypeScript over index.js shows issues, most of which we would like to fix once we know. A few of them are things we can not avoid, like checking value of a property that may be undefined.

% npx tsc --allowJS --checkJS index.js --noEmit
index.js:25:5 - error TS2322: Type 'string[]' is not assignable to type 'string'.

25     flags = flags.split(/[ ,|]+/);
       ~~~~~

index.js:26:73 - error TS2339: Property 'shift' does not exist on type 'string'.

26     if (flags.length > 1 && !/^[[<]/.test(flags[1])) this.short = flags.shift();
                                                                           ~~~~~

index.js:27:23 - error TS2339: Property 'shift' does not exist on type 'string'.

27     this.long = flags.shift();
                         ~~~~~

index.js:186:9 - error TS2339: Property 'parent' does not exist on type 'Command'.

186     cmd.parent = this;
            ~~~~~~

index.js:216:9 - error TS2339: Property 'parent' does not exist on type 'Command'.

216     cmd.parent = this;
            ~~~~~~

index.js:388:26 - error TS2339: Property 'parent' does not exist on type 'Command'.

388       while (rootCommand.parent) {
                             ~~~~~~

index.js:389:35 - error TS2339: Property 'parent' does not exist on type 'Command'.

389         rootCommand = rootCommand.parent;
                                      ~~~~~~

index.js:441:16 - error TS2339: Property 'defaultValue' does not exist on type 'Option'.

441         option.defaultValue = defaultValue;
                   ~~~~~~~~~~~~

index.js:646:5 - error TS2741: Property 'from' is missing in type '{}' but required in type '{ from: string; }'.

646     parseOptions = parseOptions || {};
        ~~~~~~~~~~~~

  index.js:637:6
    637    * @param {string} parseOptions.from - where the args are from: 'node', 'user', 'electron'
             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    638    * @return {Command} for chaining
        ~~~~~
    'from' is declared here.

index.js:651:48 - error TS2339: Property 'electron' does not exist on type 'ProcessVersions'.

651       if (process.versions && process.versions.electron) {
                                                   ~~~~~~~~

index.js:666:21 - error TS2339: Property 'defaultApp' does not exist on type 'Process'.

666         if (process.defaultApp) {
                        ~~~~~~~~~~

index.js:783:18 - error TS2769: No overload matches this call.
  The last overload gave the following error.
    Argument of type 'string' is not assignable to parameter of type 'Signals'.

783       process.on(signal, () => {
                     ~~~~~~

  node_modules/@types/node/globals.d.ts:987:9
    987         on(event: Signals, listener: SignalsListener): this;
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The last overload is declared here.

index.js:801:15 - error TS2339: Property 'code' does not exist on type 'Error'.

801       if (err.code === 'ENOENT') {
                  ~~~~

index.js:806:22 - error TS2339: Property 'code' does not exist on type 'Error'.

806       } else if (err.code === 'EACCES') {
                         ~~~~

index.js:813:22 - error TS2339: Property 'nestedError' does not exist on type 'CommanderError'.

813         wrappedError.nestedError = err;
                         ~~~~~~~~~~~

index.js:931:41 - error TS2339: Property 'parent' does not exist on type 'Command'.

931     for (let cmd = this; cmd; cmd = cmd.parent) {
                                            ~~~~~~

index.js:1122:22 - error TS8024: JSDoc '@param' tag has name 'flag', but there is no parameter with that name.

1122    * @param {string} flag
                          ~~~~

index.js:1128:31 - error TS2339: Property 'parent' does not exist on type 'Command'.

1128     for (let parentCmd = this.parent; parentCmd; parentCmd = parentCmd.parent) {
                                   ~~~~~~

index.js:1153:33 - error TS2322: Type 'string' is not assignable to type 'Command'.

1153     if (arguments.length === 0) return this._version;
                                     ~~~~~~~~~~~~~~~~~~~~~

index.js:1197:48 - error TS2551: Property '_alias' does not exist on type 'Command'. Did you mean 'alias'?

1197     if (arguments.length === 0) return command._alias;
                                                    ~~~~~~

  index.js:1191:3
    1191   alias(alias) {
           ~~~~~
    'alias' is declared here.

index.js:1201:13 - error TS2551: Property '_alias' does not exist on type 'Command'. Did you mean 'alias'?

1201     command._alias = alias;
                 ~~~~~~

  index.js:1191:3
    1191   alias(alias) {
           ~~~~~
    'alias' is declared here.

index.js:1419:14 - error TS2551: Property '_alias' does not exist on type 'Command'. Did you mean 'alias'?

1419     if (this._alias) {
                  ~~~~~~

  index.js:1191:3
    1191   alias(alias) {
           ~~~~~
    'alias' is declared here.

index.js:1420:38 - error TS2551: Property '_alias' does not exist on type 'Command'. Did you mean 'alias'?

1420       cmdName = cmdName + '|' + this._alias;
                                          ~~~~~~

  index.js:1191:3
    1191   alias(alias) {
           ~~~~~
    'alias' is declared here.

index.js:1423:31 - error TS2339: Property 'parent' does not exist on type 'Command'.

1423     for (let parentCmd = this.parent; parentCmd; parentCmd = parentCmd.parent) {
                                   ~~~~~~


Found 24 errors.

Solution

Fix most issues by altering code and fixing JSDoc, suppress issues on five lines where we are checking properties which may not exist and the like (and no other way to tell TypeScript that we are doing it on purpose).

I used undefined rather than null for a couple of properties to match existing code:

• nestedError to match existing TypeScript definition
• defaultValue to match existing code (which tests for undefined)

% npx tsc --allowJS --checkJS index.js --noEmit 

Depending on how much we like the TypeScript checking, could do less or more:

• I left in ts-check so Visual Studio Code will check file when editing, happy to take it out
• I have not included this TypeScript check in npm run lint, happy to add it

[abetomo]

👍