feat: Remove gitDir option and resolve it automatically (#327)

* Remove `gitDir` config option
* Automatically resolve `.git` directory in the project

Closes #271

BREAKING CHANGE: `gitDir` option deprecated and will be ignored

README.md

@@ -99,7 +99,6 @@ To set options and keep lint-staged extensible, advanced format can be used. Thi
 ## Options
 
 * `linters` — `Object` — keys (`String`) are glob patterns, values (`Array<String> | String`) are commands to execute.
-* `gitDir` — Sets the relative path to the `.git` root. Useful when your `package.json` is located in a subdirectory. See [working from a subdirectory](#working-from-a-subdirectory)
 * `concurrent` — *true* — runs linters for each glob pattern simultaneously. If you don’t want this, you can set `concurrent: false`
 * `chunkSize` — Max allowed chunk size based on number of files for glob pattern. This is important on windows based systems to avoid command length limitations. See [#147](https://github.com/okonet/lint-staged/issues/147)
 * `subTaskConcurrency` — `1` — Controls concurrency for processing chunks generated for each linter. Execution is **not** concurrent by default(see [#225](https://github.com/okonet/lint-staged/issues/225))
@@ -153,19 +152,6 @@ Tools like ESLint/TSLint or stylefmt can reformat your code according to an appr
 
 ~~Starting from v3.1, lint-staged will stash you remaining changes (not added to the index) and restore them from stash afterwards. This allows you to create partial commits with hunks using `git add --patch`.~~ This is still [not resolved](https://github.com/okonet/lint-staged/issues/62)
 
-## Working from a subdirectory
-
-If your `package.json` is located in a subdirectory of the git root directory, you can use `gitDir` relative path to point there in order to make lint-staged work.
-
-```diff json
-{
-+   "gitDir": "../",
-    "linters":{
-        "*": "my-task"
-    }
-}
-```
-
 ## Examples
 
 All examples assuming you’ve already set up lint-staged and husky in the `package.json`.

package.json

@@ -33,6 +33,7 @@
     "cosmiconfig": "^3.1.0",
     "dedent": "^0.7.0",
     "execa": "^0.8.0",
+    "find-parent-dir": "^0.3.0",
     "is-glob": "^4.0.0",
     "jest-validate": "^21.1.0",
     "listr": "^0.13.0",
@@ -41,6 +42,7 @@
     "minimatch": "^3.0.0",
     "npm-which": "^3.0.1",
     "p-map": "^1.1.1",
+    "path-is-inside": "^1.0.2",
     "pify": "^3.0.0",
     "staged-git-files": "0.0.4",
     "stringify-object": "^3.2.0"

src/generateTasks.js

@@ -2,22 +2,33 @@
 
 const path = require('path')
 const minimatch = require('minimatch')
+const pathIsInside = require('path-is-inside')
 const getConfig = require('./getConfig').getConfig
 const resolveGitDir = require('./resolveGitDir')
 
-module.exports = function generateTasks(config, files) {
+module.exports = function generateTasks(config, relFiles) {
   const normalizedConfig = getConfig(config) // Ensure we have a normalized config
   const linters = normalizedConfig.linters
-  const gitDir = normalizedConfig.gitDir
   const globOptions = normalizedConfig.globOptions
+
+  const gitDir = resolveGitDir()
+  const cwd = process.cwd()
+  const files = relFiles.map(file => path.resolve(gitDir, file))
+
   return Object.keys(linters).map(pattern => {
     const commands = linters[pattern]
     const filter = minimatch.filter(pattern, globOptions)
+
     const fileList = files
+      // Only worry about children of the CWD
+      .filter(file => pathIsInside(file, cwd))
+      // Make the paths relative to CWD for filtering
+      .map(file => path.relative(cwd, file))
       // We want to filter before resolving paths
       .filter(filter)
       // Return absolute path after the filter is run
-      .map(file => path.resolve(resolveGitDir(gitDir), file))
+      .map(file => path.resolve(cwd, file))
+
     return {
       pattern,
       commands,

src/getConfig.js

@@ -15,12 +15,11 @@ const isGlob = require('is-glob')
 /**
  * Default config object
  *
- * @type {{concurrent: boolean, chunkSize: number, gitDir: string, globOptions: {matchBase: boolean, dot: boolean}, linters: {}, subTaskConcurrency: number, renderer: string, verbose: boolean}}
+ * @type {{concurrent: boolean, chunkSize: number, globOptions: {matchBase: boolean, dot: boolean}, linters: {}, subTaskConcurrency: number, renderer: string, verbose: boolean}}
  */
 const defaultConfig = {
   concurrent: true,
   chunkSize: Number.MAX_SAFE_INTEGER,
-  gitDir: '.',
   globOptions: {
     matchBase: true,
     dot: true
@@ -89,7 +88,7 @@ function unknownValidationReporter(config, example, option, options) {
  *
  * @param {Object} sourceConfig
  * @returns {{
- *  concurrent: boolean, chunkSize: number, gitDir: string, globOptions: {matchBase: boolean, dot: boolean}, linters: {}, subTaskConcurrency: number, renderer: string, verbose: boolean
+ *  concurrent: boolean, chunkSize: number, globOptions: {matchBase: boolean, dot: boolean}, linters: {}, subTaskConcurrency: number, renderer: string, verbose: boolean
  * }}
  */
 function getConfig(sourceConfig) {
@@ -120,8 +119,17 @@ function validateConfig(config) {
     }
   })
 
+  const deprecatedConfig = {
+    gitDir: () => `  Option ${chalk.bold('gitDir')} was removed.
+
+  lint-staged now automatically resolves '.git' directory.
+
+  Please remove ${chalk.bold('gitDir')} from your configuration.`
+  }
+
   const validation = validate(config, {
     exampleConfig,
+    deprecatedConfig,
     unknown: unknownValidationReporter,
     comment:
       'Please refer to https://github.com/okonet/lint-staged#configuration for more information...'

src/resolveGitDir.js

@@ -1,7 +1,7 @@
 'use strict'
 
-const path = require('path')
+const findParentDir = require('find-parent-dir')
 
-module.exports = function resolveGitDir(gitDir) {
-  return gitDir ? path.resolve(gitDir) : process.cwd()
+module.exports = function resolveGitDir() {
+  return findParentDir.sync(process.cwd(), '.git')
 }

src/runAll.js

@@ -16,7 +16,7 @@ const resolveGitDir = require('./resolveGitDir')
  */
 module.exports = function runAll(scripts, config) {
   // Config validation
-  if (!config || !has(config, 'gitDir') || !has(config, 'concurrent') || !has(config, 'renderer')) {
+  if (!config || !has(config, 'concurrent') || !has(config, 'renderer')) {
     throw new Error('Invalid config provided to runAll! Use getConfig instead.')
   }
 

src/runScript.js

@@ -8,12 +8,13 @@ const pMap = require('p-map')
 const getConfig = require('./getConfig').getConfig
 const calcChunkSize = require('./calcChunkSize')
 const findBin = require('./findBin')
+const resolveGitDir = require('./resolveGitDir')
 
 module.exports = function runScript(commands, pathsToLint, scripts, config) {
   const normalizedConfig = getConfig(config)
   const chunkSize = normalizedConfig.chunkSize
   const concurrency = normalizedConfig.subTaskConcurrency
-  const gitDir = normalizedConfig.gitDir
+  const gitDir = resolveGitDir()
 
   const filePathChunks = chunk(pathsToLint, calcChunkSize(pathsToLint, chunkSize))
 
@@ -28,7 +29,7 @@ module.exports = function runScript(commands, pathsToLint, scripts, config) {
         // Only use gitDir as CWD if we are using the git binary
         // e.g `npm` should run tasks in the actual CWD
         const execaOptions =
-          /git(\.exe)?$/i.test(res.bin) && config && gitDir ? { cwd: gitDir } : {}
+          /git(\.exe)?$/i.test(res.bin) && gitDir !== process.cwd() ? { cwd: gitDir } : {}
 
         const errors = []
         const mapper = pathsChunk => {

test/__snapshots__/getConfig.spec.js.snap

@@ -4,7 +4,6 @@ exports[`getConfig should return config with defaults 1`] = `
 Object {
   "chunkSize": 9007199254740991,
   "concurrent": true,
-  "gitDir": ".",
   "globOptions": Object {
     "dot": true,
     "matchBase": true,
@@ -20,7 +19,6 @@ exports[`getConfig should return config with defaults for undefined 1`] = `
 Object {
   "chunkSize": 9007199254740991,
   "concurrent": true,
-  "gitDir": ".",
   "globOptions": Object {
     "dot": true,
     "matchBase": true,
@@ -36,7 +34,6 @@ exports[`getConfig should set linters 1`] = `
 Object {
   "chunkSize": 9007199254740991,
   "concurrent": true,
-  "gitDir": ".",
   "globOptions": Object {
     "dot": true,
     "matchBase": true,
@@ -75,6 +72,19 @@ WARN ● Validation Warning:
 Please refer to https://github.com/okonet/lint-staged#configuration for more information..."
 `;
 
+exports[`validateConfig should print deprecation warning for gitDir option 1`] = `
+"
+WARN ● Deprecation Warning:
+
+  Option gitDir was removed.
+
+  lint-staged now automatically resolves '.git' directory.
+
+  Please remove gitDir from your configuration.
+
+Please refer to https://github.com/okonet/lint-staged#configuration for more information..."
+`;
+
 exports[`validateConfig should throw and should print validation errors for invalid config 1`] = `
 "● Validation Error:
 

test/__snapshots__/index.spec.js.snap

@@ -10,7 +10,6 @@ LOG {
   },
   concurrent: true,
   chunkSize: 9007199254740991,
-  gitDir: '.',
   globOptions: {
     matchBase: true,
     dot: true
@@ -32,7 +31,6 @@ LOG {
   },
   concurrent: true,
   chunkSize: 9007199254740991,
-  gitDir: '.',
   globOptions: {
     matchBase: true,
     dot: true

test/generateTasks.spec.js

@@ -1,6 +1,7 @@
 import path from 'path'
 import os from 'os'
 import generateTasks from '../src/generateTasks'
+import resolveGitDir from '../src/resolveGitDir'
 
 const files = [
   'test.js',
@@ -22,10 +23,12 @@ const files = [
   '.hidden/test.txt'
 ]
 
+// Mocks get hoisted
+jest.mock('../src/resolveGitDir.js')
 const workDir = path.join(os.tmpdir(), 'tmp-lint-staged')
+resolveGitDir.mockReturnValue(workDir)
 
 const config = {
-  gitDir: workDir,
   linters: {
     '*.js': 'root-js',
     '**/*.js': 'any-js',
@@ -37,6 +40,14 @@ const config = {
 }
 
 describe('generateTasks', () => {
+  beforeEach(() => {
+    jest.spyOn(process, 'cwd').mockReturnValue(workDir)
+  })
+
+  afterEach(() => {
+    process.cwd.mockRestore()
+  })
+
   it('should work with simple configuration', () => {
     const result = generateTasks(
       {
@@ -77,8 +88,21 @@ describe('generateTasks', () => {
     })
   })
 
+  it('should not match non-children files', () => {
+    const relPath = path.resolve(path.join(process.cwd(), '..'))
+    resolveGitDir.mockReturnValueOnce(relPath)
+    const result = generateTasks(Object.assign({}, config), files)
+    const linter = result.find(item => item.pattern === '*.js')
+    expect(linter).toEqual({
+      pattern: '*.js',
+      commands: 'root-js',
+      fileList: []
+    })
+  })
+
   it('should return an empty file list for linters with no matches.', () => {
     const result = generateTasks(config, files)
+
     result.forEach(task => {
       if (task.commands === 'unknown-js') {
         expect(task.fileList.length).toEqual(0)
@@ -88,23 +112,6 @@ describe('generateTasks', () => {
     })
   })
 
-  it('should match pattern "*.js" for relative path', () => {
-    const relPath = path.resolve(path.join(process.cwd(), '..'))
-    const result = generateTasks(Object.assign({}, config, { gitDir: '..' }), files)
-    const linter = result.find(item => item.pattern === '*.js')
-    expect(linter).toEqual({
-      pattern: '*.js',
-      commands: 'root-js',
-      fileList: [
-        `${relPath}/test.js`,
-        `${relPath}/deeper/test.js`,
-        `${relPath}/deeper/test2.js`,
-        `${relPath}/even/deeper/test.js`,
-        `${relPath}/.hidden/test.js`
-      ].map(path.normalize)
-    })
-  })
-
   it('should match pattern "*.js"', () => {
     const result = generateTasks(config, files)
     const linter = result.find(item => item.pattern === '*.js')

test/getConfig.spec.js

@@ -105,24 +105,6 @@ describe('getConfig', () => {
     )
   })
 
-  it('should set gitDir', () => {
-    expect(getConfig({})).toEqual(
-      expect.objectContaining({
-        gitDir: '.'
-      })
-    )
-
-    expect(
-      getConfig({
-        gitDir: '../path'
-      })
-    ).toEqual(
-      expect.objectContaining({
-        gitDir: '../path'
-      })
-    )
-  })
-
   it('should set linters', () => {
     expect(getConfig()).toEqual(
       expect.objectContaining({
@@ -192,7 +174,6 @@ describe('getConfig', () => {
     const src = {
       concurrent: false,
       chunkSize: 2,
-      gitDir: '/to',
       globOptions: {
         matchBase: false,
         dot: true
@@ -216,21 +197,31 @@ describe('validateConfig', () => {
   it('should throw and should print validation errors for invalid config', () => {
     const invalidAdvancedConfig = {
       foo: false,
-      chunkSize: 'string',
-      gitDir: 111
+      chunkSize: 'string'
     }
     expect(() => validateConfig(getConfig(invalidAdvancedConfig))).toThrowErrorMatchingSnapshot()
   })
 
   it('should not throw and should print validation warnings for mixed config', () => {
     const invalidMixedConfig = {
-      gitDir: './path/to/packagejson/',
+      concurrent: false,
       '*.js': ['eslint --fix', 'git add']
     }
     expect(() => validateConfig(getConfig(invalidMixedConfig))).not.toThrow()
     expect(console.printHistory()).toMatchSnapshot()
   })
 
+  it('should print deprecation warning for gitDir option', () => {
+    const configWithDeprecatedOpt = {
+      gitDir: '../',
+      linters: {
+        '*.js': ['eslint --fix', 'git add']
+      }
+    }
+    expect(() => validateConfig(getConfig(configWithDeprecatedOpt))).not.toThrow()
+    expect(console.printHistory()).toMatchSnapshot()
+  })
+
   it('should not throw and should print nothing for simple valid config', () => {
     const validSimpleConfig = {
       '*.js': ['eslint --fix', 'git add']
@@ -241,7 +232,7 @@ describe('validateConfig', () => {
 
   it('should not throw and should print nothing for advanced valid config', () => {
     const validAdvancedConfig = {
-      gitDir: '.',
+      concurrent: false,
       linters: {
         '*.js': ['eslint --fix', 'git add']
       }