Skip to content

Commit

Permalink
refactor: use execa.command to avoid parsing of commands
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Nov 14, 2019
1 parent a2b9716 commit effdb4c
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 57 deletions.
1 change: 0 additions & 1 deletion package.json
Expand Up @@ -43,7 +43,6 @@
"micromatch": "^4.0.2",
"normalize-path": "^3.0.0",
"please-upgrade-node": "^3.1.1",
"string-argv": "^0.3.0",
"stringify-object": "^3.3.0"
},
"devDependencies": {
Expand Down
55 changes: 13 additions & 42 deletions src/resolveTaskFn.js
Expand Up @@ -4,27 +4,9 @@ const chalk = require('chalk')
const dedent = require('dedent')
const execa = require('execa')
const symbols = require('log-symbols')
const stringArgv = require('string-argv')

const debug = require('debug')('lint-staged:task')

/**
* Execute the given linter cmd using execa and
* return the promise.
*
* @param {string} cmd
* @param {Array<string>} args
* @param {Object} execaOptions
* @return {Promise} child_process
*/
const execLinter = (cmd, args, execaOptions) => {
debug('cmd:', cmd)
if (args) debug('args:', args)
debug('execaOptions:', execaOptions)

return args ? execa(cmd, args, execaOptions) : execa(cmd, execaOptions)
}

const successMsg = linter => `${symbols.success} ${linter} passed!`

/**
Expand Down Expand Up @@ -73,49 +55,38 @@ function makeErr(linter, result, context = {}) {
}

/**
* Returns the task function for the linter. It handles chunking for file paths
* if the OS is Windows.
* Returns the task function for the linter.
*
* @param {Object} options
* @param {string} options.command — Linter task
* @param {String} options.gitDir - Current git repo path
* @param {Boolean} options.isFn - Whether the linter task is a function
* @param {Array<string>} options.pathsToLint — Filepaths to run the linter task against
* @param {Array<string>} options.files — Filepaths to run the linter task against
* @param {Boolean} [options.relative] — Whether the filepaths should be relative
* @param {Boolean} [options.shell] — Whether to skip parsing linter task for better shell support
* @returns {function(): Promise<Array<string>>}
*/
module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative, shell = false }) {
const execaOptions = { preferLocal: true, reject: false, shell }
const cmd = isFn ? command : `${command} ${files.join(' ')}`
debug('cmd:', cmd)

const execaOptions = { preferLocal: true, reject: false, shell }
if (relative) {
execaOptions.cwd = process.cwd()
} else if (/^git(\.exe)?/i.test(command) && gitDir !== process.cwd()) {
// Only use gitDir as CWD if we are using the git binary
// e.g `npm` should run tasks in the actual CWD
execaOptions.cwd = gitDir
}
debug('execaOptions:', execaOptions)

let cmd
let args

if (shell) {
execaOptions.shell = true
// If `shell`, passed command shouldn't be parsed
// If `linter` is a function, command already includes `files`.
cmd = isFn ? command : `${command} ${files.join(' ')}`
} else {
const [parsedCmd, ...parsedArgs] = stringArgv.parseArgsStringToArgv(command)
cmd = parsedCmd
args = isFn ? parsedArgs : parsedArgs.concat(files)
}
return async ctx => {
const result = await execa.command(cmd, execaOptions)

return ctx =>
execLinter(cmd, args, execaOptions).then(result => {
if (result.failed || result.killed || result.signal != null) {
throw makeErr(command, result, ctx)
}
if (result.failed || result.killed || result.signal != null) {
throw makeErr(command, result, ctx)
}

return successMsg(command)
})
return successMsg(command)
}
}
6 changes: 5 additions & 1 deletion test/__mocks__/execa.js
@@ -1,4 +1,4 @@
module.exports = jest.fn(() =>
const execa = jest.fn(() =>
Promise.resolve({
stdout: 'a-ok',
stderr: '',
Expand All @@ -9,3 +9,7 @@ module.exports = jest.fn(() =>
signal: null
})
)

execa.command = execa

module.exports = execa
2 changes: 1 addition & 1 deletion test/index.spec.js
Expand Up @@ -161,7 +161,7 @@ describe('lintStaged', () => {
}
mockCosmiconfigWith({ config })
getStagedFiles.mockImplementationOnce(async () => ['sample.java'])
const passed = await lintStaged({ quiet: true }, logger)
const passed = await lintStaged({ quiet: true, shell: true }, logger)
expect(logger.printHistory()).toMatchSnapshot()
expect(passed).toBe(false)
})
Expand Down
4 changes: 2 additions & 2 deletions test/makeCmdTasks.spec.js
Expand Up @@ -41,7 +41,7 @@ describe('makeCmdTasks', () => {
expect(taskPromise).toBeInstanceOf(Promise)
await taskPromise
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('test', ['test.js'], {
expect(execa).lastCalledWith('test test.js', {
preferLocal: true,
reject: false,
shell: false
Expand All @@ -50,7 +50,7 @@ describe('makeCmdTasks', () => {
expect(taskPromise).toBeInstanceOf(Promise)
await taskPromise
expect(execa).toHaveBeenCalledTimes(2)
expect(execa).lastCalledWith('test2', ['test.js'], {
expect(execa).lastCalledWith('test2 test.js', {
preferLocal: true,
reject: false,
shell: false
Expand Down
10 changes: 5 additions & 5 deletions test/resolveTaskFn.spec.js
Expand Up @@ -17,7 +17,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
preferLocal: true,
reject: false,
shell: false
Expand All @@ -34,7 +34,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
preferLocal: true,
reject: false,
shell: false
Expand Down Expand Up @@ -86,7 +86,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git', ['add', 'test.js'], {
expect(execa).lastCalledWith('git add test.js', {
cwd: '../',
preferLocal: true,
reject: false,
Expand All @@ -100,7 +100,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('jest', ['test.js'], {
expect(execa).lastCalledWith('jest test.js', {
preferLocal: true,
reject: false,
shell: false
Expand All @@ -117,7 +117,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git', ['add', 'test.js'], {
expect(execa).lastCalledWith('git add test.js', {
cwd: process.cwd(),
preferLocal: true,
reject: false,
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Expand Up @@ -5463,11 +5463,6 @@ stealthy-require@^1.1.1:
resolved "https://registry.yarnpkg.com/stealthy-require/-/stealthy-require-1.1.1.tgz#35b09875b4ff49f26a777e509b3090a3226bf24b"
integrity sha1-NbCYdbT/SfJqd35QmzCQoyJr8ks=

string-argv@^0.3.0:
version "0.3.0"
resolved "https://registry.yarnpkg.com/string-argv/-/string-argv-0.3.0.tgz#0ea99e7257fea5e97a1bfcdfc19cf12d68e6ec6a"
integrity sha512-NGZHq3nkSXVtGZXTBjFru3MNfoZyIzN25T7BmvdgnSC0LCJczAGLLMQLyjywSIaAoqSemgLzBRHOsnrHbt60+Q==

string-length@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/string-length/-/string-length-2.0.0.tgz#d40dbb686a3ace960c1cffca562bf2c45f8363ed"
Expand Down

0 comments on commit effdb4c

Please sign in to comment.