From 96c6c2511344d164446a2f18f2acdbd45f3985af Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 4 Oct 2023 20:37:14 +1300 Subject: [PATCH] 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 --- .eslintrc.js | 2 +- index.js | 8 ++----- jest.config.js | 2 +- lib/argument.js | 2 -- lib/command.js | 2 -- lib/error.js | 2 -- lib/help.js | 2 -- lib/option.js | 2 -- package.json | 8 +++---- tsconfig.js.json | 13 ++++++++++ tsconfig.json | 62 +++++++++++++++++++++++++++++++++++------------- tsconfig.ts.json | 20 ++++++++++++++++ 12 files changed, 86 insertions(+), 39 deletions(-) create mode 100644 tsconfig.js.json create mode 100644 tsconfig.ts.json diff --git a/.eslintrc.js b/.eslintrc.js index 609dafb34..07d28ed76 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -16,7 +16,7 @@ const javascriptSettings = { const typescriptSettings = { files: ['*.ts', '*.mts'], parserOptions: { - project: './tsconfig.json' + project: './tsconfig.ts.json' }, plugins: [ '@typescript-eslint' diff --git a/index.js b/index.js index bbba6e886..d2f4a8528 100644 --- a/index.js +++ b/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. /** diff --git a/jest.config.js b/jest.config.js index efb469bbe..d0e0208f5 100644 --- a/jest.config.js +++ b/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/' diff --git a/lib/argument.js b/lib/argument.js index a51836200..f2835726d 100644 --- a/lib/argument.js +++ b/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. diff --git a/lib/command.js b/lib/command.js index f6d47b447..1f933cead 100644 --- a/lib/command.js +++ b/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`. diff --git a/lib/error.js b/lib/error.js index e7cde9cc7..a0263b501 100644 --- a/lib/error.js +++ b/lib/error.js @@ -1,5 +1,3 @@ -// @ts-check - /** * CommanderError class * @class diff --git a/lib/help.js b/lib/help.js index 32f409161..e2441ed87 100644 --- a/lib/help.js +++ b/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() { diff --git a/lib/option.js b/lib/option.js index 36968b026..048b2fc21 100644 --- a/lib/option.js +++ b/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`. diff --git a/package.json b/package.json index 9382be627..87c02be37 100644 --- a/package.json +++ b/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", diff --git a/tsconfig.js.json b/tsconfig.js.json new file mode 100644 index 000000000..ccbd8f153 --- /dev/null +++ b/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" + ], +} diff --git a/tsconfig.json b/tsconfig.json index 0fde675b6..3a2893488 100644 --- a/tsconfig.json +++ b/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" + ] } diff --git a/tsconfig.ts.json b/tsconfig.ts.json new file mode 100644 index 000000000..c87484aaa --- /dev/null +++ b/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" + ], +}