Skip to content

Commit

Permalink
refactor: pass unparsed commands to execa with --shell
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj authored and okonet committed Jul 1, 2019
1 parent 275d996 commit 19536e3
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 24 deletions.
26 changes: 19 additions & 7 deletions src/resolveTaskFn.js
Expand Up @@ -17,9 +17,10 @@ const debug = require('debug')('lint-staged:task')
*/
const execLinter = (cmd, args, execaOptions = {}) => {
debug('cmd:', cmd)
debug('args:', args)
if (args) debug('args:', args)
debug('execaOptions:', execaOptions)
return execa(cmd, args, execaOptions)

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

This comment has been minimized.

Copy link
@sheerun

sheerun Aug 28, 2019

Contributor

I don't get it. Can execa accept cmd as function? Because cmd can be javascript function here (see current version of this file)

This comment has been minimized.

Copy link
@iiroj

iiroj Aug 29, 2019

Author Member

If the task was a javascript function, it's resolved by executing it against the staged file paths before being passed to execa. In this commit, you can see it here: https://github.com/okonet/lint-staged/blob/19536e3273b680199f9840388b1533e237b3843b/src/resolveTaskFn.js#L88

In the current master, it has been refactored and moved up the chain: https://github.com/okonet/lint-staged/blob/master/src/makeCmdTasks.js#L23

}

const successMsg = linter => `${symbols.success} ${linter} passed!`
Expand Down Expand Up @@ -88,20 +89,31 @@ module.exports = function resolveTaskFn({ gitDir, linter, pathsToLint, shell = f
// Support arrays of strings/functions by treating everything as arrays
const linters = Array.isArray(linterString) ? linterString : [linterString]

const execaOptions = { preferLocal: true, reject: false, shell }

const tasks = linters.map(command => {
const [cmd, ...args] = stringArgv.parseArgsStringToArgv(command)
// If `linter` is a function, args already include `pathsToLint`.
const argsWithPaths = fnLinter ? args : args.concat(pathsToLint)
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 `pathsToLint`.
cmd = fnLinter ? command : `${command} ${pathsToLint.join(' ')}`
} else {
const [parsedCmd, ...parsedArgs] = stringArgv.parseArgsStringToArgv(command)
cmd = parsedCmd
args = fnLinter ? parsedArgs : parsedArgs.concat(pathsToLint)
}

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

return ctx =>
execLinter(cmd, argsWithPaths, execaOptions).then(result => {
execLinter(cmd, args, execaOptions).then(result => {
if (result.failed || result.killed || result.signal != null) {
throw makeErr(linter, result, ctx)
}
Expand Down
17 changes: 0 additions & 17 deletions test/resolveTaskFn.spec.js
Expand Up @@ -167,21 +167,4 @@ Mock error"
expect(context.hasErrors).toEqual(true)
}
})

it('should call execa with shell when configured so', async () => {
expect.assertions(2)
const taskFn = resolveTaskFn({
...defaultOpts,
linter: 'node --arg=true ./myscript.js',
shell: true
})

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
preferLocal: true,
reject: false,
shell: true
})
})
})
21 changes: 21 additions & 0 deletions test/resolveTaskFn.unmocked.spec.js
@@ -0,0 +1,21 @@
import resolveTaskFn from '../src/resolveTaskFn'

jest.unmock('execa')

describe('resolveTaskFn', () => {
it('should call execa with shell when configured so', async () => {
const taskFn = resolveTaskFn({
pathsToLint: ['package.json'],
linter: () => 'node -e "process.exit(1)" || echo $?',
shell: true
})

await expect(taskFn()).resolves.toMatchInlineSnapshot(`
Array [
"√ function linter() {
return 'node -e \\"process.exit(1)\\" || echo $?';
} passed!",
]
`)
})
})

0 comments on commit 19536e3

Please sign in to comment.