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
3 changes: 2 additions & 1 deletion .eslintrc.js
Expand Up @@ -16,7 +16,8 @@ const javascriptSettings = {
const typescriptSettings = {
files: ['*.ts', '*.mts'],
parserOptions: {
project: './tsconfig.json'
/* Use strict settings to enable extra checks */
project: './tsconfig.strict.json'
},
plugins: [
'@typescript-eslint'
Expand Down
2 changes: 0 additions & 2 deletions index.js
Expand Up @@ -4,8 +4,6 @@ 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.
*/
Expand Down
2 changes: 0 additions & 2 deletions 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.
Expand Down
2 changes: 0 additions & 2 deletions lib/command.js
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
@@ -1,5 +1,3 @@
// @ts-check

/**
* CommanderError class
* @class
Expand Down
2 changes: 0 additions & 2 deletions lib/help.js
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
@@ -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
10 changes: 5 additions & 5 deletions package.json
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 check-types-strict",
"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"
"check-types-strict": "tsd && tsc -p tsconfig.strict.json",
"check-types-loose": "tsc",
"test-all": "npm run test && npm run lint && npm run check-types-loose && npm run test-esm"
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved
},
"files": [
"index.js",
Expand Down Expand Up @@ -77,7 +77,7 @@
"testEnvironment": "node",
"collectCoverage": true,
"transform": {
"^.+\\.tsx?$": "ts-jest"
"^.+\\.tsx?$": ["ts-jest", "tsconfig.strict.json"]
},
"testPathIgnorePatterns": [
"/node_modules/"
Expand Down
10 changes: 5 additions & 5 deletions tests/ts-imports.test.ts
@@ -1,7 +1,5 @@
import { program, Command, Option, CommanderError, InvalidArgumentError, InvalidOptionArgumentError, Help, createCommand } from '../';

import * as commander from '../';

// Do some simple checks that expected imports are available at runtime.
// Similar tests to esm-imports-test.js

Expand All @@ -11,9 +9,11 @@ function checkClass(obj: object, name: string): void {
expect(obj.constructor.name).toEqual(name);
}

test('legacy default export of global Command', () => {
checkClass(commander, 'Command');
});
// This test is sensitive to TypeScript settings. Deprecated anyway!
// import * as commander from '../';
// test('legacy default export of global Command', () => {
// checkClass(commander, 'Command');
// });

test('program', () => {
checkClass(program, 'Command');
Expand Down
60 changes: 44 additions & 16 deletions tsconfig.json
@@ -1,18 +1,46 @@
{
"compilerOptions": {
"module": "commonjs",
"lib": [
"es6"
],
"noImplicitAny": true,
"noImplicitThis": true,
"strictNullChecks": true,
"types": [
"node",
"jest"
],
"noEmit": true,
"forceConsistentCasingInFileNames": true
},
"include": ["**/*.ts"],
/*
TypeScript is being used to do type checking across both JavaScript and TypeScript file.
In particular, this picks up some problems in the JSDoc in the JavaScript files, and validates the code
is consistent with the JSDoc.

A complication is that TypeScript does not directly support different config for .ts/.js. To avoid drowning
in strict errors that are tedious to avoid in JavaScript the settings here are not strict. There is a
another config with strict settings for opt-in use.

The settings here are used by VSCode, and TypeScript checks are enabled in included JavaScript files.
eslint supports custom config and uses the strict settings for TypeScript files.
To strictly check the published type definitions, there is a one script using the strict settings.
*/
/* Visit https://aka.ms/tsconfig to read more about tsconfig configuration. */
"compilerOptions": {
"module": "commonjs",
"lib": [
"es6"
],
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved

/* less strict settings to avoid excessive work in JavaScript */
"noImplicitAny": false,
"noImplicitThis": false,
"strictNullChecks": false,
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved

"types": [
"node",
"jest"
],
"noEmit": true, /* just type checking and not emitting transpiled files */
"forceConsistentCasingInFileNames": true,
"allowJs": true,
"checkJs": true,
"esModuleInterop": true
},
"include": [
"*.js",
"*.mjs",
"examples/**/*.ts", /* just ts, js low priority */
"lib/**/*.js",
"tests/**/*.ts", /* just ts, js|cjs|mjs low priority */
"typings/**/*.ts",
"typings/**/*.mts",
],
}
20 changes: 20 additions & 0 deletions tsconfig.strict.json
@@ -0,0 +1,20 @@
{ /*
Strict settings for use with:
- eslint for TypeScript files
- run-script check-types-strict, in particular for strict checks on the type definitions files
*/
/* Visit https://aka.ms/tsconfig to read more about tsconfig configuration. */
"extends": "./tsconfig.json",
"compilerOptions": {
"noImplicitAny": true,
"noImplicitThis": true,
"strictNullChecks": true,
"skipLibCheck": false, /* we want to check our hand crafted definitions */
},
"include": [
"examples/**/*.ts",
"tests/**/*.ts",
"typings/**/*.ts",
"typings/**/*.mts",
],
}