Fix --cache set only if with --write or no-different (#13016)
* Fixed typo * Return early if file does not exist * Added tests for cache * Added `mockWriteFileErrors` option to run helper * Added tests for write error * Remove files with differences from cache * Add `getCliArguments` utility function Refactor * Fix lint * Revert "Fix lint" This reverts commit 18be64b7c42cd669496d59ff64bd534b8e03c3df. * Revert "Add `getCliArguments` utility function" This reverts commit d653fe10a92390102ae97cb43b2efcbb5c3c9140. * Refactor with `cliArguments` variable Co-authored-by: Sosuke Suzuki <sosuke.suzuki@dr-ubie.com>
This commit is contained in:
MillyandSosuke Suzuki authored and GitHub committed
1 parent
f38111fec6
commit
ca246aface
4 files changed
+222
-70
No files matched your search
@@ -40,7 +40,7 @@ function getMetadataFromFileDescriptor(fileDescriptor) {
|
||||
|
||||
class FormatResultsCache {
|
||||
/**
|
||||
* @param {string} cacheFileLocation The path of cache file location. (default: `node_modules/.cache/prettier/prettier-cache`)
|
||||
* @param {string} cacheFileLocation The path of cache file location. (default: `node_modules/.cache/prettier/.prettier-cache`)
|
||||
* @param {string} cacheStrategy
|
||||
*/
|
||||
constructor(cacheFileLocation, cacheStrategy) {
|
||||
@@ -60,20 +60,17 @@ class FormatResultsCache {
|
||||
*/
|
||||
existsAvailableFormatResultsCache(filePath, options) {
|
||||
const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);
|
||||
const hashOfOptions = getHashOfOptions(options);
|
||||
const meta = getMetadataFromFileDescriptor(fileDescriptor);
|
||||
const changed =
|
||||
fileDescriptor.changed || meta.hashOfOptions !== hashOfOptions;
|
||||
|
||||
if (fileDescriptor.notFound) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (changed) {
|
||||
return false;
|
||||
}
|
||||
const hashOfOptions = getHashOfOptions(options);
|
||||
const meta = getMetadataFromFileDescriptor(fileDescriptor);
|
||||
const changed =
|
||||
fileDescriptor.changed || meta.hashOfOptions !== hashOfOptions;
|
||||
|
||||
return true;
|
||||
return !changed;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -88,6 +85,13 @@ class FormatResultsCache {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {string} filePath
|
||||
*/
|
||||
removeFormatResultsCache(filePath) {
|
||||
this.fileEntryCache.removeEntry(filePath);
|
||||
}
|
||||
|
||||
reconcile() {
|
||||
this.fileEntryCache.reconcile();
|
||||
}
|
||||
|
||||
+10
-4
@@ -407,9 +407,8 @@ async function formatFiles(context) {
|
||||
continue;
|
||||
}
|
||||
|
||||
formatResultsCache?.setFormatResultsCache(filename, options);
|
||||
|
||||
const isDifferent = output !== input;
|
||||
let shouldSetCache = !isDifferent;
|
||||
|
||||
if (printedFilename) {
|
||||
// Remove previously printed filename to log it with duration.
|
||||
@@ -433,14 +432,15 @@ async function formatFiles(context) {
|
||||
|
||||
try {
|
||||
await fs.writeFile(filename, output, "utf8");
|
||||
|
||||
// Set cache if format succeeds
|
||||
shouldSetCache = true;
|
||||
} catch (error) {
|
||||
/* istanbul ignore next */
|
||||
context.logger.error(
|
||||
`Unable to write file: ${filename}\n${error.message}`
|
||||
);
|
||||
|
||||
// Don't exit the process if one file failed
|
||||
/* istanbul ignore next */
|
||||
process.exitCode = 2;
|
||||
}
|
||||
} else if (!context.argv.check && !context.argv.listDifferent) {
|
||||
@@ -462,6 +462,12 @@ async function formatFiles(context) {
|
||||
writeOutput(context, result, options);
|
||||
}
|
||||
|
||||
if (shouldSetCache) {
|
||||
formatResultsCache?.setFormatResultsCache(filename, options);
|
||||
} else {
|
||||
formatResultsCache?.removeFormatResultsCache(filename);
|
||||
}
|
||||
|
||||
if (isDifferent) {
|
||||
if (context.argv.check) {
|
||||
context.logger.warn(filename);
|
||||
|
||||
@@ -96,7 +96,7 @@ describe("--cache option", () => {
|
||||
await expect(fs.stat(defaultCacheFile)).resolves.not.toThrowError();
|
||||
});
|
||||
|
||||
it("does'nt format when cache is available", async () => {
|
||||
it("doesn't format when cache is available", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--write",
|
||||
@@ -127,13 +127,14 @@ describe("--cache option", () => {
|
||||
});
|
||||
|
||||
it("re-formats when a file has been updated.", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
const cliArguments = [
|
||||
"--cache",
|
||||
"--write",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
]);
|
||||
];
|
||||
const { stdout: firstStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
@@ -144,13 +145,7 @@ describe("--cache option", () => {
|
||||
// Update `a.js`
|
||||
await fs.writeFile(path.join(dir, "a.js"), "const a = `a`;");
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--write",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
]);
|
||||
const { stdout: secondStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
// the cache of `b.js` is only available.
|
||||
expect.arrayContaining([
|
||||
@@ -161,13 +156,14 @@ describe("--cache option", () => {
|
||||
});
|
||||
|
||||
it("re-formats when timestamp has been updated", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
const cliArguments = [
|
||||
"--cache",
|
||||
"--write",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
]);
|
||||
];
|
||||
const { stdout: firstStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
@@ -179,13 +175,7 @@ describe("--cache option", () => {
|
||||
const time = new Date();
|
||||
await fs.utimes(path.join(dir, "a.js"), time, time);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--write",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
]);
|
||||
const { stdout: secondStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
// the cache of `b.js` is only available.
|
||||
expect.arrayContaining([
|
||||
@@ -227,6 +217,90 @@ describe("--cache option", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("re-formats after execution without write.", async () => {
|
||||
await runPrettier(dir, ["--cache", "--cache-strategy", "metadata", "."]);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--write",
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
]);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
expect.stringMatching(/^b\.js .+ms \(cached\)$/),
|
||||
])
|
||||
);
|
||||
});
|
||||
|
||||
it("re-formats when multiple cached files are updated.", async () => {
|
||||
const cliArguments = [
|
||||
"--write",
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
];
|
||||
await runPrettier(dir, cliArguments);
|
||||
|
||||
// Update `a.js` to unformatted
|
||||
await fs.writeFile(path.join(dir, "a.js"), "const a = `a`; ");
|
||||
|
||||
// Update `b.js` but still formatted
|
||||
const time = new Date();
|
||||
await fs.utimes(path.join(dir, "b.js"), time, time);
|
||||
|
||||
await runPrettier(dir, ["--cache", "--cache-strategy", "metadata", "."]);
|
||||
|
||||
const { stdout: thirdStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(thirdStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
expect.stringMatching(/^b\.js .+ms \(cached\)$/),
|
||||
])
|
||||
);
|
||||
});
|
||||
|
||||
it("doesn't cache files when write error.", async () => {
|
||||
const {
|
||||
stdout: firstStdout,
|
||||
stderr: firstStderr,
|
||||
status: firstStatus,
|
||||
} = await runPrettier(
|
||||
dir,
|
||||
["--write", "--cache", "--cache-strategy", "metadata", "."],
|
||||
{
|
||||
mockWriteFileErrors: {
|
||||
"a.js": "EACCES: permission denied (mock error)",
|
||||
},
|
||||
}
|
||||
);
|
||||
expect(firstStatus).toBe(2);
|
||||
expect(stripAnsi(firstStderr).split("\n").filter(Boolean)).toEqual([
|
||||
"[error] Unable to write file: a.js",
|
||||
"[error] EACCES: permission denied (mock error)",
|
||||
]);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
expect.stringMatching(/^b\.js .+ms$/),
|
||||
])
|
||||
);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--list-different",
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"metadata",
|
||||
".",
|
||||
]);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual([
|
||||
"a.js",
|
||||
]);
|
||||
});
|
||||
|
||||
it("removes cache file when run Prettier without `--cache` option", async () => {
|
||||
await runPrettier(dir, [
|
||||
"--cache",
|
||||
@@ -251,14 +325,15 @@ describe("--cache option", () => {
|
||||
await expect(fs.stat(defaultCacheFile)).resolves.not.toThrowError();
|
||||
});
|
||||
|
||||
it("does'nt format when cache is available", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
it("doesn't format when cache is available", async () => {
|
||||
const cliArguments = [
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
"--write",
|
||||
".",
|
||||
]);
|
||||
];
|
||||
const { stdout: firstStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
@@ -266,13 +341,7 @@ describe("--cache option", () => {
|
||||
])
|
||||
);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
"--write",
|
||||
".",
|
||||
]);
|
||||
const { stdout: secondStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms \(cached\)$/),
|
||||
@@ -282,13 +351,14 @@ describe("--cache option", () => {
|
||||
});
|
||||
|
||||
it("re-formats when a file has been updated.", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
const cliArguments = [
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
"--write",
|
||||
".",
|
||||
]);
|
||||
];
|
||||
const { stdout: firstStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
@@ -299,13 +369,7 @@ describe("--cache option", () => {
|
||||
// Update `a.js`
|
||||
await fs.writeFile(path.join(dir, "a.js"), "const a = `a`;");
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
"--write",
|
||||
".",
|
||||
]);
|
||||
const { stdout: secondStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
// the cache of `b.js` is only available.
|
||||
expect.arrayContaining([
|
||||
@@ -315,14 +379,15 @@ describe("--cache option", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("does'nt re-format when timestamp has been updated", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
it("doesn't re-format when timestamp has been updated", async () => {
|
||||
const cliArguments = [
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
"--write",
|
||||
".",
|
||||
]);
|
||||
];
|
||||
const { stdout: firstStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
@@ -334,13 +399,7 @@ describe("--cache option", () => {
|
||||
const time = new Date();
|
||||
await fs.utimes(path.join(dir, "a.js"), time, time);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
"--write",
|
||||
".",
|
||||
]);
|
||||
const { stdout: secondStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms \(cached\)$/),
|
||||
@@ -381,6 +440,90 @@ describe("--cache option", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("re-formats after execution without write.", async () => {
|
||||
await runPrettier(dir, ["--cache", "--cache-strategy", "content", "."]);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--write",
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
".",
|
||||
]);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
expect.stringMatching(/^b\.js .+ms \(cached\)$/),
|
||||
])
|
||||
);
|
||||
});
|
||||
|
||||
it("re-formats when multiple cached files are updated.", async () => {
|
||||
const cliArguments = [
|
||||
"--write",
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
".",
|
||||
];
|
||||
await runPrettier(dir, cliArguments);
|
||||
|
||||
// Update `a.js` to unformatted
|
||||
await fs.writeFile(path.join(dir, "a.js"), "const a = `a`; ");
|
||||
|
||||
// Update `b.js` but still formatted
|
||||
const time = new Date();
|
||||
await fs.utimes(path.join(dir, "b.js"), time, time);
|
||||
|
||||
await runPrettier(dir, ["--cache", "--cache-strategy", "content", "."]);
|
||||
|
||||
const { stdout: thirdStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(thirdStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
expect.stringMatching(/^b\.js .+ms \(cached\)$/),
|
||||
])
|
||||
);
|
||||
});
|
||||
|
||||
it("doesn't cache files when write error.", async () => {
|
||||
const {
|
||||
stdout: firstStdout,
|
||||
stderr: firstStderr,
|
||||
status: firstStatus,
|
||||
} = await runPrettier(
|
||||
dir,
|
||||
["--write", "--cache", "--cache-strategy", "content", "."],
|
||||
{
|
||||
mockWriteFileErrors: {
|
||||
"a.js": "EACCES: permission denied (mock error)",
|
||||
},
|
||||
}
|
||||
);
|
||||
expect(firstStatus).toBe(2);
|
||||
expect(stripAnsi(firstStderr).split("\n").filter(Boolean)).toEqual([
|
||||
"[error] Unable to write file: a.js",
|
||||
"[error] EACCES: permission denied (mock error)",
|
||||
]);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
expect.stringMatching(/^b\.js .+ms$/),
|
||||
])
|
||||
);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--list-different",
|
||||
"--cache",
|
||||
"--cache-strategy",
|
||||
"content",
|
||||
".",
|
||||
]);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual([
|
||||
"a.js",
|
||||
]);
|
||||
});
|
||||
|
||||
it("removes cache file when run Prettier without `--cache` option", async () => {
|
||||
await runPrettier(dir, ["--cache", "--write", "."]);
|
||||
await expect(fs.stat(defaultCacheFile)).resolves.not.toThrowError();
|
||||
@@ -437,13 +580,14 @@ describe("--cache option", () => {
|
||||
});
|
||||
|
||||
it("does'nt format when cache is available", async () => {
|
||||
const { stdout: firstStdout } = await runPrettier(dir, [
|
||||
const cliArguments = [
|
||||
"--cache",
|
||||
"--write",
|
||||
"--cache-location",
|
||||
nonDefaultCacheFileName,
|
||||
".",
|
||||
]);
|
||||
];
|
||||
const { stdout: firstStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(firstStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms$/),
|
||||
@@ -451,13 +595,7 @@ describe("--cache option", () => {
|
||||
])
|
||||
);
|
||||
|
||||
const { stdout: secondStdout } = await runPrettier(dir, [
|
||||
"--cache",
|
||||
"--write",
|
||||
"--cache-location",
|
||||
nonDefaultCacheFileName,
|
||||
".",
|
||||
]);
|
||||
const { stdout: secondStdout } = await runPrettier(dir, cliArguments);
|
||||
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.stringMatching(/^a\.js .+ms \(cached\)$/),
|
||||
|
||||
@@ -46,6 +46,10 @@ async function run(dir, args, options) {
|
||||
.spyOn(fs.promises, "writeFile")
|
||||
// eslint-disable-next-line require-await
|
||||
.mockImplementation(async (filename, content) => {
|
||||
const error = (options.mockWriteFileErrors || {})[filename];
|
||||
if (error) {
|
||||
throw new Error(error);
|
||||
}
|
||||
write.push({ filename, content });
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user