Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor type-checking setup #1969

Merged
merged 11 commits into from
Oct 4, 2023
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",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about typecheck:ts instead of typecheck-ts?
We already use lint:typescript.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been following a pattern I saw and liked long ago, but it is arbitrary. I use the colon for sub-scripts which are the parts of a combo script. So lint:typescript is one part of lint.

"lint": "npm run lint:javascript && npm run lint:typescript",

"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"
],
}