Skip to content

Commit fcba5ce

Browse files
committed
refactor: simplify dirty file linting
Refactor LintDirtyModulesPlugin to only handle dirty file filtering. The refactor allows for easier testing without the need for mocks. Rename LintDirtyModulesPlugin to DirtyFileWatcher to make the name more descriptive of the new functionality. Refactor the tests for the simplified DirtyFileWatcher. Refactor the tests to use absolute paths because Webpack passes absolute paths in the compiler.fileTimestamps map. Refactor to only test system specific paths. The functionality of the requested change (webpack#11) requires using real paths for some tests which causes issues with testing both Windows and Unix paths on the same machine. As the CI runs on Windows, Mac and Linux paths will be tested on all platforms. Preparation for requested change in webpack#11.
1 parent 05c010b commit fcba5ce

File tree

4 files changed

+131
-88
lines changed

4 files changed

+131
-88
lines changed

src/LintDirtyModulesPlugin.js renamed to src/DirtyFileWatcher.js

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,32 @@
11
import { isMatch } from 'micromatch';
22

3-
import linter from './linter';
4-
5-
export default class LintDirtyModulesPlugin {
6-
constructor(compiler, options) {
7-
this.compiler = compiler;
8-
this.options = options;
3+
export default class DirtyFileWatcher {
4+
constructor({ files = [] }) {
5+
this.patterns = files;
96
this.startTime = Date.now();
107
this.prevTimestamps = {};
118
this.isFirstRun = true;
129
}
1310

14-
apply(compilation, callback) {
15-
const fileTimestamps = compilation.fileTimestamps || new Map();
16-
11+
getDirtyFiles({ fileTimestamps = new Map() }) {
1712
if (this.isFirstRun) {
1813
this.isFirstRun = false;
1914
this.prevTimestamps = fileTimestamps;
20-
callback();
21-
return;
15+
return [];
16+
}
17+
if (this.patterns.length <= 0 || fileTimestamps.length <= 0) {
18+
return [];
2219
}
2320

24-
const dirtyOptions = { ...this.options };
25-
const glob = dirtyOptions.files.join('|').replace(/\\/g, '/');
26-
const changedFiles = this.getChangedFiles(fileTimestamps, glob);
21+
const glob = this.patterns.join('|').replace(/\\/g, '/');
22+
const changedFiles = this.filterChangedFiles(fileTimestamps, glob);
2723

2824
this.prevTimestamps = fileTimestamps;
2925

30-
if (changedFiles.length) {
31-
dirtyOptions.files = changedFiles;
32-
linter(dirtyOptions, this.compiler, callback);
33-
} else {
34-
callback();
35-
}
26+
return changedFiles;
3627
}
3728

38-
getChangedFiles(fileTimestamps, glob) {
29+
filterChangedFiles(fileTimestamps, glob) {
3930
const getTimestamps = (fileSystemInfoEntry) => {
4031
return fileSystemInfoEntry && fileSystemInfoEntry.timestamp
4132
? fileSystemInfoEntry.timestamp

src/index.js

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { isAbsolute, join } from 'path';
33
import arrify from 'arrify';
44

55
import { getOptions } from './options';
6-
import LintDirtyModulesPlugin from './LintDirtyModulesPlugin';
6+
import DirtyFileWatcher from './DirtyFileWatcher';
77
import linter from './linter';
88

99
class ESLintWebpackPlugin {
@@ -22,20 +22,23 @@ class ESLintWebpackPlugin {
2222
const plugin = { name: this.constructor.name };
2323

2424
if (options.lintDirtyModulesOnly) {
25-
const lintDirty = new LintDirtyModulesPlugin(compiler, options);
25+
const dirtyFileWatcher = new DirtyFileWatcher(options);
2626

2727
/* istanbul ignore next */
28-
compiler.hooks.watchRun.tapAsync(plugin, (compilation, callback) => {
29-
lintDirty.apply(compilation, callback);
28+
compiler.hooks.watchRun.tapPromise(plugin, async (runCompiler) => {
29+
const files = dirtyFileWatcher.getDirtyFiles(runCompiler);
30+
if (files.length > 0) {
31+
await linter({ ...options, files }, runCompiler);
32+
}
3033
});
3134
} else {
32-
compiler.hooks.run.tapPromise(plugin, (compilation) => {
33-
return linter(options, compilation);
35+
compiler.hooks.run.tapPromise(plugin, (runCompiler) => {
36+
return linter(options, runCompiler);
3437
});
3538

3639
/* istanbul ignore next */
37-
compiler.hooks.watchRun.tapPromise(plugin, (compilation) => {
38-
return linter(options, compilation);
40+
compiler.hooks.watchRun.tapPromise(plugin, (runCompiler) => {
41+
return linter(options, runCompiler);
3942
});
4043
}
4144
}

test/LintDirtyModulesPlugin.test.js

Lines changed: 0 additions & 59 deletions
This file was deleted.

test/dirty-file-watcher.test.js

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { resolve } from 'path';
2+
3+
import DirtyFileWatcher from '../src/DirtyFileWatcher';
4+
5+
describe('dirty file watcher', () => {
6+
const fixtures = resolve(__dirname, 'fixtures');
7+
8+
it('returns empty on initial run', () => {
9+
const watcher = new DirtyFileWatcher({
10+
files: ['**/*'],
11+
});
12+
13+
const dirtyFiles = watcher.getDirtyFiles({
14+
fileTimestamps: new Map([
15+
['/hello/world.js', { timestamp: Number.MAX_VALUE }],
16+
]),
17+
});
18+
19+
expect(dirtyFiles).toHaveLength(0);
20+
});
21+
22+
it('returns changed files', () => {
23+
const watcher = new DirtyFileWatcher({
24+
files: ['**/*'],
25+
});
26+
27+
const insideFolder = resolve(fixtures, 'folder/unix.js');
28+
const changed = resolve(fixtures, 'changed.js');
29+
const unchagned = resolve(fixtures, 'unchanged.js');
30+
31+
watcher.getDirtyFiles({
32+
fileTimestamps: new Map([
33+
[insideFolder, { timestamp: 1 }],
34+
[changed, { timestamp: 1 }],
35+
[unchagned, { timestamp: 1 }],
36+
]),
37+
});
38+
const dirtyFiles = watcher.getDirtyFiles({
39+
fileTimestamps: new Map([
40+
[insideFolder, { timestamp: 2 }],
41+
[changed, { timestamp: 2 }],
42+
[unchagned, { timestamp: 1 }],
43+
]),
44+
});
45+
46+
expect(dirtyFiles).toHaveLength(2);
47+
expect(dirtyFiles).toContain(insideFolder);
48+
expect(dirtyFiles).toContain(changed);
49+
});
50+
51+
it('returns new files', () => {
52+
const watcher = new DirtyFileWatcher({
53+
files: ['**/*'],
54+
});
55+
56+
const created = resolve(fixtures, 'created.js');
57+
58+
watcher.getDirtyFiles({});
59+
const dirtyFiles = watcher.getDirtyFiles({
60+
fileTimestamps: new Map([[created, { timestamp: Number.MAX_VALUE }]]),
61+
});
62+
63+
expect(dirtyFiles).toHaveLength(1);
64+
expect(dirtyFiles).toContain(created);
65+
});
66+
67+
it('returns file if it does not have a timestamp', () => {
68+
const watcher = new DirtyFileWatcher({
69+
files: ['**/*'],
70+
});
71+
72+
const noTimestamp = resolve(fixtures, 'no-timestamp.js');
73+
74+
watcher.getDirtyFiles({});
75+
const dirtyFiles = watcher.getDirtyFiles({
76+
fileTimestamps: new Map([[noTimestamp]]),
77+
});
78+
79+
expect(dirtyFiles).toHaveLength(1);
80+
expect(dirtyFiles).toContain(noTimestamp);
81+
});
82+
83+
it('returns no files when options.files is empty', () => {
84+
const watcher = new DirtyFileWatcher({});
85+
86+
const changed = resolve(fixtures, 'changed.js');
87+
88+
watcher.getDirtyFiles({
89+
fileTimestamps: new Map([[changed, 1]]),
90+
});
91+
const dirtyFiles = watcher.getDirtyFiles({
92+
fileTimestamps: new Map([[changed, 2]]),
93+
});
94+
95+
expect(dirtyFiles).toHaveLength(0);
96+
});
97+
98+
it('does not throw when timestamps are unavailable', () => {
99+
const watcher = new DirtyFileWatcher({
100+
files: ['**/*'],
101+
});
102+
103+
watcher.getDirtyFiles({});
104+
const dirtyFiles = watcher.getDirtyFiles({});
105+
106+
expect(dirtyFiles).toHaveLength(0);
107+
});
108+
});

0 commit comments

Comments
 (0)