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

remove gitDir config option #327

Merged
merged 9 commits into from
Nov 11, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
14 changes: 0 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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`.
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"
Expand Down
17 changes: 14 additions & 3 deletions src/generateTasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions src/getConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions src/resolveGitDir.js
Original file line number Diff line number Diff line change
@@ -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')
}
2 changes: 1 addition & 1 deletion src/runAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
}

Expand Down
5 changes: 3 additions & 2 deletions src/runScript.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand All @@ -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 => {
Expand Down
3 changes: 0 additions & 3 deletions test/__snapshots__/getConfig.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ exports[`getConfig should return config with defaults 1`] = `
Object {
"chunkSize": 9007199254740991,
"concurrent": true,
"gitDir": ".",
"globOptions": Object {
"dot": true,
"matchBase": true,
Expand All @@ -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,
Expand All @@ -36,7 +34,6 @@ exports[`getConfig should set linters 1`] = `
Object {
"chunkSize": 9007199254740991,
"concurrent": true,
"gitDir": ".",
"globOptions": Object {
"dot": true,
"matchBase": true,
Expand Down
2 changes: 0 additions & 2 deletions test/__snapshots__/index.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ LOG {
},
concurrent: true,
chunkSize: 9007199254740991,
gitDir: '.',
globOptions: {
matchBase: true,
dot: true
Expand All @@ -32,7 +31,6 @@ LOG {
},
concurrent: true,
chunkSize: 9007199254740991,
gitDir: '.',
globOptions: {
matchBase: true,
dot: true
Expand Down
43 changes: 25 additions & 18 deletions test/generateTasks.spec.js
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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',
Expand All @@ -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(
{
Expand Down Expand Up @@ -77,8 +88,21 @@ describe('generateTasks', () => {
})
})

it('should not match non-children files', () => {
const relPath = path.resolve(path.join(process.cwd(), '..'))
resolveGitDir.mockReturnValueOnce(relPath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@okonet check for filtering out files that aren't in the cwd but show up because gitDir is diff

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)
Expand All @@ -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')
Expand Down
26 changes: 3 additions & 23 deletions test/getConfig.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -192,7 +174,6 @@ describe('getConfig', () => {
const src = {
concurrent: false,
chunkSize: 2,
gitDir: '/to',
globOptions: {
matchBase: false,
dot: true
Expand All @@ -216,15 +197,14 @@ 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()
Expand All @@ -241,7 +221,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']
}
Expand Down
15 changes: 2 additions & 13 deletions test/resolveGitDir.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import os from 'os'
import path from 'path'
import resolveGitDir from '../src/resolveGitDir'

Expand All @@ -10,17 +9,7 @@ describe('resolveGitDir', () => {
})
it('should resolve to current working dir if set to default', () => {
const expected = path.resolve(process.cwd())
expect(resolveGitDir('.')).toEqual(expected)
expect(path.isAbsolute(resolveGitDir('.'))).toBe(true)
})
it('should resolve to relative dir from config', () => {
const expected = path.resolve(path.join(process.cwd(), '..'))
expect(resolveGitDir('..')).toEqual(expected)
expect(path.isAbsolute(resolveGitDir('..'))).toBe(true)
})
it('should resolve to absolute dir from config', () => {
const workDir = path.join(os.tmpdir(), 'tmp-lint-staged')
expect(resolveGitDir(workDir)).toEqual(workDir)
expect(path.isAbsolute(resolveGitDir(workDir))).toBe(true)
expect(resolveGitDir()).toEqual(expected)
expect(path.isAbsolute(resolveGitDir())).toBe(true)
})
})
6 changes: 4 additions & 2 deletions test/runScript-mock-findBin.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import mockFn from 'execa'
import runScript from '../src/runScript'

jest.mock('execa')
jest.mock('../src/resolveGitDir', () => () => '../')

// Mock findBin to return an absolute path
jest.mock('../src/findBin', () => commands => {
const [bin, ...otherArgs] = commands.split(' ')
Expand All @@ -28,8 +30,8 @@ describe('runScript with absolute paths', () => {
mockFn.mockClear()
})

it('can pass `gitDir` as `cwd` to `execa()` when git is called via absolute path', async () => {
const res = runScript(['git add'], ['test.js'], packageJSON, { gitDir: '../' })
it('passes `gitDir` as `cwd` to `execa()` when git is called via absolute path', async () => {
const res = runScript(['git add'], ['test.js'], packageJSON)
const taskPromise = res[0].task()
expect(taskPromise).toBeInstanceOf(Promise)
await taskPromise
Expand Down