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 CLI option #12800

Merged
merged 53 commits into from Jun 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
be32c64
Install `file-entry-cache` and types
sosukesuzuki May 6, 2022
94e27ff
Implement `FormatResultsCache`
sosukesuzuki May 6, 2022
b1173f1
Add `--cache` and `--cache-location` option
sosukesuzuki May 6, 2022
ebd577f
Implement `--cache` and `--cache-location`
sosukesuzuki May 6, 2022
56f432c
Add tests for `--cache`
sosukesuzuki May 6, 2022
7b03c76
Add tests for `--cache-location`
sosukesuzuki May 6, 2022
78f6640
Avoid spellcheck error
sosukesuzuki May 6, 2022
54cbbbc
Update snapshots for new options
sosukesuzuki May 6, 2022
2c7b9a3
Add `groupBy` for website
sosukesuzuki May 6, 2022
3e0bc43
Use `fs.promises` instead of `fs/promises`
sosukesuzuki May 6, 2022
336262c
Use `rimraf` instead of `fs.rm`
sosukesuzuki May 7, 2022
e87a01e
Fix properties order of `options`
sosukesuzuki May 7, 2022
5fcd8bd
Install `find-cache-dir` and types
sosukesuzuki May 7, 2022
e29a714
Change default cache file location to `node_modules/.cache/prettier`
sosukesuzuki May 7, 2022
1095240
Fix tests following to change default cache file location
sosukesuzuki May 7, 2022
d248cdd
Update snapshots for changing default cache file location
sosukesuzuki May 7, 2022
85c85f6
Set `os.tmpdir()` as a fallback for default cache dir
sosukesuzuki May 7, 2022
5cec2f9
Add docs for `--cache` and `--cache-location`
sosukesuzuki May 7, 2022
8536899
Use `sdbm` instead of `node:crypto`
sosukesuzuki May 7, 2022
ef3b7ab
Add changelog
sosukesuzuki May 8, 2022
a7c845f
Fix `getHashOfOptions`
sosukesuzuki May 9, 2022
11a1c4f
Remove `cache-location`
sosukesuzuki May 11, 2022
01159fb
`prettiercache` -> `prettier-cache`
sosukesuzuki May 11, 2022
ed1047b
Remove cache file when run Prettier without `--cache` option
sosukesuzuki May 11, 2022
e7d739a
Implement `--cache-strategy`
sosukesuzuki May 27, 2022
fd0484a
Add tests for invalid cache strategy
sosukesuzuki May 27, 2022
efbbe8e
Fix lint problems
sosukesuzuki May 27, 2022
c4eebf1
Tweaks tests
sosukesuzuki May 27, 2022
a5c2656
Add tests for timestamp
sosukesuzuki May 27, 2022
80f2d91
Add tests for `--cache-strategy`
sosukesuzuki May 27, 2022
2a8aa7c
Add docs for cache-strategy
sosukesuzuki May 27, 2022
882213b
Address review
sosukesuzuki May 27, 2022
c6d7287
Fix `findCacheFile`
sosukesuzuki May 27, 2022
5f9f9f6
Use Set.prototype.has
sosukesuzuki May 27, 2022
cdda1e6
Throw error with --stdin-filepath
sosukesuzuki May 27, 2022
76efb7b
Update snapshots
sosukesuzuki May 27, 2022
3fed101
Fix error for cache and stdin
sosukesuzuki May 28, 2022
fab9aa6
Use string constructor instead of toString
sosukesuzuki May 28, 2022
ead522e
Update changelog
sosukesuzuki May 28, 2022
307033e
Use flag validation
sosukesuzuki May 30, 2022
fffbd4e
Fix `cache-strategy` definition
sosukesuzuki May 30, 2022
34a6712
Update docs
sosukesuzuki Jun 4, 2022
c383f96
Mark highlihgt
sosukesuzuki Jun 4, 2022
46ec439
Throw error for `--cache-strategy` without `--cache`
sosukesuzuki Jun 4, 2022
a47884f
Update docs
sosukesuzuki Jun 4, 2022
aec547c
Fix typo
sosukesuzuki Jun 4, 2022
a66993b
Updates snapshots
sosukesuzuki Jun 4, 2022
5e7bd09
Update docs/cli.md
sosukesuzuki Jun 4, 2022
f93fa55
Fix error message
sosukesuzuki Jun 5, 2022
6a9c07b
Remove `:::` syntax
sosukesuzuki Jun 5, 2022
b2fb4bb
Defaults `content`
sosukesuzuki Jun 11, 2022
6646b2f
Update docs
sosukesuzuki Jun 11, 2022
197b568
Fix by Prettier
sosukesuzuki Jun 11, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions changelog_unreleased/cli/12800.md
@@ -0,0 +1,25 @@
#### [HIGHLIGHT]Add `--cache` and `--cache-strategy` CLI option (#12800 by @sosukesuzuki)

Two new CLI options have been added for a caching system similar to [ESLint's one](https://eslint.org/docs/user-guide/command-line-interface#caching).

##### `--cache`
lydell marked this conversation as resolved.
Show resolved Hide resolved

If this option is enabled, the following values are used as cache keys and the file is formatted only if one of them is changed.

- Prettier version
- Options
- Node.js version
- (if `--cache-strategy` is `content`) content of the file
- (if `--cache-strategy` is `metadata`) file metadata, such as timestamps

```bash
prettier --write --cache src
```

##### `--cache-strategy`

Strategy for the cache to use for detecting changed files. Can be either `metadata` or `content`. If no strategy is specified, `content` will be used.

```bash
prettier --write --cache --cache-strategy metadata src
```
1 change: 1 addition & 0 deletions cspell.json
Expand Up @@ -289,6 +289,7 @@
"sandhose",
"Sapegin",
"sbdchd",
"sdbm",
"scandir",
"Serializers",
"setlocal",
Expand Down
32 changes: 32 additions & 0 deletions docs/cli.md
Expand Up @@ -204,3 +204,35 @@ Prevent errors when pattern is unmatched.
## `--no-plugin-search`

Disable plugin autoloading.

## `--cache`

If this option is enabled, the following values are used as cache keys and the file is formatted only if one of them is changed.

- Prettier version
- Options
- Node.js version
- (if `--cache-strategy` is `metadata`) file metadata, such as timestamps
- (if `--cache-strategy` is `content`) content of the file

```bash
prettier --write --cache src
```

Running Prettier without `--cache` will delete the cache.

Also, since the cache file is stored in `./node_modules/.cache/prettier/.prettier-cache`, so you can use `rm ./node_modules/.cache/prettier/.prettier-cache` to remove it manually.

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

Strategy for the cache to use for detecting changed files. Can be either `metadata` or `content`.

In general, `metadata` is faster. However, `content` is useful for updating the timestamp without changing the file content. This can happen, for example, during git operations such as `git clone`, because it does not track file modification times.

If no strategy is specified, `content` will be used.

```bash
prettier --write --cache --cache-strategy metadata src
```
5 changes: 5 additions & 0 deletions package.json
Expand Up @@ -47,6 +47,8 @@
"esutils": "2.0.3",
"fast-glob": "3.2.11",
"fast-json-stable-stringify": "2.1.0",
"file-entry-cache": "6.0.1",
"find-cache-dir": "3.3.2",
"find-parent-dir": "0.3.1",
"flow-parser": "0.180.0",
"get-stdin": "8.0.0",
Expand Down Expand Up @@ -79,6 +81,7 @@
"remark-math": "3.0.1",
"remark-parse": "8.0.3",
"resolve": "1.22.0",
"sdbm": "2.0.0",
"semver": "7.3.7",
"string-width": "5.0.1",
"strip-ansi": "7.0.1",
Expand All @@ -96,6 +99,8 @@
"@esbuild-plugins/node-modules-polyfill": "0.1.4",
"@glimmer/reference": "0.84.2",
"@types/estree": "0.0.51",
"@types/file-entry-cache": "5.0.2",
"@types/find-cache-dir": "3.2.1",
"@types/jest": "27.4.1",
"@typescript-eslint/eslint-plugin": "5.20.0",
"babel-jest": "27.5.1",
Expand Down
18 changes: 18 additions & 0 deletions scripts/vendors/vendor-meta.json
Expand Up @@ -9,6 +9,7 @@
"html-void-elements": "html-void-elements.json",
"leven": "leven.js",
"mem": "mem.js",
"sdbm": "sdbm.js",
"string-width": "string-width.js",
"strip-ansi": "strip-ansi.js"
},
Expand Down Expand Up @@ -503,6 +504,23 @@
},
"contributors": []
},
{
"name": "sdbm",
"maintainers": [],
"version": "2.0.0",
"description": "SDBM non-cryptographic hash function",
"repository": "sindresorhus/sdbm",
"homepage": null,
"private": false,
"license": "MIT",
"licenseText": "MIT License\n\nCopyright (c) Sindre Sorhus <sindresorhus@gmail.com> (https://sindresorhus.com)\n\nPermission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the \"Software\"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.\n",
"author": {
"name": "Sindre Sorhus",
"email": "sindresorhus@gmail.com",
"url": "https://sindresorhus.com"
},
"contributors": []
},
{
"name": "ansi-regex",
"maintainers": [],
Expand Down
1 change: 1 addition & 0 deletions scripts/vendors/vendors.mjs
Expand Up @@ -8,6 +8,7 @@ const vendors = [
"html-void-elements",
"leven",
"mem",
"sdbm",
"string-width",
"strip-ansi",
"tempy",
Expand Down
19 changes: 19 additions & 0 deletions src/cli/constant.js
Expand Up @@ -71,6 +71,25 @@ const categoryOrder = [
*/
/* eslint sort-keys: "error" */
const options = {
cache: {
default: false,
description: "Only format changed files. Cannot use with --stdin-filepath.",
type: "boolean",
},
Copy link
Member

Choose a reason for hiding this comment

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

Should we enable it by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

At least we cannot enable it by default until v3

Copy link
Member

Choose a reason for hiding this comment

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

I am thinking about it too, but most of tools disable cache by default - not to waste extra disk space without allowing, also some CI can work in read only access or with write access only for specified dirs, so if we enable it by default in v2, we can break CI

"cache-strategy": {
choices: [
{
description: "Use the file metadata such as timestamps as cache keys",
value: "metadata",
},
{
description: "Use the file content as cache keys",
value: "content",
},
],
description: "Strategy for the cache to use for detecting changed files.",
type: "choice",
},
check: {
alias: "c",
category: coreOptions.CATEGORY_OUTPUT,
Expand Down
19 changes: 2 additions & 17 deletions src/cli/expand-patterns.js
@@ -1,9 +1,10 @@
"use strict";

const path = require("path");
const { promises: fs } = require("fs");
const fastGlob = require("fast-glob");

const { statSafe } = require("./utils.js");

/** @typedef {import('./context').Context} Context */

/**
Expand Down Expand Up @@ -173,22 +174,6 @@ function sortPaths(paths) {
return paths.sort((a, b) => a.localeCompare(b));
}

/**
* Get stats of a given path.
* @param {string} filePath The path to target file.
* @returns {Promise<import('fs').Stats | undefined>} The stats.
*/
async function statSafe(filePath) {
try {
return await fs.stat(filePath);
} catch (error) {
/* istanbul ignore next */
if (error.code !== "ENOENT") {
throw error;
}
}
}

/**
* This function should be replaced with `fastGlob.escapePath` when these issues are fixed:
* - https://github.com/mrmlnc/fast-glob/issues/261
Expand Down
19 changes: 19 additions & 0 deletions src/cli/find-cache-file.js
@@ -0,0 +1,19 @@
"use strict";

const os = require("os");
const path = require("path");
const findCacheDir = require("find-cache-dir");

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

module.exports = findCacheFile;
96 changes: 96 additions & 0 deletions src/cli/format-results-cache.js
@@ -0,0 +1,96 @@
"use strict";

// Inspired by LintResultsCache from ESLint
// https://github.com/eslint/eslint/blob/c2d0a830754b6099a3325e6d3348c3ba983a677a/lib/cli-engine/lint-result-cache.js

const fileEntryCache = require("file-entry-cache");
const stringify = require("fast-json-stable-stringify");
// eslint-disable-next-line no-restricted-modules
const { version: prettierVersion } = require("../index.js");
const { createHash } = require("./utils.js");

const optionsHashCache = new WeakMap();
const nodeVersion = process && process.version;
Copy link
Member

Choose a reason for hiding this comment

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

Why we need it? Something in our code depend on Node.js version?


/**
* @param {*} options
* @returns {string}
*/
function getHashOfOptions(options) {
if (optionsHashCache.has(options)) {
return optionsHashCache.get(options);
}
const hash = createHash(
`${prettierVersion}_${nodeVersion}_${stringify(options)}`
Copy link
Member

Choose a reason for hiding this comment

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

We allow functions in options, not sure how should we handle this.

plugins: [require('prettier-plugin-foo')]

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea to serialize an option that includes functions for plugins. What do you think about to mention about behavior for plugins and caching.

If you are using plugins in your Prettier config file, caching may not work well. If caching does not work well after updating the plugin, try running Prettier without the `--cache` option to clear the cache.

Copy link
Member Author

Choose a reason for hiding this comment

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

);
optionsHashCache.set(options, hash);
return hash;
}

/**
* @typedef {{ hashOfOptions?: string }} OurMeta
* @typedef {import("file-entry-cache").FileDescriptor} FileDescriptor
*
* @param {import("file-entry-cache").FileDescriptor} fileDescriptor
* @returns {FileDescriptor["meta"] & OurMeta}
*/
function getMetadataFromFileDescriptor(fileDescriptor) {
return fileDescriptor.meta;
}
Comment on lines +37 to +39
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we use fileDescriptor.meta directly? This just making code less readable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for type checking...


class FormatResultsCache {
/**
* @param {string} cacheFileLocation The path of cache file location. (default: `node_modules/.cache/prettier/prettier-cache`)
* @param {string} cacheStrategy
*/
constructor(cacheFileLocation, cacheStrategy) {
const useChecksum = cacheStrategy === "content";

this.cacheFileLocation = cacheFileLocation;
this.fileEntryCache = fileEntryCache.create(
/* cacheId */ cacheFileLocation,
/* directory */ undefined,
useChecksum
);
}

/**
* @param {string} filePath
* @param {any} options
*/
existsAvailableFormatResultsCache(filePath, options) {
const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);
Copy link
Member

Choose a reason for hiding this comment

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

Feel not safe without file content, but that will be slow.

Copy link
Member Author

Choose a reason for hiding this comment

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

For minimum speed and security, why not check the length of the file content rather than the content of the file?

Copy link

@7rulnik 7rulnik May 20, 2022

Choose a reason for hiding this comment

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

In this case it's not usable in CI. ESLint, for example, has --cache-strategy which allows to use metadata or content for detecting changed files

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I've added --cache-strategy option!

const hashOfOptions = getHashOfOptions(options);
const meta = getMetadataFromFileDescriptor(fileDescriptor);
const changed =
fileDescriptor.changed || meta.hashOfOptions !== hashOfOptions;

if (fileDescriptor.notFound) {
return false;
}

if (changed) {
return false;
}

return true;
}

/**
* @param {string} filePath
* @param {any} options
*/
setFormatResultsCache(filePath, options) {
const fileDescriptor = this.fileEntryCache.getFileDescriptor(filePath);
const meta = getMetadataFromFileDescriptor(fileDescriptor);
if (fileDescriptor && !fileDescriptor.notFound) {
meta.hashOfOptions = getHashOfOptions(options);
}
}

reconcile() {
this.fileEntryCache.reconcile();
}
}

module.exports = FormatResultsCache;