Skip to content

Commit

Permalink
Refactor type-checking setup (#1969)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
shadowspawn and aweebit authored Oct 4, 2023
1 parent 744ee3f commit 96c6c25
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const javascriptSettings = {
const typescriptSettings = {
files: ['*.ts', '*.mts'],
parserOptions: {
project: './tsconfig.json'
project: './tsconfig.ts.json'
},
plugins: [
'@typescript-eslint'
Expand Down
8 changes: 2 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

/**
Expand Down
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const config = {
testEnvironment: 'node',
collectCoverage: true,
transform: {
'^.+\\.tsx?$': 'ts-jest'
'^.+\\.tsx?$': ['ts-jest', { tsconfig: 'tsconfig.ts.json' }]
},
testPathIgnorePatterns: [
'/node_modules/'
Expand Down
2 changes: 0 additions & 2 deletions lib/argument.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
const { InvalidArgumentError } = require('./error.js');

// @ts-check

class Argument {
/**
* Initialize a new command argument with the given name and description.
Expand Down
2 changes: 0 additions & 2 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 0 additions & 2 deletions lib/error.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

/**
* CommanderError class
* @class
Expand Down
2 changes: 0 additions & 2 deletions lib/help.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 0 additions & 2 deletions lib/option.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
const { InvalidArgumentError } = require('./error.js');

// @ts-check

class Option {
/**
* Initialize a new `Option` with the given `flags` and `description`.
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 13 additions & 0 deletions tsconfig.js.json
Original file line number Diff line number Diff line change
@@ -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"
],
}
62 changes: 45 additions & 17 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
20 changes: 20 additions & 0 deletions tsconfig.ts.json
Original file line number Diff line number Diff line change
@@ -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"
],
}

0 comments on commit 96c6c25

Please sign in to comment.