Refactor type-checking setup (#1969)

* Refactor type-checking setup

* Refactor tsconfig particularly to enable loose check in VSCode, strict checks run separately for type definitions

* Simplify includes for tsconfig

* Explicitly separate the tsconfig for use with npm run-scripts

* Improve comment

* Resolved couple of work-in-progress comments

* Update tsconfig to recommended node16 lib/module/target

* Make checks strict by default and opt-out

* Restore broken code to merge later changes

* Updates after merge

---------



Co-authored-by: Wee Bit <aweebit64@gmail.com>

.eslintrc.js

@@ -16,7 +16,7 @@ const javascriptSettings = {
 const typescriptSettings = {
   files: ['*.ts', '*.mts'],
   parserOptions: {
-    project: './tsconfig.json'
+    project: './tsconfig.ts.json'
   },
   plugins: [
     '@typescript-eslint'

index.js

@@ -4,16 +4,12 @@ const { CommanderError, InvalidArgumentError } = require('./lib/error.js');
 const { Help } = require('./lib/help.js');
 const { Option } = require('./lib/option.js');
 
-// @ts-check
-
 /**
  * Expose the root command.
  */
 
-const program = new Command();
-exports = module.exports = program; // default export (deprecated)
-exports.program = program; // more explicit access to global command
-
+exports = module.exports = new Command();
+exports.program = exports; // More explicit access to global command.
 // createArgument, createCommand, and createOption are implicitly available as they are methods on program.
 
 /**

jest.config.js

@@ -2,7 +2,7 @@ const config = {
   testEnvironment: 'node',
   collectCoverage: true,
   transform: {
-    '^.+\\.tsx?$': 'ts-jest'
+    '^.+\\.tsx?$': ['ts-jest', { tsconfig: 'tsconfig.ts.json' }]
   },
   testPathIgnorePatterns: [
     '/node_modules/'

lib/argument.js

@@ -1,7 +1,5 @@
 const { InvalidArgumentError } = require('./error.js');
 
-// @ts-check
-
 class Argument {
   /**
    * Initialize a new command argument with the given name and description.

lib/command.js

@@ -10,8 +10,6 @@ const { Help } = require('./help.js');
 const { Option, splitOptionFlags, DualOptions } = require('./option.js');
 const { suggestSimilar } = require('./suggestSimilar');
 
-// @ts-check
-
 class Command extends EventEmitter {
   /**
    * Initialize a new `Command`.

lib/error.js

@@ -1,5 +1,3 @@
-// @ts-check
-
 /**
  * CommanderError class
  * @class

lib/help.js

@@ -8,8 +8,6 @@ const { humanReadableArgName } = require('./argument.js');
  * @typedef { import("./option.js").Option } Option
  */
 
-// @ts-check
-
 // Although this is a class, methods are static in style to allow override using subclass or just functions.
 class Help {
   constructor() {

lib/option.js

@@ -1,7 +1,5 @@
 const { InvalidArgumentError } = require('./error.js');
 
-// @ts-check
-
 class Option {
   /**
    * Initialize a new `Option` with the given `flags` and `description`.

package.json

@@ -22,11 +22,11 @@
     "lint": "npm run lint:javascript && npm run lint:typescript",
     "lint:javascript": "eslint index.js esm.mjs \"lib/*.js\" \"tests/**/*.js\"",
     "lint:typescript": "eslint typings/*.ts tests/*.ts",
-    "test": "jest && npm run test-typings",
+    "test": "jest && npm run typecheck-ts",
     "test-esm": "node ./tests/esm-imports-test.mjs",
-    "test-typings": "tsd",
-    "typescript-checkJS": "tsc --allowJS --checkJS index.js lib/*.js --noEmit",
-    "test-all": "npm run test && npm run lint && npm run typescript-checkJS && npm run test-esm"
+    "typecheck-ts": "tsd && tsc -p tsconfig.ts.json",
+    "typecheck-js": "tsc -p tsconfig.js.json",
+    "test-all": "npm run test && npm run lint && npm run typecheck-js && npm run test-esm"
   },
   "files": [
     "index.js",

tsconfig.js.json

@@ -0,0 +1,13 @@
+{ /* 
+    Simple override including just JavaScript files.
+    Used by npm run-script typecheck-js
+  */
+  /* Visit https://aka.ms/tsconfig to read more about tsconfig configuration. */
+  "extends": "./tsconfig.json",
+  "include": [
+    /* All JavaScript targets from tsconfig.json include. */
+    "*.js",
+    "*.mjs",
+    "lib/**/*.js"
+  ],
+}

tsconfig.json

@@ -1,19 +1,47 @@
 {
-    "compilerOptions": {
-        "module": "commonjs",
-        "lib": [
-            "es6"
-        ],
-        "noImplicitAny": true,
-        "noImplicitThis": true,
-        "strictNullChecks": true,
-        "types": [
-          "node",
-          "jest"
-        ],
-        "esModuleInterop": true, // Mainly so can test an import problem which only occurs with this option on!
-        "noEmit": true,
-        "forceConsistentCasingInFileNames": true
-    },
-    "include": ["**/*.ts"],
+  /*
+    TypeScript is being used to do type checking across both JavaScript and TypeScript files.
+    In particular, this picks up some problems in the JSDoc in the JavaScript files, and validates the code
+    is consistent with the JSDoc.
+     
+    The settings here are used by VSCode.
+    
+    See also tsconfig.js.json and tsconfig.ts.json.
+  */
+  /* Visit https://aka.ms/tsconfig to read more about tsconfig configuration. */
+  "compilerOptions": {
+    "lib": ["es2021"],
+    "module": "node16",
+    "target": "es2021",
+
+    "allowJs": true,
+    "checkJs": true,
+
+    /* Strict by default, but dial it down to reduce churn in our JavaScript code. */
+    "strict": true,
+    "noImplicitAny": false, 
+    "strictNullChecks": false,
+    "useUnknownInCatchVariables": false,
+
+    "types": [
+      "node",
+      "jest"
+    ],
+    "noEmit": true, /* just type checking and not emitting transpiled files */
+    "skipLibCheck": false, /* we want to check our hand crafted definitions */
+    "forceConsistentCasingInFileNames": true,
+    "esModuleInterop": true /* common TypeScript config */
+  },
+  "include": [
+    /* JavaScript. Should match includes in tsconfig.js.json. */
+    "*.js",
+    "*.mjs",
+    "lib/**/*.js",
+    /* TypeScript. Should match includes in tsconfig.ts.json. */
+    "**/*.ts",
+    "**/*.mts"
+  ],
+  "exclude": [
+    "node_modules"
+  ]
 }

tsconfig.ts.json

@@ -0,0 +1,20 @@
+{ /* 
+    Override to include just TypeScript files and use stricter settings than we do with JavaScript.
+    Used by:
+    - npm run-script typecheck-ts
+    - eslint
+  */
+  /* Visit https://aka.ms/tsconfig to read more about tsconfig configuration. */
+  "extends": "./tsconfig.json",
+  "compilerOptions": {
+    /* Full strict is fine for the TypeScript files, so turn back on the checks we turned off for mixed-use. */
+    "noImplicitAny": true, 
+    "strictNullChecks": true,
+    "useUnknownInCatchVariables": true,
+  },
+  "include": [
+    /* All TypeScript targets from tsconfig.json include. */
+    "**/*.ts",
+    "**/*.mts"
+  ],
+}