perf: use finishModules (#73)

* perf: use `finishModules`

* chore: update deps

* style: formatting

* test: add test to perf for hook `finishModules`

* test: add more tests

* refactor: finishModules

* refactor: finishModules

* perf: use for

declarations/ESLintError.d.ts

@@ -1,7 +1,8 @@
-export default class ESLintError extends WebpackError {
+export default class ESLintError {
   /**
    * @param {string=} messages
    */
   constructor(messages?: string | undefined);
+  name: string;
+  stack: string;
 }
-import { WebpackError } from 'webpack';

declarations/cjs.d.ts

@@ -1,2 +1,3 @@
-declare const _exports: typeof import('.').default;
+declare const _exports: typeof plugin.default;
 export = _exports;
+import plugin = require('.');

declarations/linter.d.ts

@@ -28,9 +28,7 @@ export type GenerateReport = (compilation: Compilation) => Promise<void>;
 export type Report = {
   errors?: ESLintError | undefined;
   warnings?: ESLintError | undefined;
-  generateReportAsset?:
-    | ((compilation: Compilation) => Promise<void>)
-    | undefined;
+  generateReportAsset?: GenerateReport | undefined;
 };
 export type Reporter = () => Promise<Report>;
 export type Linter = (files: string | string[]) => void;

package.json

@@ -46,40 +46,40 @@
     "webpack": "^4.0.0 || ^5.0.0"
   },
   "dependencies": {
-    "@types/eslint": "^7.2.4",
+    "@types/eslint": "^7.2.6",
     "arrify": "^2.0.1",
     "jest-worker": "^26.6.2",
     "micromatch": "^4.0.2",
     "schema-utils": "^3.0.0"
   },
   "devDependencies": {
-    "@babel/cli": "^7.12.1",
-    "@babel/core": "^7.12.3",
-    "@babel/preset-env": "^7.12.1",
+    "@babel/cli": "^7.12.10",
+    "@babel/core": "^7.12.10",
+    "@babel/preset-env": "^7.12.11",
     "@commitlint/cli": "^11.0.0",
     "@commitlint/config-conventional": "^11.0.0",
-    "@types/fs-extra": "^9.0.4",
+    "@types/fs-extra": "^9.0.6",
     "@types/micromatch": "^4.0.1",
-    "@types/webpack": "^4.41.25",
+    "@types/webpack": "^4.41.26",
     "@webpack-contrib/eslint-config-webpack": "^3.0.0",
     "babel-eslint": "^10.1.0",
     "babel-jest": "^26.6.3",
-    "chokidar": "^3.4.3",
-    "cross-env": "^7.0.2",
+    "chokidar": "^3.5.1",
+    "cross-env": "^7.0.3",
     "del": "^6.0.0",
     "del-cli": "^3.0.1",
-    "eslint": "^7.13.0",
-    "eslint-config-prettier": "^6.15.0",
+    "eslint": "^7.19.0",
+    "eslint-config-prettier": "^7.2.0",
     "eslint-plugin-import": "^2.22.1",
-    "fs-extra": "^9.0.1",
-    "husky": "^4.3.0",
+    "fs-extra": "^9.1.0",
+    "husky": "^4.3.8",
     "jest": "^26.6.3",
-    "lint-staged": "^10.5.1",
+    "lint-staged": "^10.5.3",
     "npm-run-all": "^4.1.5",
-    "prettier": "^2.1.2",
-    "standard-version": "^9.0.0",
-    "typescript": "^4.0.5",
-    "webpack": "^5.15.0"
+    "prettier": "^2.2.1",
+    "standard-version": "^9.1.0",
+    "typescript": "^4.1.3",
+    "webpack": "^5.19.0"
   },
   "keywords": [
     "eslint",

src/index.js

@@ -95,20 +95,31 @@ class ESLintWebpackPlugin {
         return;
       }
 
-      // @ts-ignore
-      const processModule = (module) => {
-        if (module.resource) {
-          const [file] = module.resource.split('?');
+      // Gather Files to lint
+      compilation.hooks.finishModules.tap(ESLINT_PLUGIN, (modules) => {
+        /** @type {string[]} */
+        const files = [];
 
-          if (file && isMatch(file, wanted) && !isMatch(file, exclude)) {
-            // Queue file for linting.
-            lint(file);
+        // @ts-ignore
+        for (const { resource } of modules) {
+          if (resource) {
+            const [file] = resource.split('?');
+
+            if (
+              file &&
+              !files.includes(file) &&
+              isMatch(file, wanted) &&
+              !isMatch(file, exclude)
+            ) {
+              files.push(file);
+            }
           }
         }
-      };
 
-      // Gather Files to lint
-      compilation.hooks.succeedModule.tap(ESLINT_PLUGIN, processModule);
+        if (files.length > 0) {
+          lint(files);
+        }
+      });
 
       // await and interpret results
       compilation.hooks.additionalAssets.tapPromise(

test/error.test.js

@@ -17,11 +17,13 @@ describe('error', () => {
   });
 
   it('should propagate eslint exceptions as errors', (done) => {
-    jest.mock('eslint', () => ({
-      ESLint: function ESLint() {
-        this.lintFiles = async () => Promise.reject(new Error('Oh no!'));
-      },
-    }));
+    jest.mock('eslint', () => {
+      return {
+        ESLint: function ESLint() {
+          this.lintFiles = async () => Promise.reject(new Error('Oh no!'));
+        },
+      };
+    });
 
     const compiler = pack('good', { threads: false });
 

test/eslint-lint.test.js

@@ -0,0 +1,59 @@
+import pack from './utils/pack';
+
+describe('eslint lint', () => {
+  const mockLintFiles = jest.fn().mockReturnValue([]);
+
+  beforeAll(() => {
+    jest.mock('eslint', () => {
+      return {
+        ESLint: function ESLint() {
+          this.lintFiles = mockLintFiles;
+        },
+      };
+    });
+  });
+
+  beforeEach(() => {
+    mockLintFiles.mockClear();
+  });
+
+  it('should lint one file', (done) => {
+    const compiler = pack('lint-one', { threads: false });
+
+    compiler.run((err) => {
+      const files = [expect.stringMatching('lint-one-entry.js')];
+      expect(mockLintFiles).toHaveBeenCalledWith(files);
+      expect(err).toBeNull();
+      done();
+    });
+  });
+
+  it('should lint two files', (done) => {
+    const compiler = pack('lint-two', { threads: false });
+
+    compiler.run((err) => {
+      const files = [
+        expect.stringMatching('lint-two-entry.js'),
+        expect.stringMatching('lint.js'),
+      ];
+      expect(mockLintFiles).toHaveBeenCalledWith(files);
+      expect(err).toBeNull();
+      done();
+    });
+  });
+
+  it('should lint more files', (done) => {
+    const compiler = pack('lint-more', { threads: false });
+
+    compiler.run((err) => {
+      const files = [
+        expect.stringMatching('lint-more-entry.js'),
+        expect.stringMatching('lint-more.js'),
+        expect.stringMatching('lint.js'),
+      ];
+      expect(mockLintFiles).toHaveBeenCalledWith(files);
+      expect(err).toBeNull();
+      done();
+    });
+  });
+});

test/fixtures/lint-more-entry.js

@@ -0,0 +1 @@
+require('./lint-more')

test/fixtures/lint-more.js

@@ -0,0 +1 @@
+require('./lint')

test/fixtures/lint-one-entry.js

@@ -0,0 +1 @@
+//

test/fixtures/lint-two-entry.js

@@ -0,0 +1,2 @@
+require('./lint')
+require('./lint.js?foo=bar')

test/fixtures/lint.js

@@ -0,0 +1,5 @@
+function test() {
+  return "value"
+}
+
+test()

test/threads.test.js

@@ -44,16 +44,18 @@ describe('Threading', () => {
     test('worker can start', async () => {
       const mockThread = { parentPort: { on: jest.fn() }, workerData: {} };
       const mockLintFiles = jest.fn();
-      jest.mock('eslint', () => ({
-        ESLint: Object.assign(
-          function ESLint() {
-            this.lintFiles = mockLintFiles;
-          },
-          {
-            outputFixes: jest.fn(),
-          }
-        ),
-      }));
+      jest.mock('eslint', () => {
+        return {
+          ESLint: Object.assign(
+            function ESLint() {
+              this.lintFiles = mockLintFiles;
+            },
+            {
+              outputFixes: jest.fn(),
+            }
+          ),
+        };
+      });
       jest.mock('worker_threads', () => mockThread);
       const { setup, lintFiles } = require('../src/worker');
 

test/utils/pack.js

@@ -2,6 +2,5 @@ import webpack from 'webpack';
 
 import conf from './conf';
 
-export default (context, pluginConf = {}, webpackConf = {}) => {
-  return webpack(conf(context, pluginConf, webpackConf));
-};
+export default (context, pluginConf = {}, webpackConf = {}) =>
+  webpack(conf(context, pluginConf, webpackConf));