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

Add --cache-location option #13019

Merged
merged 19 commits into from
Jul 31, 2022
Merged
9 changes: 9 additions & 0 deletions changelog_unreleased/cli/13019.md
@@ -0,0 +1,9 @@
#### Add `--cache-location` option (#13019 by @sosukesuzuki)

Path to the cache file location used by `--cache` flag. If you don't explicit `--cache-location`, Prettier saves cache file at `./node_modules/.cache/prettier/.prettier-cache`.

If a file path is passed, that file is used as the cache file.

```bash
prettier --write --cache --cache-location=my_cache_file src
```
10 changes: 10 additions & 0 deletions docs/cli.md
Expand Up @@ -225,6 +225,16 @@ Also, since the cache file is stored in `./node_modules/.cache/prettier/.prettie

> Plugins version and implementation are not used as cache keys. We recommend that you delete the cache when updating plugins.

## `--cache-location`

Path to the cache file location used by `--cache` flag. If you don't explicit `--cache-location`, Prettier saves cache file at `./node_modules/.cache/prettier/.prettier-cache`.

If a file path is passed, that file is used as the cache file.

```bash
prettier --write --cache --cache-location=my_cache_file src
```

## `--cache-strategy`

Strategy for the cache to use for detecting changed files. Can be either `metadata` or `content`.
Expand Down
4 changes: 4 additions & 0 deletions src/cli/constant.js
Expand Up @@ -76,6 +76,10 @@ const options = {
description: "Only format changed files. Cannot use with --stdin-filepath.",
type: "boolean",
},
"cache-location": {
description: "Path to the cache file.",
type: "path",
},
"cache-strategy": {
choices: [
{
Expand Down
38 changes: 35 additions & 3 deletions src/cli/find-cache-file.js
@@ -1,19 +1,51 @@
"use strict";

const fs = require("fs").promises;
const os = require("os");
const path = require("path");
const findCacheDir = require("find-cache-dir");
const { statSafe, isJson } = require("./utils.js");

/**
* Find default cache file (`./node_modules/.cache/prettier/.prettier-cache`) using https://github.com/avajs/find-cache-dir
*
* @returns {string}
*/
function findCacheFile() {
function findDefaultCacheFile() {
const cacheDir =
findCacheDir({ name: "prettier", create: true }) || os.tmpdir();
const cacheFilePath = path.join(cacheDir, ".prettier-cache");
return cacheFilePath;
}

async function findCacheFileFromOption(cacheLocation) {
const cacheFile = path.resolve(cacheLocation);

const stat = await statSafe(cacheFile);
if (stat) {
if (stat.isDirectory()) {
throw new Error(
`Resolved --cache-location '${cacheFile}' is a directory`
);
}

const data = await fs.readFile(cacheFile, "utf8");
if (!isJson(data)) {
throw new Error(`'${cacheFile}' isn't a valid JSON file`);
}
}

return cacheFile;
}

/**
* @param {string | undefined} cacheLocation
* @returns {Promise<string>}
*/
async function findCacheFile(cacheLocation) {
if (!cacheLocation) {
return findDefaultCacheFile();
}
const cacheFile = await findCacheFileFromOption(cacheLocation);
return cacheFile;
}

module.exports = findCacheFile;
10 changes: 6 additions & 4 deletions src/cli/format.js
Expand Up @@ -299,7 +299,7 @@ async function formatFiles(context) {
}

let formatResultsCache;
const cacheFilePath = findCacheFile();
const cacheFilePath = await findCacheFile(context.argv.cacheLocation);
if (context.argv.cache) {
formatResultsCache = new FormatResultsCache(
cacheFilePath,
Expand All @@ -312,9 +312,11 @@ async function formatFiles(context) {
);
process.exit(2);
}
const stat = await statSafe(cacheFilePath);
if (stat) {
await fs.unlink(cacheFilePath);
if (!context.argv.cacheLocation) {
const stat = await statSafe(cacheFilePath);
if (stat) {
await fs.unlink(cacheFilePath);
}
Copy link
Member

@fisker fisker Jul 10, 2022

Choose a reason for hiding this comment

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

If I didn't get this wrong, this line is going to remove default cache file when running Prettier without --cache, but I don't think we should. People may use Prettier without --cache temporarily.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a direct implementation of ESLint's behavior and has nothing to do with --cache-location. It will be fixed in another PR.

}
}

Expand Down
15 changes: 14 additions & 1 deletion src/cli/utils.js
Expand Up @@ -67,4 +67,17 @@ async function statSafe(filePath) {
}
}

module.exports = { printToScreen, groupBy, pick, createHash, statSafe };
/**
* @param {string} value
* @returns {boolean}
*/
function isJson(value) {
try {
JSON.parse(value);
return true;
} catch {
return false;
}
}

module.exports = { printToScreen, groupBy, pick, createHash, statSafe, isJson };
2 changes: 2 additions & 0 deletions tests/integration/__tests__/__snapshots__/early-exit.js.snap
Expand Up @@ -141,6 +141,7 @@ Other options:

--cache Only format changed files. Cannot use with --stdin-filepath.
Defaults to false.
--cache-location <path> Path to the cache file.
--cache-strategy <metadata|content>
Strategy for the cache to use for detecting changed files.
--no-color Do not colorize error messages.
Expand Down Expand Up @@ -315,6 +316,7 @@ Other options:

--cache Only format changed files. Cannot use with --stdin-filepath.
Defaults to false.
--cache-location <path> Path to the cache file.
--cache-strategy <metadata|content>
Strategy for the cache to use for detecting changed files.
--no-color Do not colorize error messages.
Expand Down
11 changes: 11 additions & 0 deletions tests/integration/__tests__/__snapshots__/help-options.js.snap
Expand Up @@ -57,6 +57,17 @@ Default: false

exports[`show detailed usage with --help cache (write) 1`] = `[]`;

exports[`show detailed usage with --help cache-location (stderr) 1`] = `""`;

exports[`show detailed usage with --help cache-location (stdout) 1`] = `
"--cache-location <path>

Path to the cache file.
"
`;

exports[`show detailed usage with --help cache-location (write) 1`] = `[]`;

exports[`show detailed usage with --help cache-strategy (stderr) 1`] = `""`;

exports[`show detailed usage with --help cache-strategy (stdout) 1`] = `
Expand Down
97 changes: 97 additions & 0 deletions tests/integration/__tests__/cache.js
Expand Up @@ -18,6 +18,9 @@ describe("--cache option", () => {
"node_modules/.cache/prettier/.prettier-cache"
);

const nonDefaultCacheFileName = ".non-default-cache-file";
const nonDefaultCacheFilePath = path.join(dir, nonDefaultCacheFileName);

let contentA;
let contentB;

Expand All @@ -28,6 +31,7 @@ describe("--cache option", () => {

afterEach(async () => {
rimraf.sync(path.join(dir, "node_modules"));
rimraf.sync(nonDefaultCacheFilePath);
await fs.writeFile(path.join(dir, "a.js"), contentA);
await fs.writeFile(path.join(dir, "b.js"), contentB);
});
Expand Down Expand Up @@ -68,6 +72,20 @@ describe("--cache option", () => {
);
});

it("throws error when `--cache-location` is a directory.", async () => {
const { stderr } = await runPrettier(dir, [
"foo.js",
"--cache",
"--cache-location",
"dir",
]);
expect(stripAnsi(stderr.trim())).toEqual(
expect.stringMatching(
/\[error] Resolved --cache-location '.+' is a directory/
)
);
});

describe("--cache-strategy metadata", () => {
it("creates default cache file named `node_modules/.cache/prettier/.prettier-cache`", async () => {
await expect(fs.stat(defaultCacheFile)).rejects.toHaveProperty(
Expand Down Expand Up @@ -370,4 +388,83 @@ describe("--cache option", () => {
await expect(fs.stat(defaultCacheFile)).rejects.toThrowError();
});
});

describe("--cache-location", () => {
it("doesn't create default cache file when `--cache-location` exists", async () => {
await expect(fs.stat(defaultCacheFile)).rejects.toHaveProperty(
"code",
"ENOENT"
);
await runPrettier(dir, [
"--cache",
"--cache-location",
nonDefaultCacheFileName,
".",
]);
await expect(fs.stat(defaultCacheFile)).rejects.toHaveProperty(
"code",
"ENOENT"
);
});

it("throws error for invalid JSON file", async () => {
const { stderr } = await runPrettier(dir, [
"--cache",
"--cache-location",
"a.js",
".",
]);
expect(stripAnsi(stderr).trim()).toEqual(
expect.stringMatching(/\[error] '.+' isn't a valid JSON file/)
);
});

describe("file", () => {
it("creates the cache file at location specified by `--cache-location`", async () => {
await expect(fs.stat(nonDefaultCacheFilePath)).rejects.toHaveProperty(
"code",
"ENOENT"
);
await runPrettier(dir, [
"--cache",
"--cache-location",
nonDefaultCacheFileName,
".",
]);
await expect(
fs.stat(nonDefaultCacheFilePath)
).resolves.not.toThrowError();
});

it("does'nt format when cache is available", async () => {
const { stdout: firstStdout } = await runPrettier(dir, [
"--cache",
"--write",
"--cache-location",
nonDefaultCacheFileName,
".",
]);
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, [
"--cache",
"--write",
"--cache-location",
nonDefaultCacheFileName,
".",
]);
expect(stripAnsi(secondStdout).split("\n").filter(Boolean)).toEqual(
expect.arrayContaining([
expect.stringMatching(/^a\.js .+ms \(cached\)$/),
expect.stringMatching(/^b\.js .+ms \(cached\)$/),
])
);
});
});
});
});
Empty file.