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

fix: lint files and create a global cache #79

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,47 @@ class ESLintWebpackPlugin {
const wanted = parseFoldersToGlobs(options.files, options.extensions);
const exclude = parseFoldersToGlobs(options.exclude, []);

/** @type {import('./linter').InvalidateLinterCache} */
let invalidateLinterCache;

// TODO: Remove on v3
// use `compiler.modifiedFiles` and `compiler.removedFiles`
// directly on hook finishModules
compiler.hooks.invalid.tap(ESLINT_PLUGIN, (invalidFile) => {
/* istanbul ignore else */
if (invalidateLinterCache) {
/** @type {string[]} */
const invalidFiles = [];

/* istanbul ignore else */
if (invalidFile) {
invalidFiles.push(invalidFile);
}

if (compiler.modifiedFiles) {
invalidFiles.push(...compiler.modifiedFiles);
}

if (compiler.removedFiles) {
invalidFiles.push(...compiler.removedFiles);
}

invalidateLinterCache(invalidFiles);
}
});

compiler.hooks.thisCompilation.tap(ESLINT_PLUGIN, (compilation) => {
/** @type {import('./linter').Linter} */
let lint;
/** @type {import('./linter').Reporter} */
let report;

try {
({ lint, report } = linter(this.key, options, compilation));
({ lint, report, invalidateLinterCache } = linter(
this.key,
options,
compilation
));
} catch (e) {
compilation.errors.push(e);
return;
Expand Down Expand Up @@ -122,12 +155,7 @@ class ESLintWebpackPlugin {
});

// await and interpret results
compilation.hooks.additionalAssets.tapPromise(
ESLINT_PLUGIN,
processResults
);

async function processResults() {
compilation.hooks.additionalAssets.tapPromise(ESLINT_PLUGIN, async () => {
const { errors, warnings, generateReportAsset } = await report();

if (warnings && !options.failOnWarning) {
Expand All @@ -146,7 +174,7 @@ class ESLintWebpackPlugin {
if (generateReportAsset) {
await generateReportAsset(compilation);
}
}
});
});
}

Expand Down
90 changes: 45 additions & 45 deletions src/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,27 @@ import { dirname, isAbsolute, join } from 'path';
import ESLintError from './ESLintError';
import getESLint from './getESLint';

/** @type {Map<string, LintResult>} */
const linterCache = new Map();

/** @typedef {import('eslint').ESLint} ESLint */
/** @typedef {import('eslint').ESLint.Formatter} Formatter */
/** @typedef {import('eslint').ESLint.LintResult} LintResult */
/** @typedef {import('webpack').Compiler} Compiler */
/** @typedef {import('webpack').Compilation} Compilation */
/** @typedef {import('webpack-sources').Source} Source */
/** @typedef {import('./options').Options} Options */
/** @typedef {import('./options').FormatterFunction} FormatterFunction */
/** @typedef {(compilation: Compilation) => Promise<void>} GenerateReport */
/** @typedef {{errors?: ESLintError, warnings?: ESLintError, generateReportAsset?: GenerateReport}} Report */
/** @typedef {() => Promise<Report>} Reporter */
/** @typedef {(files: string|string[]) => void} Linter */
/** @typedef {{[files: string]: LintResult}} LintResultMap */

/** @type {WeakMap<Compiler, LintResultMap>} */
const resultStorage = new WeakMap();
/** @typedef {(files: string[]) => void} Linter */
/** @typedef {(filesChanged: string[]) => void} InvalidateLinterCache */

/**
* @param {string|undefined} key
* @param {Options} options
* @param {Compilation} compilation
* @returns {{lint: Linter, report: Reporter}}
* @returns {{lint: Linter, report: Reporter, invalidateLinterCache: InvalidateLinterCache}}
*/
export default function linter(key, options, compilation) {
/** @type {ESLint} */
Expand All @@ -39,8 +38,6 @@ export default function linter(key, options, compilation) {
/** @type {Promise<LintResult[]>[]} */
const rawResults = [];

const crossRunResultStorage = getResultStorage(compilation);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cache held all results that had been computed... invalidations were supposed to be deleted from this and re-run.. so we don't recompute results for files that did not change... but still show them if they had issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related #83


try {
({ eslint, lintFiles, cleanup } = getESLint(key, options));
} catch (e) {
Expand All @@ -50,39 +47,52 @@ export default function linter(key, options, compilation) {
return {
lint,
report,
invalidateLinterCache,
};

/**
* @param {string | string[]} files
* @param {string[]} files
*/
function lint(files) {
for (const file of asList(files)) {
delete crossRunResultStorage[file];
/** @type {string[]} */
const filesToLint = [];

/** @type {LintResult[]} */
const cacheResults = [];

for (const file of files) {
const cacheResult = linterCache.get(file);
if (cacheResult) cacheResults.push(cacheResult);
else filesToLint.push(file);
}
rawResults.push(
lintFiles(files).catch((e) => {
compilation.errors.push(e);
return [];
})
);

const resultsPromise = new Promise((resolve) => {
lintFiles(filesToLint)
.then((lintResults) => {
// add lintResults to cache
for (const lintResult of lintResults)
linterCache.set(lintResult.filePath, lintResult);
resolve([...lintResults, ...cacheResults]);
})
.catch((e) => {
compilation.errors.push(e);
resolve([]);
});
});

rawResults.push(resultsPromise);
}

async function report() {
// Filter out ignored files.
let results = await removeIgnoredWarnings(
const results = await removeIgnoredWarnings(
eslint,
// Get the current results, resetting the rawResults to empty
await flatten(rawResults.splice(0, rawResults.length))
);

await cleanup();

for (const result of results) {
crossRunResultStorage[result.filePath] = result;
}

results = Object.values(crossRunResultStorage);

// do not analyze if there are no results or eslint config
if (!results || results.length < 1) {
return {};
Expand Down Expand Up @@ -143,6 +153,16 @@ export default function linter(key, options, compilation) {
await save(filePath, content);
}
}

/**
* @param {string[]} filesChanged
*/
function invalidateLinterCache(filesChanged) {
// remove modified files
for (const fileChanged of filesChanged) {
linterCache.delete(fileChanged);
}
}
}

/**
Expand Down Expand Up @@ -289,23 +309,3 @@ async function flatten(results) {
const flat = (acc, list) => [...acc, ...list];
return (await Promise.all(results)).reduce(flat, []);
}

/**
* @param {Compilation} compilation
* @returns {LintResultMap}
*/
function getResultStorage({ compiler }) {
let storage = resultStorage.get(compiler);
if (!storage) {
resultStorage.set(compiler, (storage = {}));
}
return storage;
}

/**
* @param {string | string[]} x
*/
function asList(x) {
/* istanbul ignore next */
return Array.isArray(x) ? x : [x];
}
6 changes: 1 addition & 5 deletions test/watch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,7 @@ describe('watch', () => {
function finish(err, stats) {
expect(err).toBeNull();
expect(stats.hasWarnings()).toBe(false);
const { errors } = stats.compilation;
const [{ message }] = errors;
expect(stats.hasErrors()).toBe(true);
expect(message).toEqual(expect.stringMatching('prefer-const'));
expect(message).toEqual(expect.stringMatching('\\(2 errors,'));
expect(stats.hasErrors()).toBe(false);
done();
}
});
Expand Down