diff --git a/src/findBin.js b/src/findBin.js index c81a85d4b..684d7b4b2 100644 --- a/src/findBin.js +++ b/src/findBin.js @@ -2,16 +2,50 @@ const npmWhich = require('npm-which')(process.cwd()) -module.exports = function findBin(cmd, packageJson, options) { +module.exports = function findBin(cmd, scripts, options) { + const npmArgs = (bin, args) => + ['run', options && options.verbose ? undefined : '--silent', bin] + // args could be undefined but we filter that out. + .concat(args) + .filter(arg => arg !== undefined) + /* - * If package.json has script with cmd defined - * we want it to be executed first - */ - if (packageJson.scripts && packageJson.scripts[cmd] !== undefined) { + * If package.json has script with cmd defined we want it to be executed + * first. For finding the bin from scripts defined in package.json, there + * are 2 possibilities. It's a command which does not have any arguments + * passed to it in the lint-staged config. Or it could be something like + * `kcd-scripts` which has arguments such as `format`, `lint` passed to it. + * But we always cannot assume that the cmd, which has a space in it, is of + * the format `bin argument` because it is legal to define a package.json + * script with space in it's name. So we do 2 types of lookup. First a naive + * lookup which just looks for the scripts entry with cmd. Then we split on + * space, parse the bin and args, and try again. + * + * Related: + * - https://github.com/kentcdodds/kcd-scripts/pull/3 + * - https://github.com/okonet/lint-staged/issues/294 + * + * Example: + * + * "scripts": { + * "my cmd": "echo deal-wth-it", + * "demo-bin": "node index.js" + * }, + * "lint-staged": { + * "*.js": ["my cmd", "demo-bin hello"] + * } + */ + if (scripts[cmd] !== undefined) { // Support for scripts from package.json - const args = ['run', options && options.verbose ? undefined : '--silent', cmd].filter(Boolean) + return { bin: 'npm', args: npmArgs(cmd) } + } - return { bin: 'npm', args } + const parts = cmd.split(' ') + let bin = parts[0] + const args = parts.splice(1) + + if (scripts[bin] !== undefined) { + return { bin: 'npm', args: npmArgs(bin, args) } } /* @@ -32,10 +66,6 @@ module.exports = function findBin(cmd, packageJson, options) { * } */ - const parts = cmd.split(' ') - let bin = parts[0] - const args = parts.splice(1) - try { /* npm-which tries to resolve the bin in local node_modules/.bin */ /* and if this fails it look in $PATH */ diff --git a/src/index.js b/src/index.js index 4084af3e2..b9e22ba2c 100644 --- a/src/index.js +++ b/src/index.js @@ -44,7 +44,9 @@ ${stringifyObject(config)} `) } - runAll(packageJson, config) + const scripts = packageJson.scripts || {} + + runAll(scripts, config) .then(() => { // No errors, exiting with 0 process.exitCode = 0 diff --git a/src/runAll.js b/src/runAll.js index a2cc94afa..04038a62c 100644 --- a/src/runAll.js +++ b/src/runAll.js @@ -9,11 +9,11 @@ const resolveGitDir = require('./resolveGitDir') /** * Executes all tasks and either resolves or rejects the promise - * @param packageJson + * @param scripts * @param config {Object} * @returns {Promise} */ -module.exports = function runAll(packageJson, config) { +module.exports = function runAll(scripts, config) { // Config validation if (!config || !has(config, 'gitDir') || !has(config, 'concurrent') || !has(config, 'renderer')) { throw new Error('Invalid config provided to runAll! Use getConfig instead.') @@ -35,7 +35,7 @@ module.exports = function runAll(packageJson, config) { const tasks = generateTasks(config, filenames).map(task => ({ title: `Running tasks for ${task.pattern}`, task: () => - new Listr(runScript(task.commands, task.fileList, packageJson, config), { + new Listr(runScript(task.commands, task.fileList, scripts, config), { // In sub-tasks we don't want to run concurrently // and we want to abort on errors concurrent: false, diff --git a/src/runScript.js b/src/runScript.js index 71848d828..e19aba304 100644 --- a/src/runScript.js +++ b/src/runScript.js @@ -8,7 +8,7 @@ const getConfig = require('./getConfig').getConfig const calcChunkSize = require('./calcChunkSize') const findBin = require('./findBin') -module.exports = function runScript(commands, pathsToLint, packageJson, config) { +module.exports = function runScript(commands, pathsToLint, scripts, config) { const normalizedConfig = getConfig(config) const chunkSize = normalizedConfig.chunkSize const concurrency = normalizedConfig.subTaskConcurrency @@ -22,7 +22,7 @@ module.exports = function runScript(commands, pathsToLint, packageJson, config) title: linter, task: () => { try { - const res = findBin(linter, packageJson, config) + const res = findBin(linter, scripts, config) const separatorArgs = /npm(\.exe)?$/i.test(res.bin) ? ['--'] : [] diff --git a/test/findBin.spec.js b/test/findBin.spec.js index c9d1d710a..9ec5ef7a7 100644 --- a/test/findBin.spec.js +++ b/test/findBin.spec.js @@ -2,50 +2,47 @@ import findBin from '../src/findBin' jest.mock('npm-which') -const packageJSON = { - scripts: { - test: 'noop' - }, - 'lint-staged': {} -} +const scripts = { test: 'noop' } describe('findBin', () => { it('should favor `npm run` command if exists in both package.json and .bin/', () => { - const packageJSONMock = { - scripts: { - 'my-linter': 'my-linter' - } - } - const { bin, args } = findBin('my-linter', packageJSONMock) + const { bin, args } = findBin('my-linter', { 'my-linter': 'my-linter' }) expect(bin).toEqual('npm') expect(args).toEqual(['run', '--silent', 'my-linter']) }) it('should return npm run command without --silent in verbose mode', () => { - const packageJSONMock = { - scripts: { - eslint: 'eslint' - } - } - const { bin, args } = findBin('eslint', packageJSONMock, { verbose: true }) + const { bin, args } = findBin('eslint', { eslint: 'eslint' }, { verbose: true }) expect(bin).toEqual('npm') expect(args).toEqual(['run', 'eslint']) }) + it('should resolve cmd defined in scripts with args', () => { + const { bin, args } = findBin('kcd-scripts format', { 'kcd-scripts': 'node index.js' }) + expect(bin).toEqual('npm') + expect(args).toEqual(['run', '--silent', 'kcd-scripts', 'format']) + }) + + it('should resolve cmd defined in scripts with space in name', () => { + const { bin, args } = findBin('my cmd', { 'my cmd': 'echo deal-with-it' }) + expect(bin).toEqual('npm') + expect(args).toEqual(['run', '--silent', 'my cmd']) + }) + it('should return path to bin if there is no `script` with name in package.json', () => { - const { bin, args } = findBin('my-linter', packageJSON) + const { bin, args } = findBin('my-linter', scripts) expect(bin).toEqual('my-linter') expect(args).toEqual([]) }) it('should throw an error if bin not found and there is no entry in scripts section', () => { expect(() => { - findBin('my-missing-linter', packageJSON) + findBin('my-missing-linter', scripts) }).toThrow('my-missing-linter could not be found. Try `npm install my-missing-linter`.') }) it('should parse cmd and add arguments to args', () => { - const { bin, args } = findBin('my-linter task --fix', packageJSON) + const { bin, args } = findBin('my-linter task --fix', scripts) expect(bin).toEqual('my-linter') expect(args).toEqual(['task', '--fix']) }) diff --git a/test/runAll.spec.js b/test/runAll.spec.js index c37b85b5b..255ed04cb 100644 --- a/test/runAll.spec.js +++ b/test/runAll.spec.js @@ -8,35 +8,31 @@ sgfMock.mockImplementation((params, callback) => { callback(null, []) }) -const packageJson = { - scripts: { - mytask: 'echo "Running task"' - } -} +const scripts = { mytask: 'echo "Running task"' } describe('runAll', () => { afterEach(() => { sgfMock.mockClear() }) it('should throw when invalid config is provided', () => { - expect(() => runAll(packageJson, {})).toThrowErrorMatchingSnapshot() - expect(() => runAll(packageJson)).toThrowErrorMatchingSnapshot() + expect(() => runAll(scripts, {})).toThrowErrorMatchingSnapshot() + expect(() => runAll(scripts)).toThrowErrorMatchingSnapshot() }) it('should not throw when a valid config is provided', () => { const config = getConfig({ concurrent: false }) - expect(() => runAll(packageJson, config)).not.toThrow() + expect(() => runAll(scripts, config)).not.toThrow() }) it('should return a promise', () => { - expect(runAll(packageJson, getConfig({}))).toBeInstanceOf(Promise) + expect(runAll(scripts, getConfig({}))).toBeInstanceOf(Promise) }) it('should resolve the promise with no tasks', () => { expect.assertions(1) - return expect(runAll(packageJson, getConfig({}))).resolves.toEqual('No tasks to run.') + return expect(runAll(scripts, getConfig({}))).resolves.toEqual('No tasks to run.') }) it('should reject the promise when staged-git-files errors', () => { @@ -44,6 +40,6 @@ describe('runAll', () => { callback('test', undefined) }) expect.assertions(1) - return expect(runAll(packageJson, getConfig({}))).rejects.toEqual('test') + return expect(runAll(scripts, getConfig({}))).rejects.toEqual('test') }) }) diff --git a/test/runScript.spec.js b/test/runScript.spec.js index 3d9ca7b89..05768274d 100644 --- a/test/runScript.spec.js +++ b/test/runScript.spec.js @@ -4,12 +4,9 @@ import runScript from '../src/runScript' jest.mock('execa') -const packageJSON = { - scripts: { - test: 'noop', - test2: 'noop' - }, - 'lint-staged': {} +const scripts = { + test: 'noop', + test2: 'noop' } describe('runScript', () => { @@ -18,17 +15,17 @@ describe('runScript', () => { }) it('should return an array', () => { - expect(runScript('test', ['test.js'], packageJSON)).toBeInstanceOf(Array) + expect(runScript('test', ['test.js'], scripts)).toBeInstanceOf(Array) }) it('should throw for non-existend script', () => { expect(() => { - runScript('missing-module', ['test.js'], packageJSON)[0].task() + runScript('missing-module', ['test.js'], scripts)[0].task() }).toThrow() }) it('should work with a single command', async () => { - const res = runScript('test', ['test.js'], packageJSON) + const res = runScript('test', ['test.js'], scripts) expect(res.length).toBe(1) expect(res[0].title).toBe('test') expect(res[0].task).toBeInstanceOf(Function) @@ -38,7 +35,7 @@ describe('runScript', () => { }) it('should work with multiple commands', async () => { - const res = runScript(['test', 'test2'], ['test.js'], packageJSON) + const res = runScript(['test', 'test2'], ['test.js'], scripts) expect(res.length).toBe(2) expect(res[0].title).toBe('test') expect(res[1].title).toBe('test2') @@ -56,7 +53,7 @@ describe('runScript', () => { }) it('should respect chunk size', async () => { - const res = runScript(['test'], ['test1.js', 'test2.js'], packageJSON, { + const res = runScript(['test'], ['test1.js', 'test2.js'], scripts, { chunkSize: 1 }) const taskPromise = res[0].task() @@ -68,7 +65,7 @@ describe('runScript', () => { }) it('should support non npm scripts', async () => { - const res = runScript(['node --arg=true ./myscript.js', 'git add'], ['test.js'], packageJSON) + const res = runScript(['node --arg=true ./myscript.js', 'git add'], ['test.js'], scripts) expect(res.length).toBe(2) expect(res[0].title).toBe('node --arg=true ./myscript.js') expect(res[1].title).toBe('git add') @@ -89,7 +86,7 @@ describe('runScript', () => { }) it('should pass cwd to execa if gitDir option is set for non-npm tasks', async () => { - const res = runScript(['test', 'git add'], ['test.js'], packageJSON, { gitDir: '../' }) + const res = runScript(['test', 'git add'], ['test.js'], scripts, { gitDir: '../' }) let taskPromise = res[0].task() expect(taskPromise).toBeInstanceOf(Promise) await taskPromise @@ -106,7 +103,7 @@ describe('runScript', () => { }) it('should not pass `gitDir` as `cwd` to `execa()` if a non-git binary is called', async () => { - const res = runScript(['jest'], ['test.js'], packageJSON, { gitDir: '../' }) + const res = runScript(['jest'], ['test.js'], scripts, { gitDir: '../' }) const taskPromise = res[0].task() expect(taskPromise).toBeInstanceOf(Promise) await taskPromise @@ -115,7 +112,7 @@ describe('runScript', () => { }) it('should use --silent in non-verbose mode', async () => { - const res = runScript('test', ['test.js'], packageJSON, { verbose: false }) + const res = runScript('test', ['test.js'], scripts, { verbose: false }) const taskPromise = res[0].task() expect(taskPromise).toBeInstanceOf(Promise) await taskPromise @@ -124,7 +121,7 @@ describe('runScript', () => { }) it('should not use --silent in verbose mode', async () => { - const res = runScript('test', ['test.js'], packageJSON, { verbose: true }) + const res = runScript('test', ['test.js'], scripts, { verbose: true }) const taskPromise = res[0].task() expect(taskPromise).toBeInstanceOf(Promise) await taskPromise @@ -138,7 +135,7 @@ describe('runScript', () => { linterErr.stderr = '' mockFn.mockImplementationOnce(() => Promise.reject(linterErr)) - const res = runScript('mock-fail-linter', ['test.js'], packageJSON) + const res = runScript('mock-fail-linter', ['test.js'], scripts) const taskPromise = res[0].task() try { await taskPromise