Skip to content

Commit bb47dc6

Browse files
mdjermanovicDMartens
mdjermanovic
and
DMartens
authored
fix: update dependency file-entry-cache to v11 (#20801)
* chore: update dependency file-entry-cache to v11 * update lint-result-cache tests * update eslint tests * update eslint tests * fix content strategy * handle invalid JSON cache files * add test that fails with file-entry-cache v11 * add another test that fails with file-entry-cache v11 * add another test that fails with file-entry-cache v11 * add more tests that fails with file-entry-cache v11 * use file-entry-cache 11.1.5 * fileEntryCache.create is no longer a getter * fix flaky test * remove handling invalid JSON * delete legacy properties * update range to exclude v11.1.6 * change variable name in tests * use format string in debug call Co-authored-by: fnx <966276+DMartens@users.noreply.github.com> --------- Co-authored-by: fnx <966276+DMartens@users.noreply.github.com>
1 parent 427ac0a commit bb47dc6

4 files changed

Lines changed: 574 additions & 45 deletions

File tree

‎lib/cli-engine/lint-result-cache.js‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//-----------------------------------------------------------------------------
1010

1111
const fs = require("node:fs");
12+
const path = require("node:path");
1213
const fileEntryCache = require("file-entry-cache");
1314
const stringify = require("json-stable-stringify-without-jsonify");
1415
const pkg = require("../../package.json");
@@ -85,15 +86,30 @@ class LintResultCache {
8586

8687
debug("Caching results to %s", cacheFileLocation);
8788

88-
const useChecksum = cacheStrategy === "content";
89+
const useCheckSum = cacheStrategy === "content";
8990

9091
debug('Using "%s" strategy to detect changes', cacheStrategy);
9192

93+
/*
94+
* useModifiedTime: If `true` (default), use mtime and size to determine if the file has changed.
95+
* It corresponds to the "metadata" cache strategy.
96+
* useCheckSum: If `true`, use hash of the content to determine if the file has changed.
97+
* It corresponds to the "content" cache strategy.
98+
*
99+
* For the "content" cache strategy, it is important to set useModifiedTime to `false`.
100+
* Otherwise, file-entry-cache would use _both_ checks to determine if the file has changed,
101+
* which would defeat the purpose of this cache strategy (use cases where the modification time
102+
* of files changes even if their contents have not, e.g., after `git clone`).
103+
*/
92104
this.fileEntryCache = fileEntryCache.create(
93-
cacheFileLocation,
94-
void 0,
95-
useChecksum,
105+
path.basename(cacheFileLocation),
106+
path.dirname(cacheFileLocation),
107+
{
108+
useModifiedTime: !useCheckSum,
109+
useCheckSum,
110+
},
96111
);
112+
97113
this.cacheFileLocation = cacheFileLocation;
98114
}
99115

@@ -157,17 +173,22 @@ class LintResultCache {
157173
return null;
158174
}
159175

176+
if (!fileDescriptor.changed && !fileDescriptor.meta.data) {
177+
debug("Legacy cache entry found: %s", filePath);
178+
return null;
179+
}
180+
160181
const hashOfConfig = hashOfConfigFor(config);
161182
const changed =
162183
fileDescriptor.changed ||
163-
fileDescriptor.meta.hashOfConfig !== hashOfConfig;
184+
fileDescriptor.meta.data.hashOfConfig !== hashOfConfig;
164185

165186
if (changed) {
166187
debug("Cache entry not found or no longer valid: %s", filePath);
167188
return null;
168189
}
169190

170-
return fileDescriptor.meta.results;
191+
return fileDescriptor.meta.data.results;
171192
}
172193

173194
/**
@@ -203,8 +224,16 @@ class LintResultCache {
203224
resultToSerialize.source = null;
204225
}
205226

206-
fileDescriptor.meta.results = resultToSerialize;
207-
fileDescriptor.meta.hashOfConfig = hashOfConfigFor(config);
227+
// remove legacy properties set by a previous version of ESLint, if present
228+
if (fileDescriptor.meta.hashOfConfig) {
229+
delete fileDescriptor.meta.results;
230+
delete fileDescriptor.meta.hashOfConfig;
231+
}
232+
233+
fileDescriptor.meta.data = {
234+
results: resultToSerialize,
235+
hashOfConfig: hashOfConfigFor(config),
236+
};
208237
}
209238
}
210239

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@
152152
"esquery": "^1.7.0",
153153
"esutils": "^2.0.2",
154154
"fast-deep-equal": "^3.1.3",
155-
"file-entry-cache": "^8.0.0",
155+
"file-entry-cache": "11.1.5 || >11.1.6 <12",
156156
"find-up": "^5.0.0",
157157
"glob-parent": "^6.0.2",
158158
"ignore": "^5.2.0",

‎tests/lib/cli-engine/lint-result-cache.js‎

Lines changed: 186 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
const assert = require("chai").assert,
1212
{ ESLint } = require("../../../lib/eslint"),
1313
fs = require("node:fs"),
14+
os = require("node:os"),
1415
path = require("node:path"),
1516
proxyquire = require("proxyquire"),
16-
sinon = require("sinon");
17+
sinon = require("sinon"),
18+
OriginalLintResultCache = require("../../../lib/cli-engine/lint-result-cache");
1719

1820
//-----------------------------------------------------------------------------
1921
// Tests
@@ -137,12 +139,14 @@ describe("LintResultCache", () => {
137139
beforeEach(() => {
138140
cacheEntry = {
139141
meta: {
140-
// Serialized results will have null source
141-
results: Object.assign({}, fakeErrorResults, {
142-
source: null,
143-
}),
142+
data: {
143+
// Serialized results will have null source
144+
results: Object.assign({}, fakeErrorResults, {
145+
source: null,
146+
}),
144147

145-
hashOfConfig,
148+
hashOfConfig,
149+
},
146150
},
147151
};
148152

@@ -276,6 +280,170 @@ describe("LintResultCache", () => {
276280
);
277281
});
278282
});
283+
284+
describe("When called multiple times for the same file", () => {
285+
let testDir;
286+
let testFile;
287+
let anotherFile;
288+
let cacheFile;
289+
let lintResult, anotherLintResult;
290+
let config;
291+
292+
beforeEach(async () => {
293+
testDir = await fs.promises.mkdtemp(
294+
path.join(os.tmpdir(), "eslint-cache-"),
295+
);
296+
297+
testFile = path.resolve(testDir, "file.js");
298+
await fs.promises.writeFile(testFile, "foo;");
299+
300+
anotherFile = path.resolve(testDir, "anotherfile.js");
301+
await fs.promises.writeFile(anotherFile, "bar;");
302+
303+
const testEslint = new ESLint({
304+
cwd: testDir,
305+
cache: false,
306+
overrideConfigFile: true,
307+
});
308+
309+
[lintResult] = await testEslint.lintFiles([testFile]);
310+
assert(lintResult, "Lint result should have been created");
311+
312+
[anotherLintResult] = await testEslint.lintFiles([anotherFile]);
313+
assert(
314+
anotherLintResult,
315+
"Another lint result should have been created",
316+
);
317+
318+
config = await testEslint.calculateConfigForFile(testFile);
319+
320+
cacheFile = path.resolve(testDir, ".eslintcache");
321+
});
322+
323+
afterEach(async () => {
324+
await fs.promises.rm(testDir, {
325+
recursive: true,
326+
force: true,
327+
});
328+
testDir = void 0;
329+
});
330+
331+
["metadata", "content"].forEach(cacheStrategy => {
332+
it(`should return null for a file when cache file hasn't been created yet when cacheStrategy is "${cacheStrategy}"`, async () => {
333+
const testLintResultCache = new OriginalLintResultCache(
334+
cacheFile,
335+
cacheStrategy,
336+
);
337+
338+
// Get results from cache multiple times
339+
assert.isNull(
340+
testLintResultCache.getCachedLintResults(
341+
testFile,
342+
config,
343+
),
344+
);
345+
assert.isNull(
346+
testLintResultCache.getCachedLintResults(
347+
testFile,
348+
config,
349+
),
350+
);
351+
});
352+
353+
it(`should return null for a file that hasn't already been cached when cacheStrategy is "${cacheStrategy}"`, async () => {
354+
const initLintResultCache = new OriginalLintResultCache(
355+
cacheFile,
356+
cacheStrategy,
357+
);
358+
359+
initLintResultCache.setCachedLintResults(
360+
anotherFile,
361+
config,
362+
anotherLintResult,
363+
);
364+
initLintResultCache.reconcile();
365+
366+
const checkLintResultCache = new OriginalLintResultCache(
367+
cacheFile,
368+
cacheStrategy,
369+
);
370+
assert(
371+
checkLintResultCache.getCachedLintResults(
372+
anotherFile,
373+
config,
374+
),
375+
"Cache entry for another file should have be valid",
376+
);
377+
378+
const testLintResultCache = new OriginalLintResultCache(
379+
cacheFile,
380+
cacheStrategy,
381+
);
382+
383+
// Get results from cache multiple times
384+
assert.isNull(
385+
testLintResultCache.getCachedLintResults(
386+
testFile,
387+
config,
388+
),
389+
);
390+
assert.isNull(
391+
testLintResultCache.getCachedLintResults(
392+
testFile,
393+
config,
394+
),
395+
);
396+
});
397+
398+
it(`should return null for a file that has already been cached but modified when cacheStrategy is "${cacheStrategy}"`, async () => {
399+
const initLintResultCache = new OriginalLintResultCache(
400+
cacheFile,
401+
cacheStrategy,
402+
);
403+
404+
initLintResultCache.setCachedLintResults(
405+
testFile,
406+
config,
407+
lintResult,
408+
);
409+
initLintResultCache.reconcile();
410+
411+
const checkLintResultCache = new OriginalLintResultCache(
412+
cacheFile,
413+
cacheStrategy,
414+
);
415+
assert(
416+
checkLintResultCache.getCachedLintResults(
417+
testFile,
418+
config,
419+
),
420+
"Cache entry should have initially be valid",
421+
);
422+
423+
// Modify file. This should make cache entry invalid for all cache strategies.
424+
await fs.promises.writeFile(testFile, "barbaz;");
425+
426+
const testLintResultCache = new OriginalLintResultCache(
427+
cacheFile,
428+
cacheStrategy,
429+
);
430+
431+
// Get results from cache multiple times
432+
assert.isNull(
433+
testLintResultCache.getCachedLintResults(
434+
testFile,
435+
config,
436+
),
437+
);
438+
assert.isNull(
439+
testLintResultCache.getCachedLintResults(
440+
testFile,
441+
config,
442+
),
443+
);
444+
});
445+
});
446+
});
279447
});
280448

281449
describe("setCachedLintResults", () => {
@@ -321,8 +489,7 @@ describe("LintResultCache", () => {
321489
fakeErrorResultsAutofix,
322490
);
323491

324-
assert.notProperty(cacheEntry.meta, "results");
325-
assert.notProperty(cacheEntry.meta, "hashOfConfig");
492+
assert.notProperty(cacheEntry.meta, "data");
326493
});
327494
});
328495

@@ -338,8 +505,7 @@ describe("LintResultCache", () => {
338505
fakeErrorResults,
339506
);
340507

341-
assert.notProperty(cacheEntry.meta, "results");
342-
assert.notProperty(cacheEntry.meta, "hashOfConfig");
508+
assert.notProperty(cacheEntry.meta, "data");
343509
});
344510
});
345511

@@ -353,7 +519,10 @@ describe("LintResultCache", () => {
353519
});
354520

355521
it("stores hash of config in file entry", () => {
356-
assert.strictEqual(cacheEntry.meta.hashOfConfig, hashOfConfig);
522+
assert.strictEqual(
523+
cacheEntry.meta.data.hashOfConfig,
524+
hashOfConfig,
525+
);
357526
});
358527

359528
it("stores results (except source) in file entry", () => {
@@ -366,7 +535,7 @@ describe("LintResultCache", () => {
366535
);
367536

368537
assert.deepStrictEqual(
369-
cacheEntry.meta.results,
538+
cacheEntry.meta.data.results,
370539
expectedCachedResults,
371540
);
372541
});
@@ -382,7 +551,10 @@ describe("LintResultCache", () => {
382551
});
383552

384553
it("stores hash of config in file entry", () => {
385-
assert.strictEqual(cacheEntry.meta.hashOfConfig, hashOfConfig);
554+
assert.strictEqual(
555+
cacheEntry.meta.data.hashOfConfig,
556+
hashOfConfig,
557+
);
386558
});
387559

388560
it("stores results (except source) in file entry", () => {
@@ -395,7 +567,7 @@ describe("LintResultCache", () => {
395567
);
396568

397569
assert.deepStrictEqual(
398-
cacheEntry.meta.results,
570+
cacheEntry.meta.data.results,
399571
expectedCachedResults,
400572
);
401573
});

0 commit comments

Comments
 (0)