feat: use webpack's built-in watching instead of polling

Centralize files watching using webpack's `WatchFileSystem`. The default implementation of `NodeWatchFileSystem` doesn't fill our needs, so we've implemented `InclusiveNodeWatchFileSystem` BREAKING CHANGE: 🧨 Remove issue.scope option and use new watch architecture
TypeStrong · Oct 16, 2020 · fb22e47 · fb22e47
1 parent b964d05
commit fb22e47
Show file tree

Hide file tree

Showing 49 changed files with 1,164 additions and 836 deletions.
diff --git a/README.md b/README.md
@@ -62,12 +62,6 @@ module.exports = {
 };
 ```
 
-If you are using **TypeScript >= 3.8.0**, it's recommended to:
- * for `ts-loader` set `"importsNotUsedAsValues": "preserve"` [compiler option](https://www.typescriptlang.org/docs/handbook/compiler-options.html) in the [`tsconfig.json`](./examples/ts-loader/tsconfig.json)
- * for `babel-loader` set `"onlyRemoveTypeImports": true` [preset option](https://babeljs.io/docs/en/babel-preset-typescript#onlyremovetypeimports) in the [babel configuration](./examples/babel-loader/.babelrc.js)
-
-[Read more](#type-only-modules-watching) about type-only modules watching.
-
 > Examples how to configure it with [babel-loader](https://github.com/babel/babel-loader), [ts-loader](https://github.com/TypeStrong/ts-loader),
 > [eslint](https://github.com/eslint/eslint) and [Visual Studio Code](https://code.visualstudio.com/) are in the 
 > [**examples**](./examples) directory.
@@ -204,7 +198,7 @@ Options for the issues filtering (`issues` option object).
 ## Vue.js
 
 ⚠️ There are additional **constraints** regarding Vue.js Single File Component support: ⚠️
- * It requires **TypeScript >= 3.8.0** and `"importsNotUsedAsValues": "preserve"` option in the `tsconfig.json` (it's a limitation of the `transpileOnly` mode from `ts-loader`)
+ * It requires **TypeScript >= 3.8.0** (it's a limitation of the `transpileOnly` mode from `ts-loader`)
  * It doesn't work with the `build` mode (project references)
 
 To enable Vue.js support, follow these steps:
@@ -314,17 +308,6 @@ declare module "*.vue" {
 
 </details>
 
-## Type-Only modules watching
-
-At present `ts-loader` with `transpileOnly` mode and `babel-loader` will not add type-only files (files that contains only interfaces and/or types) 
-to the webpack dependencies set. Webpack watches only files that are in the dependencies set. This means that
-changes in type-only files will **not** trigger new compilation and therefore type-checker in watch mode.
-
-If you use **TypeScript >=3.8.0**, you can fix it: 
- * for `ts-loader` set `"importsNotUsedAsValues": "preserve"` [compiler option](https://www.typescriptlang.org/docs/handbook/compiler-options.html) in the [`tsconfig.json`](./examples/ts-loader/tsconfig.json)
- * for `babel-loader` set `"onlyRemoveTypeImports": true` [preset option](https://babeljs.io/docs/en/babel-preset-typescript#onlyremovetypeimports) in the [babel configuration](./examples/babel-loader/.babelrc.js)
-
-
 ## Plugin hooks
 
 This plugin provides some custom webpack hooks:

diff --git a/examples/babel-loader/.babelrc.js b/examples/babel-loader/.babelrc.js
@@ -1,11 +1,3 @@
 module.exports = {
-  presets: [
-    '@babel/preset-env',
-    [
-      '@babel/preset-typescript',
-      {
-        onlyRemoveTypeImports: true, // this is important for proper files watching
-      },
-    ],
-  ],
+  presets: ['@babel/preset-env', ['@babel/preset-typescript']],
 };
diff --git a/examples/ts-loader/tsconfig.json b/examples/ts-loader/tsconfig.json
@@ -10,8 +10,7 @@
     "strict": true,
     "baseUrl": "./src",
     "outDir": "./dist",
-    "forceConsistentCasingInFileNames": true,
-    "importsNotUsedAsValues": "preserve" // this is important for proper files watching
+    "forceConsistentCasingInFileNames": true
   },
   "include": ["./src"],
   "exclude": ["node_modules"]

diff --git a/package.json b/package.json
@@ -63,6 +63,7 @@
     "@babel/code-frame": "^7.8.3",
     "@types/json-schema": "^7.0.5",
     "chalk": "^4.1.0",
+    "chokidar": "^3.4.2",
     "cosmiconfig": "^6.0.0",
     "deepmerge": "^4.2.2",
     "fs-extra": "^9.0.0",

diff --git a/src/ForkTsCheckerWebpackPlugin.ts b/src/ForkTsCheckerWebpackPlugin.ts
@@ -16,10 +16,10 @@ import { assertEsLintSupport } from './eslint-reporter/assertEsLintSupport';
 import { createEsLintReporterRpcClient } from './eslint-reporter/reporter/EsLintReporterRpcClient';
 import { tapStartToConnectAndRunReporter } from './hooks/tapStartToConnectAndRunReporter';
 import { tapStopToDisconnectReporter } from './hooks/tapStopToDisconnectReporter';
-import { tapDoneToCollectRemoved } from './hooks/tapDoneToCollectRemoved';
 import { tapAfterCompileToAddDependencies } from './hooks/tapAfterCompileToAddDependencies';
 import { tapErrorToLogMessage } from './hooks/tapErrorToLogMessage';
 import { getForkTsCheckerWebpackPluginHooks } from './hooks/pluginHooks';
+import { tapAfterEnvironmentToPatchWatching } from './hooks/tapAfterEnvironmentToPatchWatching';
 
 class ForkTsCheckerWebpackPlugin implements webpack.Plugin {
   static readonly version: string = '{{VERSION}}'; // will be replaced by the @semantic-release/exec
@@ -62,9 +62,9 @@ class ForkTsCheckerWebpackPlugin implements webpack.Plugin {
     if (reporters.length) {
       const reporter = createAggregatedReporter(composeReporterRpcClients(reporters));
 
+      tapAfterEnvironmentToPatchWatching(compiler, state);
       tapStartToConnectAndRunReporter(compiler, reporter, configuration, state);
-      tapDoneToCollectRemoved(compiler, configuration, state);
-      tapAfterCompileToAddDependencies(compiler, configuration);
+      tapAfterCompileToAddDependencies(compiler, configuration, state);
       tapStopToDisconnectReporter(compiler, reporter, state);
       tapErrorToLogMessage(compiler, configuration);
     } else {

diff --git a/src/ForkTsCheckerWebpackPluginOptions.json b/src/ForkTsCheckerWebpackPluginOptions.json
@@ -261,11 +261,6 @@
         },
         "exclude": {
           "$ref": "#/definitions/IssuePredicateOption"
-        },
-        "scope": {
-          "type": "string",
-          "enum": ["all", "webpack"],
-          "description": "Defines issues scope to be reported. If 'webpack', reports errors only related to a given webpack compilation. Reports all errors otherwise."
         }
       }
     },

diff --git a/src/ForkTsCheckerWebpackPluginState.ts b/src/ForkTsCheckerWebpackPluginState.ts
@@ -1,18 +1,23 @@
-import { Report } from './reporter';
 import { Tap } from 'tapable';
+import { Dependencies, Report } from './reporter';
+import { Issue } from './issue';
 
 interface ForkTsCheckerWebpackPluginState {
-  report: Promise<Report | undefined>;
-  removedFiles: string[];
+  reportPromise: Promise<Report | undefined>;
+  issuesPromise: Promise<Issue[] | undefined>;
+  dependenciesPromise: Promise<Dependencies | undefined>;
+  lastDependencies: Dependencies | undefined;
   watching: boolean;
   initialized: boolean;
   webpackDevServerDoneTap: Tap | undefined;
 }
 
 function createForkTsCheckerWebpackPluginState(): ForkTsCheckerWebpackPluginState {
   return {
-    report: Promise.resolve([]),
-    removedFiles: [],
+    reportPromise: Promise.resolve(undefined),
+    issuesPromise: Promise.resolve(undefined),
+    dependenciesPromise: Promise.resolve(undefined),
+    lastDependencies: undefined,
     watching: false,
     initialized: false,
     webpackDevServerDoneTap: undefined,

diff --git a/src/eslint-reporter/reporter/EsLintReporter.ts b/src/eslint-reporter/reporter/EsLintReporter.ts
@@ -16,55 +16,69 @@ function createEsLintReporter(configuration: EsLintReporterConfiguration): Repor
 
   return {
     getReport: async ({ changedFiles = [], deletedFiles = [] }) => {
-      // cleanup old results
-      changedFiles.forEach((changedFile) => {
-        lintResults.delete(changedFile);
-      });
-      deletedFiles.forEach((removedFile) => {
-        lintResults.delete(removedFile);
-      });
+      return {
+        async getDependencies() {
+          return {
+            files: [],
+            dirs: [],
+            extensions: [],
+          };
+        },
+        async getIssues() {
+          // cleanup old results
+          changedFiles.forEach((changedFile) => {
+            lintResults.delete(changedFile);
+          });
+          deletedFiles.forEach((removedFile) => {
+            lintResults.delete(removedFile);
+          });
 
-      // get reports
-      const lintReports: LintReport[] = [];
+          // get reports
+          const lintReports: LintReport[] = [];
 
-      if (isInitialRun) {
-        lintReports.push(engine.executeOnFiles(includedFilesPatterns));
-        isInitialRun = false;
-      } else {
-        // we need to take care to not lint files that are not included by the configuration.
-        // the eslint engine will not exclude them automatically
-        const changedAndIncludedFiles = changedFiles.filter(
-          (changedFile) =>
-            includedFilesPatterns.some((includedFilesPattern) =>
-              minimatch(changedFile, includedFilesPattern)
-            ) &&
-            (configuration.options.extensions || []).some((extension) =>
-              changedFile.endsWith(extension)
-            ) &&
-            !engine.isPathIgnored(changedFile)
-        );
+          if (isInitialRun) {
+            lintReports.push(engine.executeOnFiles(includedFilesPatterns));
+            isInitialRun = false;
+          } else {
+            // we need to take care to not lint files that are not included by the configuration.
+            // the eslint engine will not exclude them automatically
+            const changedAndIncludedFiles = changedFiles.filter(
+              (changedFile) =>
+                includedFilesPatterns.some((includedFilesPattern) =>
+                  minimatch(changedFile, includedFilesPattern)
+                ) &&
+                (configuration.options.extensions || []).some((extension) =>
+                  changedFile.endsWith(extension)
+                ) &&
+                !engine.isPathIgnored(changedFile)
+            );
 
-        if (changedAndIncludedFiles.length) {
-          lintReports.push(engine.executeOnFiles(changedAndIncludedFiles));
-        }
-      }
+            if (changedAndIncludedFiles.length) {
+              lintReports.push(engine.executeOnFiles(changedAndIncludedFiles));
+            }
+          }
 
-      // output fixes if `fix` option is provided
-      if (configuration.options.fix) {
-        await Promise.all(lintReports.map((lintReport) => CLIEngine.outputFixes(lintReport)));
-      }
+          // output fixes if `fix` option is provided
+          if (configuration.options.fix) {
+            await Promise.all(lintReports.map((lintReport) => CLIEngine.outputFixes(lintReport)));
+          }
 
-      // store results
-      lintReports.forEach((lintReport) => {
-        lintReport.results.forEach((lintResult) => {
-          lintResults.set(lintResult.filePath, lintResult);
-        });
-      });
+          // store results
+          lintReports.forEach((lintReport) => {
+            lintReport.results.forEach((lintResult) => {
+              lintResults.set(lintResult.filePath, lintResult);
+            });
+          });
 
-      // get actual list of previous and current reports
-      const results = Array.from(lintResults.values());
+          // get actual list of previous and current reports
+          const results = Array.from(lintResults.values());
 
-      return createIssuesFromEsLintResults(results);
+          return createIssuesFromEsLintResults(results);
+        },
+        async close() {
+          // do nothing
+        },
+      };
     },
   };
 }

diff --git a/src/eslint-reporter/reporter/EsLintReporterRpcClient.ts b/src/eslint-reporter/reporter/EsLintReporterRpcClient.ts
@@ -1,4 +1,4 @@
-import * as path from 'path';
+import path from 'path';
 import { EsLintReporterConfiguration } from '../EsLintReporterConfiguration';
 import { createReporterRpcClient, ReporterRpcClient } from '../../reporter';
 import { createRpcIpcMessageChannel } from '../../rpc/rpc-ipc';

diff --git a/src/hooks/getChangedFiles.ts b/src/hooks/getChangedFiles.ts
@@ -1,21 +1,15 @@
-/* eslint-disable @typescript-eslint/no-explicit-any */
 import webpack from 'webpack';
 import path from 'path';
-import { getWatcher } from './getWatcher';
+import { CompilerWithWatchFileSystem } from '../watch/CompilerWithWatchFileSystem';
+import { InclusiveNodeWatchFileSystem } from '../watch/InclusiveNodeWatchFileSystem';
 
 function getChangedFiles(compiler: webpack.Compiler): string[] {
-  let changedFiles: string[] = [];
+  const watchFileSystem = (compiler as CompilerWithWatchFileSystem<InclusiveNodeWatchFileSystem>)
+    .watchFileSystem;
 
-  if ((compiler as any).modifiedFiles) {
-    // webpack 5+
-    changedFiles = Array.from((compiler as any).modifiedFiles);
-  } else {
-    const watcher = getWatcher(compiler);
-    // webpack 4
-    changedFiles = Object.keys((watcher && watcher.mtimes) || {});
-  }
-
-  return changedFiles.map((changedFile) => path.normalize(changedFile));
+  return watchFileSystem
+    ? Array.from(watchFileSystem.changedFiles).map((changedFile) => path.normalize(changedFile))
+    : [];
 }
 
 export { getChangedFiles };
diff --git a/src/hooks/getDeletedFiles.ts b/src/hooks/getDeletedFiles.ts
@@ -1,23 +1,15 @@
-/* eslint-disable @typescript-eslint/no-explicit-any */
 import webpack from 'webpack';
 import path from 'path';
-import { ForkTsCheckerWebpackPluginState } from '../ForkTsCheckerWebpackPluginState';
+import { CompilerWithWatchFileSystem } from '../watch/CompilerWithWatchFileSystem';
+import { InclusiveNodeWatchFileSystem } from '../watch/InclusiveNodeWatchFileSystem';
 
-function getDeletedFiles(
-  compiler: webpack.Compiler,
-  state: ForkTsCheckerWebpackPluginState
-): string[] {
-  let deletedFiles: string[] = [];
+function getDeletedFiles(compiler: webpack.Compiler): string[] {
+  const watchFileSystem = (compiler as CompilerWithWatchFileSystem<InclusiveNodeWatchFileSystem>)
+    .watchFileSystem;
 
-  if ((compiler as any).removedFiles) {
-    // webpack 5+
-    deletedFiles = Array.from((compiler as any).removedFiles || []);
-  } else {
-    // webpack 4
-    deletedFiles = [...state.removedFiles];
-  }
-
-  return deletedFiles.map((changedFile) => path.normalize(changedFile));
+  return watchFileSystem
+    ? Array.from(watchFileSystem.removedFiles).map((removeFile) => path.normalize(removeFile))
+    : [];
 }
 
 export { getDeletedFiles };
diff --git a/src/hooks/tapAfterCompileToAddDependencies.ts b/src/hooks/tapAfterCompileToAddDependencies.ts
@@ -1,15 +1,29 @@
 import webpack from 'webpack';
-import path from 'path';
 import { ForkTsCheckerWebpackPluginConfiguration } from '../ForkTsCheckerWebpackPluginConfiguration';
+import { ForkTsCheckerWebpackPluginState } from '../ForkTsCheckerWebpackPluginState';
 
 function tapAfterCompileToAddDependencies(
   compiler: webpack.Compiler,
-  configuration: ForkTsCheckerWebpackPluginConfiguration
+  configuration: ForkTsCheckerWebpackPluginConfiguration,
+  state: ForkTsCheckerWebpackPluginState
 ) {
-  compiler.hooks.afterCompile.tap('ForkTsCheckerWebpackPlugin', (compilation) => {
-    if (configuration.typescript.enabled) {
-      // watch tsconfig.json file
-      compilation.fileDependencies.add(path.normalize(configuration.typescript.configFile));
+  compiler.hooks.afterCompile.tapPromise('ForkTsCheckerWebpackPlugin', async (compilation) => {
+    if (compilation.compiler !== compiler) {
+      // run only for the compiler that the plugin was registered for
+      return;
+    }
+
+    const dependencies = await state.dependenciesPromise;
+
+    if (dependencies) {
+      state.lastDependencies = dependencies;
+
+      dependencies.files.forEach((file) => {
+        compilation.fileDependencies.add(file);
+      });
+      dependencies.dirs.forEach((dir) => {
+        compilation.contextDependencies.add(dir);
+      });
     }
   });
 }

diff --git a/src/hooks/tapAfterCompileToGetIssues.ts b/src/hooks/tapAfterCompileToGetIssues.ts
@@ -22,7 +22,7 @@ function tapAfterCompileToGetIssues(
     let issues: Issue[] | undefined = [];
 
     try {
-      issues = await state.report;
+      issues = await state.issuesPromise;
     } catch (error) {
       hooks.error.call(error, compilation);
       return;
@@ -33,13 +33,6 @@ function tapAfterCompileToGetIssues(
       return;
     }
 
-    if (configuration.issue.scope === 'webpack') {
-      // exclude issues that are related to files outside webpack compilation
-      issues = issues.filter(
-        (issue) => !issue.file || compilation.fileDependencies.has(path.normalize(issue.file))
-      );
-    }
-
     // filter list of issues by provided issue predicate
     issues = issues.filter(configuration.issue.predicate);
 

diff --git a/src/hooks/tapAfterEnvironmentToPatchWatching.ts b/src/hooks/tapAfterEnvironmentToPatchWatching.ts
@@ -0,0 +1,22 @@
+import webpack from 'webpack';
+import { ForkTsCheckerWebpackPluginState } from '../ForkTsCheckerWebpackPluginState';
+import { InclusiveNodeWatchFileSystem } from '../watch/InclusiveNodeWatchFileSystem';
+import { CompilerWithWatchFileSystem } from '../watch/CompilerWithWatchFileSystem';
+
+function tapAfterEnvironmentToPatchWatching(
+  compiler: webpack.Compiler,
+  state: ForkTsCheckerWebpackPluginState
+) {
+  compiler.hooks.afterEnvironment.tap('ForkTsCheckerWebpackPlugin', () => {
+    const watchFileSystem = (compiler as CompilerWithWatchFileSystem).watchFileSystem;
+    if (watchFileSystem) {
+      // wrap original watch file system
+      (compiler as CompilerWithWatchFileSystem).watchFileSystem = new InclusiveNodeWatchFileSystem(
+        watchFileSystem,
+        state
+      );
+    }
+  });
+}
+
+export { tapAfterEnvironmentToPatchWatching };