Add `--cache` CLI option #12800

sosukesuzuki · 2022-05-06T19:45:26Z

Description

Two new CLI options have been added for a caching system similar to ESLint's one. Please see the documentation for details.

`--cache`

When you want to format only changed files, you can run Prettier with --cache flag. Enabling this flag can dramatically improve running time.

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, metadata will be used.

prettier --write --cache --cache-strategy content src

Checklist

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
(If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

alexander-akait · 2022-05-06T20:14:53Z

Great job!

fisker · 2022-05-07T05:51:32Z

This options should be implement on API side, not on CLI (like eslint/stylelint do), it is allow to use cache for node API users (plugins for IDE, custom CLI runner, etc)

#6577 (comment)

fisker · 2022-05-07T05:56:43Z

src/cli/constant.js

+  "cache-location": {
+    type: "path",
+    description: "Path to the cache file or directory",
+    default: ".prettiercache",


Default to node_modules/.cache/prettier? https://github.com/avajs/find-cache-dir, #5910 also use this.

in my opinion, node_modules/.cache is a really bad place to put the cache.
Because when using yarn, if any dependency is upgraded (not just prettier), then .cache is cleared.
find-cache-dir does allow specifying a CACHE_DIR so I can use that to get around it, but if you use alot of packages and upgrade daily and use yarn, it make the cache option useless.

sosukesuzuki · 2022-05-07T07:50:34Z

This options should be implement on API side, not on CLI (like eslint/stylelint do), it is allow to use cache for node API users (plugins for IDE, custom CLI runner, etc)

@fisker /cc @alexander-akait The current Prettier JavaScript API does not take a file path as an argument. Therefore, I don't think it is possible to implement a file-based cache in the API.

fisker · 2022-05-07T08:50:51Z

src/cli/find-cache-file.js

+ * Find default cache file (`./node_modules/.cache/prettier/.prettiercache`) using https://github.com/avajs/find-cache-dir
+ */
+function findDefaultCacheFile() {
+  const cacheDirThunk = findCacheDir({ name: "prettier", thunk: true });


#5910 (comment)

fisker · 2022-05-07T09:01:07Z

src/cli/utils.js

+ * @returns {string}
+ */
+function createHash(source) {
+  return crypto.createHash("md5").update(source).digest("hex");


https://github.com/prettier/prettier/pull/5910/files#r273623309, or maybe some other non-cryptographic-hash? I use sdbm sometime.

sdbm looks good f1b2c74

fisker · 2022-05-10T13:31:21Z

src/cli/format-results-cache.js

+function getMetadataFromFileDescriptor(fileDescriptor) {
+  return fileDescriptor.meta;
+}


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

Just for type checking...

fisker · 2022-05-10T13:35:13Z

src/cli/constant.js

+    default: false,
+    description: "Only format changed files",
+    type: "boolean",
+  },


Should we enable it by default?

At least we cannot enable it by default until v3

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

fisker · 2022-05-10T13:37:12Z

src/cli/constant.js

+  "cache-location": {
+    description:
+      "Path to the cache file or directory\nDefaults to node_modules/.cache/prettier/.prettiercache",
+    type: "path",


Do we need this?

I added cache-location options because ESLint has same option. But, I think we can remove this.

I think it makes sense to leave cache-location. It's a good idea to leave users the option to control where this cache is stored. It's useful in CI at least. Usually, people cache their node_modules by yarn.lock and it could lead to undesired caching of prettier.

Also removing node_modules for reinstall is still a widely spreaded approach and it will wipe all prettier cache

Why don't we add it if someone request in future?

cspell.json

fisker · 2022-05-10T13:40:20Z

src/cli/format-results-cache.js

+    return optionsHashCache.get(options);
+  }
+  const hash = createHash(
+    `${prettierVersion}_${nodeVersion}_${stringify(options)}`


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

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

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.

fisker · 2022-05-10T13:41:29Z

src/cli/format-results-cache.js

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


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

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

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

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

fisker · 2022-05-10T13:46:18Z

src/cli/format.js

@@ -362,17 +368,28 @@ async function formatFiles(context) {

    const start = Date.now();

+    const existsCache = formatResultsCache?.existsAvailableFormatResultsCache(
+      filename,


Should we use absolute path here?

Why? I think current implementation is enough.

kachkaev · 2022-05-10T18:28:39Z

How realistic would it be to cache markdown code blocks on their own (as if there were separate files)?

I am maintaining prettier-plugin-elm which calls elm-format fore every code block. This is pretty expensive, so I have to cache Elm code blocks within the plugin. Offloading caching to Prettier core would be amazing because my plugin will be free from side effects!

alexander-akait · 2022-05-20T11:48:51Z

src/cli/format-results-cache.js

+const { createHash } = require("./utils.js");
+
+const optionsHashCache = new WeakMap();
+const nodeVersion = process && process.version;


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

alexander-akait · 2022-05-21T15:21:05Z

@sosukesuzuki

@fisker /cc @alexander-akait The current Prettier JavaScript API does not take a file path as an argument. Therefore, I don't think it is possible to implement a file-based cache in the API.

Good question. But we have a glob, can we use it?

fisker · 2022-05-21T16:33:06Z

I think @sosukesuzuki is right. it's impossible to add this in API, unless we add something like formatFile that ignores the input.

src/cli/format.js

src/cli/constant.js

src/cli/find-cache-file.js

src/cli/format-results-cache.js

tests/integration/__tests__/cache.js

fisker · 2022-05-28T01:26:11Z

changelog_unreleased/cli/12800.md

@@ -0,0 +1,19 @@
+#### Add `--cache` CLI option (#12800 by @sosukesuzuki)


Missed cache-strategy in title.

fisker · 2022-05-28T01:28:18Z

changelog_unreleased/cli/12800.md

+
+##### `--cache`
+
+When you want to format only changed files, you can run Prettier with `--cache` flag. Enabling this flag can dramatically improve running time.


This sounds like CLI will skip unchanged files, but it's not.

What is wrong?

Even if file is unchanged, but last run was not using --cache we'll format it.

Or only config changed, but file didn't change.

I got it. However, I have no idea to better sentence now... I'll figure it out later.

fisker · 2022-05-28T01:29:42Z

changelog_unreleased/cli/12800.md

@@ -0,0 +1,19 @@
+#### Add `--cache` 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). Please see the [documentation](https://prettier.io/docs/en/cli.html#--cache) for details.


The link to docs seems unnecessary, you already explained here.

src/cli/utils.js

tests/integration/__tests__/cache.js

src/cli/format-results-cache.js

sosukesuzuki · 2022-05-30T18:58:42Z

How realistic would it be to cache markdown code blocks on their own (as if there were separate files)?

@kachkaev Sorry I forgot to reply to you. Since the cache feature implemented here is a per-file cache, it is not possible to use this feature for caching Markdown code blocks.

If caching Markdown code blocks is beneficial enough for the Prettier core, it could be implemented in the future( But don't expect...)

Source: prettier#12800

Co-authored-by: Simon Lydell <simon.lydell@gmail.com>

sosukesuzuki · 2022-06-11T14:32:27Z

@fisker @alexander-akait @lydell I will merge for now, but if you have any concerns, please comment again before the release.

andersk · 2022-09-21T22:37:21Z

src/cli/format.js

      output = result.formatted;
    } catch (error) {
      handleError(context, filename, error, printedFilename);
      continue;
    }

+    formatResultsCache?.setFormatResultsCache(filename, options);


If we’re running without --write, this incorrectly caches the unformatted file rather than the formatted file, which makes the --cache option unusable. Please merge #13016.

* Install `file-entry-cache` and types * Implement `FormatResultsCache` * Add `--cache` and `--cache-location` option * Implement `--cache` and `--cache-location` * Add tests for `--cache` * Add tests for `--cache-location` * Avoid spellcheck error * Update snapshots for new options * Add `groupBy` for website * Use `fs.promises` instead of `fs/promises` * Use `rimraf` instead of `fs.rm` * Fix properties order of `options` * Install `find-cache-dir` and types * Change default cache file location to `node_modules/.cache/prettier` * Fix tests following to change default cache file location * Update snapshots for changing default cache file location * Set `os.tmpdir()` as a fallback for default cache dir * Add docs for `--cache` and `--cache-location` * Use `sdbm` instead of `node:crypto` * Add changelog * Fix `getHashOfOptions` * Remove `cache-location` * `prettiercache` -> `prettier-cache` * Remove cache file when run Prettier without `--cache` option * Implement `--cache-strategy` * Add tests for invalid cache strategy * Fix lint problems * Tweaks tests * Add tests for timestamp * Add tests for `--cache-strategy` * Add docs for cache-strategy * Address review * Fix `findCacheFile` * Use Set.prototype.has * Throw error with --stdin-filepath * Update snapshots * Fix error for cache and stdin * Use string constructor instead of toString * Update changelog * Use flag validation * Fix `cache-strategy` definition * Update docs * Mark highlihgt * Throw error for `--cache-strategy` without `--cache` * Update docs * Fix typo * Updates snapshots * Update docs/cli.md Co-authored-by: Simon Lydell <simon.lydell@gmail.com> * Fix error message * Remove `:::` syntax * Defaults `content` * Update docs * Fix by Prettier Co-authored-by: Simon Lydell <simon.lydell@gmail.com>

fisker reviewed May 7, 2022

View reviewed changes

sosukesuzuki force-pushed the cache branch from 71b05be to ba829a9 Compare May 7, 2022 07:52

fisker reviewed May 7, 2022

View reviewed changes

sosukesuzuki requested review from fisker and alexander-akait May 9, 2022 14:58

fisker reviewed May 10, 2022

View reviewed changes

sosukesuzuki force-pushed the cache branch from 70a2972 to cf3450e Compare May 18, 2022 11:17

alexander-akait reviewed May 20, 2022

View reviewed changes

sosukesuzuki force-pushed the cache branch from cf3450e to 862b405 Compare May 27, 2022 09:53

sosukesuzuki changed the title ~~Add new --cache and --cache-location~~ Add --cache CLI optio May 27, 2022

sosukesuzuki changed the title ~~Add --cache CLI optio~~ Add --cache CLI option May 27, 2022

sosukesuzuki added this to the 2.7 milestone May 27, 2022

fisker reviewed May 27, 2022

View reviewed changes

fisker reviewed May 28, 2022

View reviewed changes

src/cli/utils.js Outdated Show resolved Hide resolved

fisker reviewed May 28, 2022

View reviewed changes

tests/integration/__tests__/cache.js Outdated Show resolved Hide resolved

fisker reviewed May 30, 2022

View reviewed changes

src/cli/format-results-cache.js Outdated Show resolved Hide resolved

sosukesuzuki force-pushed the cache branch from ec0c9f7 to a1a9d47 Compare May 30, 2022 18:33

bpaquet added a commit to bpaquet/prettier that referenced this pull request Jun 3, 2022

Backport cache feature to 2.6.2

647a45e

Source: prettier#12800

sosukesuzuki and others added 19 commits June 11, 2022 22:37

Use Set.prototype.has

5f9f9f6

Throw error with --stdin-filepath

cdda1e6

Update snapshots

76efb7b

Fix error for cache and stdin

3fed101

Use string constructor instead of toString

fab9aa6

Update changelog

ead522e

Use flag validation

307033e

Fix cache-strategy definition

fffbd4e

Update docs

34a6712

Mark highlihgt

c383f96

Throw error for --cache-strategy without --cache

46ec439

Update docs

a47884f

Fix typo

aec547c

Updates snapshots

a66993b

Update docs/cli.md

5e7bd09

Co-authored-by: Simon Lydell <simon.lydell@gmail.com>

Fix error message

f93fa55

Remove ::: syntax

6a9c07b

Defaults content

b2fb4bb

Update docs

6646b2f

sosukesuzuki force-pushed the cache branch from 87cd384 to 6646b2f Compare June 11, 2022 13:38

Fix by Prettier

197b568

sosukesuzuki merged commit fdccc0f into prettier:main Jun 11, 2022

sosukesuzuki deleted the cache branch June 11, 2022 14:32

HolgerJeromin mentioned this pull request Jun 14, 2022

Better cache handling of multiple (but different) prettier calls #13010

Closed

Milly mentioned this pull request Jun 17, 2022

Fix --cache set only if with --write or no-different #13016

Merged

4 tasks

fisker mentioned this pull request Jun 19, 2022

Add --cache-location option #13019

Merged

4 tasks

adrianjost mentioned this pull request Jul 5, 2022

node_modules/.cache/prettier #13032

Open

andersk reviewed Sep 21, 2022

View reviewed changes

		@@ -0,0 +1,19 @@
		#### Add `--cache` CLI option (#12800 by @sosukesuzuki)


		##### `--cache`

		When you want to format only changed files, you can run Prettier with `--cache` flag. Enabling this flag can dramatically improve running time.

Add --cache CLI option #12800

Add --cache CLI option #12800

Conversation

sosukesuzuki commented May 6, 2022 • edited

Description

--cache

--cache-strategy

Checklist

alexander-akait commented May 6, 2022

fisker commented May 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosukesuzuki commented May 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fisker May 21, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

7rulnik May 20, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kachkaev commented May 10, 2022 • edited

Choose a reason for hiding this comment

alexander-akait commented May 21, 2022

fisker commented May 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sosukesuzuki commented May 30, 2022

sosukesuzuki commented Jun 11, 2022

Choose a reason for hiding this comment

Add `--cache` CLI option #12800

Add `--cache` CLI option #12800

sosukesuzuki commented May 6, 2022 •

edited

`--cache`

`--cache-strategy`

fisker May 21, 2022 •

edited

7rulnik May 20, 2022 •

edited

kachkaev commented May 10, 2022 •

edited