From 848c7aa1e4d2966467f6ad6cd8d802f824d00245 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Thu, 12 Oct 2017 16:30:04 -0600 Subject: [PATCH] feat: add opt-out capability for the auto-formatting (#6) --- CONTRIBUTING.md | 12 +++++++- src/config/__tests__/lintstagedrc.js | 41 ++++++++++++++++++++++++++++ src/config/lintstagedrc.js | 8 +++--- src/scripts/precommit/index.js | 13 ++------- src/utils.js | 18 ++++++++++++ 5 files changed, 76 insertions(+), 16 deletions(-) create mode 100644 src/config/__tests__/lintstagedrc.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0ca9c187..960ff204 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,7 +55,7 @@ Please make sure to run the tests before you commit your changes. You can run `npm run test:update` which will update any snapshots that need updating. Make sure to include those changes (if they exist) in your commit. -### opt into git hooks +### opt in/out of git hooks There are git hooks set up with this project that are automatically installed when you install dependencies. They're really handy, but are turned off by @@ -67,6 +67,16 @@ inside: pre-commit ``` +One of the things that the git hooks does is automatically format the files you +change. It does this by reformating the entire file and running `git add` on +the file after. This breaks workflows where you're trying to commit portions of +the file only. You can always run your commit with `--no-verify`, but if this +is a bummer to your workflow, you can add an `.opt-out` file with the contents: + +``` +autoformat +``` + ## Help needed Please checkout the [the open issues][issues] diff --git a/src/config/__tests__/lintstagedrc.js b/src/config/__tests__/lintstagedrc.js new file mode 100644 index 00000000..142dcfc6 --- /dev/null +++ b/src/config/__tests__/lintstagedrc.js @@ -0,0 +1,41 @@ +import * as utilsMock from '../../utils' + +jest.mock('../../utils', () => ({ + ...require.requireActual('../../utils'), + isOptedOut: jest.fn((key, t) => t), +})) + +afterEach(() => { + jest.resetModules() +}) + +test('includes format and git add when not opted out', () => { + utilsMock.isOptedOut.mockImplementation((key, t, f) => f) + const config = require('../lintstagedrc') + const jsLinter = getJsLinter(config.linters) + expect(hasFormat(jsLinter)).toBe(true) + expect(hasGitAdd(jsLinter)).toBe(true) +}) + +test('does not include format and git add when opted out', () => { + utilsMock.isOptedOut.mockImplementation((key, t) => t) + const config = require('../lintstagedrc') + const jsLinter = getJsLinter(config.linters) + expect(hasFormat(jsLinter)).toBe(false) + expect(hasGitAdd(jsLinter)).toBe(false) +}) + +function hasFormat(linter) { + return linter.some(l => l.includes('format')) +} + +function hasGitAdd(linter) { + return linter.includes('git add') +} + +function getJsLinter(linters) { + const key = Object.keys(linters).find( + k => k.includes('**') && k.includes('js'), + ) + return linters[key] +} diff --git a/src/config/lintstagedrc.js b/src/config/lintstagedrc.js index 1d39798c..ca3997e1 100644 --- a/src/config/lintstagedrc.js +++ b/src/config/lintstagedrc.js @@ -1,4 +1,4 @@ -const {resolveKcdScripts, resolveBin} = require('../utils') +const {resolveKcdScripts, resolveBin, isOptedOut} = require('../utils') const kcdScripts = resolveKcdScripts() const doctoc = resolveBin('doctoc') @@ -7,11 +7,11 @@ module.exports = { concurrent: false, linters: { '**/*.+(js|json|less|css|ts)': [ - `${kcdScripts} format`, + isOptedOut('autoformat', null, `${kcdScripts} format`), `${kcdScripts} lint`, `${kcdScripts} test --findRelatedTests`, - 'git add', - ], + isOptedOut('autoformat', null, 'git add'), + ].filter(Boolean), '.all-contributorsrc': [ `${kcdScripts} contributors generate`, 'git add README.md', diff --git a/src/scripts/precommit/index.js b/src/scripts/precommit/index.js index ba8a779d..991456d5 100644 --- a/src/scripts/precommit/index.js +++ b/src/scripts/precommit/index.js @@ -1,6 +1,5 @@ -const fs = require('fs') const spawn = require('cross-spawn') -const {fromRoot} = require('../../utils') +const {isOptedIn} = require('../../utils') const [executor, ...args] = process.argv @@ -10,7 +9,7 @@ const lintStagedResult = spawn.sync( {stdio: 'inherit'}, ) -if (lintStagedResult.status !== 0 || !isOptedIntoValidate()) { +if (lintStagedResult.status !== 0 || !isOptedIn('pre-commit')) { process.exit(lintStagedResult.status) } else { const validateResult = spawn.sync('npm', ['run', 'validate'], { @@ -19,11 +18,3 @@ if (lintStagedResult.status !== 0 || !isOptedIntoValidate()) { process.exit(validateResult.status) } - -function isOptedIntoValidate() { - if (!fs.existsSync(fromRoot('.opt-in'))) { - return false - } - const contents = fs.readFileSync(fromRoot('.opt-in'), 'utf-8') - return contents.includes('pre-commit') -} diff --git a/src/utils.js b/src/utils.js index dc49e0dd..b2d47ca3 100644 --- a/src/utils.js +++ b/src/utils.js @@ -115,7 +115,25 @@ function getConcurrentlyArgs(scripts, {killOthers = true} = {}) { ].filter(Boolean) } +function isOptedOut(key, t = true, f = false) { + if (!fs.existsSync(fromRoot('.opt-out'))) { + return f + } + const contents = fs.readFileSync(fromRoot('.opt-out'), 'utf-8') + return contents.includes(key) ? t : f +} + +function isOptedIn(key, t = true, f = false) { + if (!fs.existsSync(fromRoot('.opt-in'))) { + return f + } + const contents = fs.readFileSync(fromRoot('.opt-in'), 'utf-8') + return contents.includes(key) ? t : f +} + module.exports = { + isOptedOut, + isOptedIn, ifDevDep, ifPeerDep, ifScript,