feat: add eslint typescript support (#305)

* test: create test fixture for eslint + activate vscode plugin * feat: add eslint option infrastructure * feat: add basic implementation (untested) - might work! * chore: disable eslint in Travis for now * test: added first test of eslint message being published * test: add error test for eslint * feat: add eslint support to IncrementalChecker * test: remove duplicate test and up test timeout * docs: add eslint details to readme * feat: validate eslint and allow options to be passed * fix: correctly output version * feat: persist eslinter instance between runs * feat: incrementalchecker no longer recreates eslint each time * feat: share eslint between checkers * feat: share eslint logic between checkers * fix: respond to @piotr-oles PR feedback * fix: delete stale lints first * fix: addressed comments suggested by @phryneas * docs: more eslint info in the readme * fix: respond to review comments from piotr * fix: move cancellation token check
TypeStrong · Jul 12, 2019 · 544f205 · 544f205
1 parent cae170b
commit 544f205
Show file tree

Hide file tree

Showing 35 changed files with 931 additions and 333 deletions.
diff --git a/.eslintrc.json b/.eslintrc.json
diff --git a/.gitignore b/.gitignore
@@ -25,4 +25,3 @@ package-lock.json
 
 # Editor directories and files
 .idea
-.vscode
diff --git a/.travis.yml b/.travis.yml
@@ -14,7 +14,6 @@ install:
   - yarn add $WEBPACK $TSLOADER $VUELOADER -D
 
 script:
-  - yarn lint
   - yarn test
 
 env:

diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -0,0 +1,8 @@
+{
+  "eslint.validate": [
+    "javascript",
+    "javascriptreact",
+    "typescript",
+    "typescriptreact"
+  ]
+}
diff --git a/README.md b/README.md
@@ -1,29 +1,35 @@
 # Fork TS Checker Webpack Plugin
 
 [![npm version](https://img.shields.io/npm/v/fork-ts-checker-webpack-plugin.svg)](https://www.npmjs.com/package/fork-ts-checker-webpack-plugin)
+
 [![npm beta version](https://img.shields.io/npm/v/fork-ts-checker-webpack-plugin/beta.svg)](https://www.npmjs.com/package/fork-ts-checker-webpack-plugin)
+
 [![build status](https://travis-ci.org/TypeStrong/fork-ts-checker-webpack-plugin.svg?branch=master)](https://travis-ci.org/TypeStrong/fork-ts-checker-webpack-plugin)
+
 [![downloads](http://img.shields.io/npm/dm/fork-ts-checker-webpack-plugin.svg)](https://npmjs.org/package/fork-ts-checker-webpack-plugin)
+
 [![commitizen friendly](https://img.shields.io/badge/commitizen-friendly-brightgreen.svg)](http://commitizen.github.io/cz-cli/)
+
 [![code style: prettier](https://img.shields.io/badge/code_style-prettier-ff69b4.svg)](https://github.com/prettier/prettier)
+
 [![semantic-release](https://img.shields.io/badge/%20%20%F0%9F%93%A6%F0%9F%9A%80-semantic--release-e10079.svg)](https://github.com/semantic-release/semantic-release)
 
 Webpack plugin that runs TypeScript type checker on a separate process.
 
 ## Installation
 
-This plugin requires minimum **webpack 2.3**, **TypeScript 2.1** and optionally **tslint 4.0**
+This plugin requires minimum **webpack 2.3**, **TypeScript 2.1** and optionally **ESLint 6.0.0** or **TSLint 4.0**
 
 ```sh
-npm install --save-dev fork-ts-checker-webpack-plugin
+yarn add fork-ts-checker-webpack-plugin --dev
 ```
 
 Basic webpack config (with [ts-loader](https://github.com/TypeStrong/ts-loader))
 
 ```js
-var ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
+const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
 
-var webpackConfig = {
+const webpackConfig = {
   context: __dirname, // to automatically find tsconfig.json
   entry: './src/index.ts',
   module: {
@@ -44,12 +50,12 @@ var webpackConfig = {
 
 ## Motivation
 
-There is already similar solution - [awesome-typescript-loader](https://github.com/s-panferov/awesome-typescript-loader). You can
+There was already similar solution - [awesome-typescript-loader](https://github.com/s-panferov/awesome-typescript-loader). You can
 add `CheckerPlugin` and delegate checker to the separate process. The problem with `awesome-typescript-loader` was that, in our case,
 it was a lot slower than [ts-loader](https://github.com/TypeStrong/ts-loader) on an incremental build (~20s vs ~3s).
-Secondly, we use [tslint](https://palantir.github.io/tslint) and we wanted to run this, along with type checker, in a separate process.
-This is why we've created this plugin. To provide better performance, plugin reuses Abstract Syntax Trees between compilations and shares
-these trees with tslint. It can be scaled with a multi-process mode to utilize maximum CPU power.
+Secondly, we used [tslint](https://palantir.github.io/tslint) and we wanted to run this, along with type checker, in a separate process.
+This is why this plugin was created. To provide better performance, the plugin reuses Abstract Syntax Trees between compilations and shares
+these trees with TSLint. It can be scaled with a multi-process mode to utilize maximum CPU power.
 
 ## Modules resolution
 
@@ -61,12 +67,47 @@ to compile files (which traverses dependency graph during compilation) - we have
 
 To debug TypeScript's modules resolution, you can use `tsc --traceResolution` command.
 
+## ESLint
+
+[ESLint is the future of linting in the TypeScript world.](https://eslint.org/blog/2019/01/future-typescript-eslint) If you'd like to use eslint with the plugin, supply this option: `eslint: true` and ensure you have the relevant dependencies installed:
+
+`yarn add eslint @typescript-eslint/parser @typescript-eslint/eslint-plugin --dev`
+
+You should have an ESLint configuration file in your root project directory. Here is a sample `.eslintrc.js` configuration for a TypeScript project:
+
+```js
+const path = require('path');
+module.exports = {
+  parser: '@typescript-eslint/parser', // Specifies the ESLint parser
+  extends: [
+    'plugin:@typescript-eslint/recommended' // Uses the recommended rules from the @typescript-eslint/eslint-plugin
+  ],
+  parserOptions: {
+    project: path.resolve(__dirname, './tsconfig.json'),
+    tsconfigRootDir: __dirname,
+    ecmaVersion: 2018, // Allows for the parsing of modern ECMAScript features
+    sourceType: 'module', // Allows for the use of imports
+  },
+  rules: {
+    // Place to specify ESLint rules. Can be used to overwrite rules specified from the extended configs
+    // e.g. "@typescript-eslint/explicit-function-return-type": "off",
+  }
+};
+```
+
+There's a good explanation on setting up TypeScript ESLint support by Robert Cooper [here](https://dev.to/robertcoopercode/using-eslint-and-prettier-in-a-typescript-project-53jb).
+
 ## TSLint
 
+*[TSLint is being replaced by ESLint](https://medium.com/palantir/tslint-in-2019-1a144c2317a9).
+https://eslint.org/blog/2019/01/future-typescript-eslint. As a consequence, support for TSLint in fork-ts-checker-webpack-plugin will be deprecated and removed in future versions of the plugin.*
+
 If you have installed [tslint](https://palantir.github.io/tslint), you can enable it by setting `tslint: true` or
 `tslint: './path/to/tslint.json'`. We recommend changing `defaultSeverity` to a `"warning"` in `tslint.json` file.
 It helps to distinguish lints from TypeScript's diagnostics.
 
+
+
 ## Options
 
 - **tsconfig** `string`:
@@ -75,6 +116,14 @@ It helps to distinguish lints from TypeScript's diagnostics.
 - **compilerOptions** `object`:
   Allows overriding TypeScript options. Should be specified in the same format as you would do for the `compilerOptions` property in tsconfig.json. Default: `{}`.
 
+- **eslint** `true | undefined`:
+
+  - If `true`, this activates eslint support.
+
+- **eslintOptions** `object`:
+
+  - Options that can be used to initialise ESLint. See https://eslint.org/docs/1.0.0/developer-guide/nodejs-api#cliengine
+
 - **tslint** `string | true | undefined`:
 
   - If `string`, path to _tslint.json_ file to check source files against.

diff --git a/package.json b/package.json
@@ -67,11 +67,6 @@
         "git add"
       ],
       "*.ts": [
-        "tslint --fix",
-        "prettier --write",
-        "git add"
-      ],
-      "*.{yml,json,md,html}": [
         "prettier --write",
         "git add"
       ]
@@ -113,6 +108,7 @@
     "@commitlint/config-conventional": "^7.5.0",
     "@types/babel-code-frame": "^6.20.1",
     "@types/chokidar": "^1.7.5",
+    "@types/eslint": "^4.16.6",
     "@types/jest": "^24.0.11",
     "@types/lodash": "^4.14.134",
     "@types/micromatch": "^3.1.0",
@@ -122,10 +118,12 @@
     "@types/rimraf": "^2.0.2",
     "@types/semver": "^5.5.0",
     "@types/webpack": "^4.4.19",
+    "@typescript-eslint/eslint-plugin": "^1.11.0",
+    "@typescript-eslint/parser": "^1.11.0",
     "commitlint": "^7.5.2",
     "copy-dir": "^0.4.0",
     "css-loader": "0.28.11",
-    "eslint": "^5.7.0",
+    "eslint": "^6.0.0",
     "git-cz": "^3.0.1",
     "husky": "^1.1.4",
     "istanbul": "^0.4.5",

diff --git a/src/.eslintrc.js b/src/.eslintrc.js
@@ -0,0 +1,9 @@
+module.exports = {
+  parser: '@typescript-eslint/parser',
+  plugins: ['@typescript-eslint'],
+  parserOptions: {
+    project: './tsconfig.json',
+    tsconfigRootDir: __dirname,
+    sourceType: 'module'
+  }
+};
diff --git a/src/ApiIncrementalChecker.ts b/src/ApiIncrementalChecker.ts
@@ -1,10 +1,15 @@
 // tslint:disable-next-line:no-implicit-dependencies
 import * as ts from 'typescript'; // Imported for types alone
 // tslint:disable-next-line:no-implicit-dependencies
-import { Linter, LintResult, RuleFailure } from 'tslint';
+import { Linter, LintResult, RuleFailure } from 'tslint'; // Imported for types alone
+// tslint:disable-next-line:no-implicit-dependencies
+import * as eslinttypes from 'eslint'; // Imported for types alone
 import * as minimatch from 'minimatch';
 import * as path from 'path';
-import { IncrementalCheckerInterface } from './IncrementalCheckerInterface';
+import {
+  IncrementalCheckerInterface,
+  ApiIncrementalCheckerParams
+} from './IncrementalCheckerInterface';
 import { CancellationToken } from './CancellationToken';
 import {
   ConfigurationFile,
@@ -13,8 +18,8 @@ import {
 } from './linterConfigHelpers';
 import { NormalizedMessage } from './NormalizedMessage';
 import { CompilerHost } from './CompilerHost';
-import { ResolveModuleName, ResolveTypeReferenceDirective } from './resolution';
-import { FsHelper } from './FsHelper';
+import { fileExistsSync } from './FsHelper';
+import { createEslinter } from './createEslinter';
 
 export class ApiIncrementalChecker implements IncrementalCheckerInterface {
   private linterConfig?: ConfigurationFile;
@@ -24,28 +29,47 @@ export class ApiIncrementalChecker implements IncrementalCheckerInterface {
   private linterExclusions: minimatch.IMinimatch[] = [];
 
   private currentLintErrors = new Map<string, LintResult>();
+  private currentEsLintErrors = new Map<
+    string,
+    eslinttypes.CLIEngine.LintReport
+  >();
   private lastUpdatedFiles: string[] = [];
   private lastRemovedFiles: string[] = [];
 
   private readonly hasFixedConfig: boolean;
 
-  constructor(
-    typescript: typeof ts,
-    private createNormalizedMessageFromDiagnostic: (
-      diagnostic: ts.Diagnostic
-    ) => NormalizedMessage,
-    private createNormalizedMessageFromRuleFailure: (
-      ruleFailure: RuleFailure
-    ) => NormalizedMessage,
-    programConfigFile: string,
-    compilerOptions: ts.CompilerOptions,
-    private context: string,
-    private linterConfigFile: string | boolean,
-    private linterAutoFix: boolean,
-    checkSyntacticErrors: boolean,
-    resolveModuleName: ResolveModuleName | undefined,
-    resolveTypeReferenceDirective: ResolveTypeReferenceDirective | undefined
-  ) {
+  private readonly context: string;
+  private readonly createNormalizedMessageFromDiagnostic: (
+    diagnostic: ts.Diagnostic
+  ) => NormalizedMessage;
+  private readonly linterConfigFile: string | boolean;
+  private readonly linterAutoFix: boolean;
+  private readonly createNormalizedMessageFromRuleFailure: (
+    ruleFailure: RuleFailure
+  ) => NormalizedMessage;
+  private readonly eslinter: ReturnType<typeof createEslinter> | undefined;
+
+  constructor({
+    typescript,
+    context,
+    programConfigFile,
+    compilerOptions,
+    createNormalizedMessageFromDiagnostic,
+    linterConfigFile,
+    linterAutoFix,
+    createNormalizedMessageFromRuleFailure,
+    eslinter,
+    checkSyntacticErrors = false,
+    resolveModuleName,
+    resolveTypeReferenceDirective
+  }: ApiIncrementalCheckerParams) {
+    this.context = context;
+    this.createNormalizedMessageFromDiagnostic = createNormalizedMessageFromDiagnostic;
+    this.linterConfigFile = linterConfigFile;
+    this.linterAutoFix = linterAutoFix;
+    this.createNormalizedMessageFromRuleFailure = createNormalizedMessageFromRuleFailure;
+    this.eslinter = eslinter;
+
     this.hasFixedConfig = typeof this.linterConfigFile === 'string';
 
     this.initLinterConfig();
@@ -97,6 +121,10 @@ export class ApiIncrementalChecker implements IncrementalCheckerInterface {
     return !!this.linterConfigFile;
   }
 
+  public hasEsLinter(): boolean {
+    return this.eslinter !== undefined;
+  }
+
   public isFileExcluded(filePath: string): boolean {
     return (
       filePath.endsWith('.d.ts') ||
@@ -140,7 +168,7 @@ export class ApiIncrementalChecker implements IncrementalCheckerInterface {
         this.currentLintErrors.set(updatedFile, lints);
       } catch (e) {
         if (
-          FsHelper.existsSync(updatedFile) &&
+          fileExistsSync(updatedFile) &&
           // check the error type due to file system lag
           !(e instanceof Error) &&
           !(e.constructor.name === 'FatalError') &&
@@ -165,4 +193,24 @@ export class ApiIncrementalChecker implements IncrementalCheckerInterface {
       allLints.map(this.createNormalizedMessageFromRuleFailure)
     );
   }
+
+  public getEsLints(cancellationToken: CancellationToken) {
+    for (const removedFile of this.lastRemovedFiles) {
+      this.currentEsLintErrors.delete(removedFile);
+    }
+
+    for (const updatedFile of this.lastUpdatedFiles) {
+      cancellationToken.throwIfCancellationRequested();
+      if (this.isFileExcluded(updatedFile)) {
+        continue;
+      }
+
+      const lints = this.eslinter!.getLints(updatedFile);
+      if (lints !== undefined) {
+        this.currentEsLintErrors.set(updatedFile, lints);
+      }
+    }
+
+    return this.eslinter!.getFormattedLints(this.currentEsLintErrors.values());
+  }
 }
diff --git a/src/CancellationToken.ts b/src/CancellationToken.ts
@@ -5,7 +5,7 @@ import * as path from 'path';
 // tslint:disable-next-line:no-implicit-dependencies
 import * as ts from 'typescript'; // Imported for types alone
 
-import { FsHelper } from './FsHelper';
+import { fileExistsSync } from './FsHelper';
 
 export interface CancellationTokenData {
   isCancelled: boolean;
@@ -60,7 +60,7 @@ export class CancellationToken {
     if (duration > 10) {
       // check no more than once every 10ms
       this.lastCancellationCheckTime = time;
-      this.isCancelled = FsHelper.existsSync(this.getCancellationFilePath());
+      this.isCancelled = fileExistsSync(this.getCancellationFilePath());
     }
 
     return this.isCancelled;
@@ -78,10 +78,7 @@ export class CancellationToken {
   }
 
   public cleanupCancellation() {
-    if (
-      this.isCancelled &&
-      FsHelper.existsSync(this.getCancellationFilePath())
-    ) {
+    if (this.isCancelled && fileExistsSync(this.getCancellationFilePath())) {
       fs.unlinkSync(this.getCancellationFilePath());
       this.isCancelled = false;
     }

diff --git a/src/FilesRegister.ts b/src/FilesRegister.ts
@@ -2,11 +2,13 @@
 import * as ts from 'typescript'; // import for types alone
 // tslint:disable-next-line:no-implicit-dependencies
 import { RuleFailure } from 'tslint'; // import for types alone
+import { CLIEngine } from 'eslint'; // import for types alone
 
 export interface DataShape {
   source?: ts.SourceFile;
   linted: boolean;
-  lints: RuleFailure[];
+  tslints: RuleFailure[];
+  eslints: CLIEngine.LintReport[];
 }
 
 export class FilesRegister {