From 6d4d6a759984c40ad2648fa17ad709c7a9830efb Mon Sep 17 00:00:00 2001 From: Evilebot Tnawi Date: Thu, 3 Oct 2019 17:21:05 +0300 Subject: [PATCH] revert: `--only-changed` argument (#6594) --- CHANGELOG.unreleased.md | 4 - docs/cli.md | 2 - package.json | 2 - src/cli/changed-cache.js | 79 --------- src/cli/constant.js | 4 - src/cli/index.js | 5 - src/cli/util.js | 58 +------ src/common/third-party.js | 3 +- .../__snapshots__/early-exit.js.snap | 11 -- .../__snapshots__/help-options.js.snap | 11 -- .../__snapshots__/only-changed.js.snap | 43 ----- tests_integration/__tests__/early-exit.js | 6 - tests_integration/__tests__/only-changed.js | 162 ------------------ .../cli/only-changed/formatted.js | 1 - .../cli/only-changed/unformatted.js | 1 - tests_integration/runPrettier.js | 20 --- yarn.lock | 23 +-- 17 files changed, 9 insertions(+), 426 deletions(-) delete mode 100644 src/cli/changed-cache.js delete mode 100644 tests_integration/__tests__/__snapshots__/only-changed.js.snap delete mode 100644 tests_integration/__tests__/only-changed.js delete mode 100644 tests_integration/cli/only-changed/formatted.js delete mode 100644 tests_integration/cli/only-changed/unformatted.js diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index f7c895079..697e6138d 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -423,10 +423,6 @@ useEffect( This version updates the TypeScript parser to correctly handle JSX text with double slashes (`//`). In previous versions, this would cause Prettier to crash. -#### CLI: Add `--only-changed` flag ([#5910] by [@g-harel]) - -Flag used with `--write` to avoid re-checking files that were not changed since they were last written (with the same formatting configuration). - #### HTML, Vue: Don't break the template element included in a line shorter than print-width([#6284] by [@sosukesuzuki]) Previously, even if the line length is shorter than print-width is Prettier breaks the line with a template element. diff --git a/docs/cli.md b/docs/cli.md index a7cae539a..d1ef8bae0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -148,8 +148,6 @@ Prettier CLI will ignore files located in `node_modules` directory. To opt-out f This rewrites all processed files in place. This is comparable to the `eslint --fix` workflow. -To avoid re-checking unchanged files, use the `--only-changed` flag. - ## `--loglevel` Change the level of logging for the CLI. Valid options are: diff --git a/package.json b/package.json index 8783668ad..0cd5bafd3 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,6 @@ "editorconfig-to-prettier": "0.1.1", "escape-string-regexp": "1.0.5", "esutils": "2.0.3", - "find-cache-dir": "1.0.0", "find-parent-dir": "0.3.0", "find-project-root": "1.1.1", "flow-parser": "0.89.0", @@ -72,7 +71,6 @@ "unicode-regex": "3.0.0", "unified": "6.1.6", "vnopts": "1.0.2", - "write-file-atomic": "2.3.0", "yaml": "1.1.0", "yaml-unist-parser": "1.0.0" }, diff --git a/src/cli/changed-cache.js b/src/cli/changed-cache.js deleted file mode 100644 index abb377e79..000000000 --- a/src/cli/changed-cache.js +++ /dev/null @@ -1,79 +0,0 @@ -"use strict"; - -const crypto = require("crypto"); - -// Generates a hash of the input string. -function hash(data) { - return crypto - .createHash("sha1") - .update(data) - .digest("base64"); -} - -// Generates the cache key using the file path, options and the support info hash. -function calcKey(path, options, supportInfoHash) { - return hash(path + JSON.stringify(options) + supportInfoHash); -} - -class ChangedCache { - // Initializes the in-memory cache data from the configured location. - // Also calculates the static support info hash used to compute file keys. - // A missing cache file is not treated as an error because it is expected on first run. - constructor(options) { - this.location = options.location; - this.readFile = options.readFile; - this.writeFile = options.writeFile; - this.context = options.context; - this.supportInfoHash = hash(JSON.stringify(options.supportInfo)); - - this.cache = {}; - - let contents; - try { - contents = this.readFile(this.location, "utf8"); - } catch (err) { - if (err.code !== "ENOENT") { - this.context.logger.error(`Could not read cache file: ${err}`); - } - return; - } - - try { - this.cache = JSON.parse(contents); - } catch (err) { - this.context.logger.error(`Could not parse cache contents: ${err}`); - } - } - - // Writes the in-memory cache data to the configured file. - // Previous file contents are overwritten. - close() { - let contents; - try { - contents = JSON.stringify(this.cache); - } catch (err) { - this.context.logger.error(`Could not serialize cache: ${err}`); - return; - } - - try { - this.writeFile(this.location, contents, "utf8"); - } catch (err) { - this.context.logger.error(`Could not write cache to file: ${err}`); - } - } - - // Checks if the expected contents of the file path match the in-memory data. - notChanged(path, options, content) { - return ( - this.cache[calcKey(path, options, this.supportInfoHash)] === hash(content) - ); - } - - // Updates the expected contents of the file path in the in-memory data. - update(path, options, content) { - this.cache[calcKey(path, options, this.supportInfoHash)] = hash(content); - } -} - -module.exports = ChangedCache; diff --git a/src/cli/constant.js b/src/cli/constant.js index be4175a97..05f7047fe 100644 --- a/src/cli/constant.js +++ b/src/cli/constant.js @@ -187,10 +187,6 @@ const options = { default: "log", choices: ["silent", "error", "warn", "log", "debug"] }, - "only-changed": { - type: "boolean", - description: "Only format files changed since last '--write'." - }, stdin: { type: "boolean", description: "Force reading input from stdin." diff --git a/src/cli/index.js b/src/cli/index.js index 9ffd63e45..f7a6b7832 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -22,11 +22,6 @@ function run(args) { process.exit(1); } - if (context.argv["only-changed"] && !context.argv["write"]) { - context.logger.error("Cannot use --only-changed without --write."); - process.exit(1); - } - if (context.argv["find-config-path"] && context.filePatterns.length) { context.logger.error("Cannot use --find-config-path with multiple files"); process.exit(1); diff --git a/src/cli/util.js b/src/cli/util.js index 2c17fc8a9..592575c00 100644 --- a/src/cli/util.js +++ b/src/cli/util.js @@ -8,8 +8,6 @@ const globby = require("globby"); const chalk = require("chalk"); const readline = require("readline"); const stringify = require("json-stable-stringify"); -const findCacheDir = require("find-cache-dir"); -const os = require("os"); const minimist = require("./minimist"); const prettier = require("../../index"); @@ -22,7 +20,6 @@ const optionsNormalizer = require("../main/options-normalizer"); const thirdParty = require("../common/third-party"); const arrayify = require("../utils/arrayify"); const isTTY = require("../utils/is-tty"); -const ChangedCache = require("./changed-cache"); const OPTION_USAGE_THRESHOLD = 25; const CHOICE_USAGE_MARGIN = 3; @@ -444,20 +441,6 @@ function formatFiles(context) { context.logger.log("Checking formatting..."); } - let changedCache = null; - if (context.argv["only-changed"]) { - const cacheDir = - findCacheDir({ name: "prettier", create: true }) || os.tmpdir(); - - changedCache = new ChangedCache({ - location: path.join(cacheDir, "changed"), - readFile: fs.readFileSync, - writeFile: thirdParty.writeFileAtomic, - context: context, - supportInfo: prettier.getSupportInfo() - }); - } - eachFilename(context, context.filePatterns, filename => { const fileIgnored = ignorer.filter([filename]).length === 0; if ( @@ -474,15 +457,9 @@ function formatFiles(context) { filepath: filename }); - let removeFilename = () => {}; if (isTTY()) { // Don't use `console.log` here since we need to replace this line. context.logger.log(filename, { newline: false }); - removeFilename = () => { - readline.clearLine(process.stdout, 0); - readline.cursorTo(process.stdout, 0, null); - removeFilename = () => {}; - }; } let input; @@ -500,19 +477,6 @@ function formatFiles(context) { return; } - if (changedCache) { - if (changedCache.notChanged(filename, options, input)) { - // Remove previously printed filename to log it with "unchanged". - removeFilename(); - - if (!context.argv["check"] && !context.argv["list-different"]) { - context.logger.log(chalk.grey(`${filename} unchanged`)); - } - - return; - } - } - if (fileIgnored) { writeOutput(context, { formatted: input }, options); return; @@ -537,8 +501,11 @@ function formatFiles(context) { const isDifferent = output !== input; - // Remove previously printed filename to log it with duration. - removeFilename(); + if (isTTY()) { + // Remove previously printed filename to log it with duration. + readline.clearLine(process.stdout, 0); + readline.cursorTo(process.stdout, 0, null); + } if (context.argv["write"]) { // Don't write the file if it won't change in order not to invalidate @@ -557,15 +524,8 @@ function formatFiles(context) { // Don't exit the process if one file failed process.exitCode = 2; } - } else { - if (!context.argv["check"] && !context.argv["list-different"]) { - context.logger.log(`${chalk.grey(filename)} ${Date.now() - start}ms`); - } - } - - // Cache is updated to record pretty content. - if (changedCache) { - changedCache.update(filename, options, output); + } else if (!context.argv["check"] && !context.argv["list-different"]) { + context.logger.log(`${chalk.grey(filename)} ${Date.now() - start}ms`); } } else if (context.argv["debug-check"]) { if (result.filepath) { @@ -586,10 +546,6 @@ function formatFiles(context) { } }); - if (changedCache) { - changedCache.close(); - } - // Print check summary based on expected exit code if (context.argv["check"]) { context.logger.log( diff --git a/src/common/third-party.js b/src/common/third-party.js index 570dbbc2d..8089f5fde 100644 --- a/src/common/third-party.js +++ b/src/common/third-party.js @@ -4,6 +4,5 @@ module.exports = { cosmiconfig: require("cosmiconfig"), findParentDir: require("find-parent-dir").sync, getStream: require("get-stream"), - isCI: () => require("is-ci"), - writeFileAtomic: require("write-file-atomic").sync + isCI: () => require("is-ci") }; diff --git a/tests_integration/__tests__/__snapshots__/early-exit.js.snap b/tests_integration/__tests__/__snapshots__/early-exit.js.snap index 06e249305..95af8c1c2 100644 --- a/tests_integration/__tests__/__snapshots__/early-exit.js.snap +++ b/tests_integration/__tests__/__snapshots__/early-exit.js.snap @@ -144,7 +144,6 @@ Other options: --loglevel What level of logs to report. Defaults to log. - --only-changed Only format files changed since last '--write'. --require-pragma Require either '@prettier' or '@format' to be present in the file's first docblock comment in order for it to be formatted. Defaults to false. @@ -301,7 +300,6 @@ Other options: --loglevel What level of logs to report. Defaults to log. - --only-changed Only format files changed since last '--write'. --require-pragma Require either '@prettier' or '@format' to be present in the file's first docblock comment in order for it to be formatted. Defaults to false. @@ -352,15 +350,6 @@ exports[`throw error with --help not-found (stdout) 1`] = `""`; exports[`throw error with --help not-found (write) 1`] = `Array []`; -exports[`throw error with --only-changed without --write (stderr) 1`] = ` -"[error] Cannot use --only-changed without --write. -" -`; - -exports[`throw error with --only-changed without --write (stdout) 1`] = `""`; - -exports[`throw error with --only-changed without --write (write) 1`] = `Array []`; - exports[`throw error with --write + --debug-check (stderr) 1`] = ` "[error] Cannot use --write and --debug-check together. " diff --git a/tests_integration/__tests__/__snapshots__/help-options.js.snap b/tests_integration/__tests__/__snapshots__/help-options.js.snap index e412d98ce..4e4d216b1 100644 --- a/tests_integration/__tests__/__snapshots__/help-options.js.snap +++ b/tests_integration/__tests__/__snapshots__/help-options.js.snap @@ -329,17 +329,6 @@ exports[`show detailed usage with --help no-semi (stdout) 1`] = ` exports[`show detailed usage with --help no-semi (write) 1`] = `Array []`; -exports[`show detailed usage with --help only-changed (stderr) 1`] = `""`; - -exports[`show detailed usage with --help only-changed (stdout) 1`] = ` -"--only-changed - - Only format files changed since last '--write'. -" -`; - -exports[`show detailed usage with --help only-changed (write) 1`] = `Array []`; - exports[`show detailed usage with --help parser (stderr) 1`] = `""`; exports[`show detailed usage with --help parser (stdout) 1`] = ` diff --git a/tests_integration/__tests__/__snapshots__/only-changed.js.snap b/tests_integration/__tests__/__snapshots__/only-changed.js.snap deleted file mode 100644 index c49c2d715..000000000 --- a/tests_integration/__tests__/__snapshots__/only-changed.js.snap +++ /dev/null @@ -1,43 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`create cache with --write --only-changed + formatted file (stderr) 1`] = `""`; - -exports[`create cache with --write --only-changed + formatted file (stdout) 1`] = ` -"formatted.js 0ms -" -`; - -exports[`create cache with --write --only-changed + unformatted file (stderr) 1`] = `""`; - -exports[`create cache with --write --only-changed + unformatted file (stdout) 1`] = ` -"unformatted.js 0ms -" -`; - -exports[`detect config change with --write --only-changed + unformatted file (stderr) 1`] = `""`; - -exports[`detect config change with --write --only-changed + unformatted file (stderr) 2`] = `""`; - -exports[`detect config change with --write --only-changed + unformatted file (stdout) 1`] = ` -"unformatted.js 0ms -" -`; - -exports[`detect config change with --write --only-changed + unformatted file (stdout) 2`] = ` -"unformatted.js 0ms -" -`; - -exports[`detect unchanged with --write --only-changed + formatted file (stderr) 1`] = `""`; - -exports[`detect unchanged with --write --only-changed + formatted file (stderr) 2`] = `""`; - -exports[`detect unchanged with --write --only-changed + formatted file (stdout) 1`] = ` -"formatted.js 0ms -" -`; - -exports[`detect unchanged with --write --only-changed + formatted file (stdout) 2`] = ` -"formatted.js unchanged -" -`; diff --git a/tests_integration/__tests__/early-exit.js b/tests_integration/__tests__/early-exit.js index e0d291b8b..2148eaa74 100644 --- a/tests_integration/__tests__/early-exit.js +++ b/tests_integration/__tests__/early-exit.js @@ -68,12 +68,6 @@ describe("throw error with --write + --debug-check", () => { }); }); -describe("throw error with --only-changed without --write", () => { - runPrettier("cli", ["--only-changed"]).test({ - status: 1 - }); -}); - describe("throw error with --find-config-path + multiple files", () => { runPrettier("cli", ["--find-config-path", "abc.js", "def.js"]).test({ status: 1 diff --git a/tests_integration/__tests__/only-changed.js b/tests_integration/__tests__/only-changed.js deleted file mode 100644 index e6205b4d4..000000000 --- a/tests_integration/__tests__/only-changed.js +++ /dev/null @@ -1,162 +0,0 @@ -"use strict"; - -const path = require("path"); -const findCacheDir = require("find-cache-dir"); - -const runPrettier = require("../runPrettier"); -const ChangedCache = require("../../src/cli/changed-cache"); - -// Cache name must be kept consistent with value in the implementation. -const cacheName = "changed"; -const cachePath = path.join(findCacheDir({ name: "prettier" }), cacheName); - -describe("create cache with --write --only-changed + unformatted file", () => { - runPrettier("cli/only-changed", [ - "--write", - "--only-changed", - "unformatted.js" - ]).test({ - write: [{ filename: "unformatted.js" }, { filename: cachePath }], - status: 0 - }); -}); - -describe("create cache with --write --only-changed + formatted file", () => { - runPrettier("cli/only-changed", [ - "--write", - "--only-changed", - "formatted.js" - ]).test({ - write: [{ filename: cachePath }], - status: 0 - }); -}); - -describe("detect unchanged with --write --only-changed + formatted file", () => { - const res = runPrettier("cli/only-changed", [ - "--write", - "--only-changed", - "formatted.js" - ]).test({ - write: [{ filename: cachePath }], - status: 0 - }); - - const cacheContents = res.write[0].content; - - runPrettier( - "cli/only-changed", - ["--write", "--only-changed", "formatted.js"], - { - virtualFiles: { - [cachePath]: cacheContents - } - } - ).test({ - write: [{ filename: cachePath, content: cacheContents }], - status: 0 - }); -}); - -describe("detect config change with --write --only-changed + unformatted file", () => { - const resBefore = runPrettier("cli/only-changed", [ - "--write", - "--only-changed", - "unformatted.js" - ]).test({ - write: [{ filename: "unformatted.js" }, { filename: cachePath }], - status: 0 - }); - - const cacheContentsBefore = resBefore.write[1].content; - - const resAfter = runPrettier( - "cli/only-changed", - ["--write", "--only-changed", "--use-tabs", "unformatted.js"], - { - virtualFiles: { - [cachePath]: cacheContentsBefore - } - } - ).test({ - write: [{ filename: "unformatted.js" }, { filename: cachePath }], - status: 0 - }); - - const cacheContentsAfter = resAfter.write[1].content; - - expect(cacheContentsAfter).not.toBe(cacheContentsBefore); -}); - -describe("ChangedCache", () => { - it("should log errors when opening the cache file", () => { - const errLogger = jest.fn(); - const msg = "open-cache-file-error"; - - new ChangedCache({ - location: cachePath, - readFile: () => { - throw new Error(msg); - }, - writeFile: () => {}, - context: { logger: { error: errLogger } }, - supportInfo: {} - }); - - expect(errLogger).toHaveBeenCalledWith(expect.stringContaining(msg)); - }); - - it("should log errors when parsing the cache file", () => { - const errLogger = jest.fn(); - - new ChangedCache({ - location: cachePath, - readFile: () => "invalid json", - writeFile: () => {}, - context: { logger: { error: errLogger } }, - supportInfo: {} - }); - - expect(errLogger).toHaveBeenCalledWith( - expect.stringContaining("cache content") - ); - }); - - it("should log errors when closing the cache file", () => { - const errLogger = jest.fn(); - const msg = "close-cache-file-error"; - - const changedCache = new ChangedCache({ - location: cachePath, - readFile: () => "{}", - writeFile: () => { - throw new Error(msg); - }, - context: { logger: { error: errLogger } }, - supportInfo: {} - }); - changedCache.close(); - - expect(errLogger).toHaveBeenCalledWith(expect.stringContaining(msg)); - }); - - it("should log errors when serializing the cache contents", () => { - const errLogger = jest.fn(); - - const changedCache = new ChangedCache({ - location: cachePath, - readFile: () => "{}", - writeFile: () => {}, - context: { logger: { error: errLogger } }, - supportInfo: {} - }); - const mirror = {}; - mirror.self = mirror; - changedCache.cache = mirror; - changedCache.close(); - - expect(errLogger).toHaveBeenCalledWith( - expect.stringContaining("serialize cache") - ); - }); -}); diff --git a/tests_integration/cli/only-changed/formatted.js b/tests_integration/cli/only-changed/formatted.js deleted file mode 100644 index b506100a9..000000000 --- a/tests_integration/cli/only-changed/formatted.js +++ /dev/null @@ -1 +0,0 @@ -var x = 1; diff --git a/tests_integration/cli/only-changed/unformatted.js b/tests_integration/cli/only-changed/unformatted.js deleted file mode 100644 index 646611c06..000000000 --- a/tests_integration/cli/only-changed/unformatted.js +++ /dev/null @@ -1 +0,0 @@ -var x = 1; \ No newline at end of file diff --git a/tests_integration/runPrettier.js b/tests_integration/runPrettier.js index c1d62bc1e..27226065a 100644 --- a/tests_integration/runPrettier.js +++ b/tests_integration/runPrettier.js @@ -64,18 +64,6 @@ function runPrettier(dir, args, options) { return origStatSync(filename); }); - // Mock contents of "virtualFiles" when option is defined. - const origReadFileSync = fs.readFileSync; - jest.spyOn(fs, "readFileSync").mockImplementation((filename, opts) => { - if ( - typeof options.virtualFiles === "object" && - typeof options.virtualFiles[filename] === "string" - ) { - return options.virtualFiles[filename]; - } - return origReadFileSync(filename, opts); - }); - const originalCwd = process.cwd(); const originalArgv = process.argv; const originalExitCode = process.exitCode; @@ -111,11 +99,6 @@ function runPrettier(dir, args, options) { jest .spyOn(require(thirdParty), "findParentDir") .mockImplementation(() => process.cwd()); - jest - .spyOn(require(thirdParty), "writeFileAtomic") - .mockImplementation((filename, content) => { - write.push({ filename, content }); - }); try { require(prettierCli); @@ -152,9 +135,6 @@ function runPrettier(dir, args, options) { if (name in testOptions) { if (name === "status" && testOptions[name] === "non-zero") { expect(value).not.toEqual(0); - } else if (name === "write") { - // Allows assertions on a subset of the "write" result. (ex. only file name) - expect(value).toMatchObject(testOptions[name]); } else { expect(value).toEqual(testOptions[name]); } diff --git a/yarn.lock b/yarn.lock index 0195c9483..8522a5412 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3041,15 +3041,6 @@ fill-range@^4.0.0: repeat-string "^1.6.1" to-regex-range "^2.1.0" -find-cache-dir@1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/find-cache-dir/-/find-cache-dir-1.0.0.tgz#9288e3e9e3cc3748717d39eade17cf71fc30ee6f" - integrity sha1-kojj6ePMN0hxfTnq3hfPcfww7m8= - dependencies: - commondir "^1.0.1" - make-dir "^1.0.0" - pkg-dir "^2.0.0" - find-cache-dir@^2.0.0, find-cache-dir@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/find-cache-dir/-/find-cache-dir-2.1.0.tgz#8d0f94cd13fe43c6c7c261a0d86115ca918c05f7" @@ -4880,13 +4871,6 @@ magic-string@^0.25.2: dependencies: sourcemap-codec "^1.4.4" -make-dir@^1.0.0: - version "1.3.0" - resolved "https://registry.yarnpkg.com/make-dir/-/make-dir-1.3.0.tgz#79c1033b80515bd6d24ec9933e860ca75ee27f0c" - integrity sha512-2w31R7SJtieJJnQtGc7RVL2StM2vGYVfqUOvUDxH6bC6aJTxPxTF0GnIgCyu7tjockiUWAYQRbxa7vKn34s5sQ== - dependencies: - pify "^3.0.0" - make-dir@^2.0.0: version "2.1.0" resolved "https://registry.yarnpkg.com/make-dir/-/make-dir-2.1.0.tgz#5f0310e18b8be898cc07009295a30ae41e91e6f5" @@ -5669,11 +5653,6 @@ pify@^2.0.0: version "2.3.0" resolved "https://registry.yarnpkg.com/pify/-/pify-2.3.0.tgz#ed141a6ac043a849ea588498e7dca8b15330e90c" -pify@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/pify/-/pify-3.0.0.tgz#e5a4acd2c101fdf3d9a4d07f0dbc4db49dd28176" - integrity sha1-5aSs0sEB/fPZpNB/DbxNtJ3SgXY= - pify@^4.0.1: version "4.0.1" resolved "https://registry.yarnpkg.com/pify/-/pify-4.0.1.tgz#4b2cd25c50d598735c50292224fd8c6df41e3231" @@ -7539,7 +7518,7 @@ wrappy@1: version "1.0.2" resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f" -write-file-atomic@2.3.0, write-file-atomic@^2.1.0: +write-file-atomic@^2.1.0: version "2.3.0" resolved "https://registry.yarnpkg.com/write-file-atomic/-/write-file-atomic-2.3.0.tgz#1ff61575c2e2a4e8e510d6fa4e243cce183999ab" integrity sha512-xuPeK4OdjWqtfi59ylvVL0Yn35SF3zgcAcv7rBPFHVaEapaDr4GdGgm3j7ckTwH9wHL7fGmgfAnb0+THrHb8tA==