Skip to content

Commit

Permalink
fix: evaluate functional configuration only once
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Feb 21, 2020
1 parent f71c1c9 commit abe4b92
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 73 deletions.
37 changes: 20 additions & 17 deletions lib/makeCmdTasks.js
@@ -1,6 +1,7 @@
'use strict'

const resolveTaskFn = require('./resolveTaskFn')
const { createError } = require('./validateConfig')

const debug = require('debug')('lint-staged:make-cmd-tasks')

Expand All @@ -15,32 +16,34 @@ const debug = require('debug')('lint-staged:make-cmd-tasks')
*/
module.exports = async function makeCmdTasks({ commands, files, gitDir, shell }) {
debug('Creating listr tasks for commands %o', commands)
const commandsArray = Array.isArray(commands) ? commands : [commands]
const commandArray = Array.isArray(commands) ? commands : [commands]
const cmdTasks = []

for (const cmd of commandsArray) {
for (const cmd of commandArray) {
// command function may return array of commands that already include `stagedFiles`
const isFn = typeof cmd === 'function'
const resolved = isFn ? await cmd(files) : cmd

const resolvedArray = Array.isArray(resolved) ? resolved : [resolved] // Wrap non-array command as array

// Function command should not be used as the task title as-is
// because the resolved string it might be very long
// Create a matching command array with [file] in place of file names
let mockCmdTasks
if (isFn) {
const mockFileList = Array(files.length).fill('[file]')
const resolved = await cmd(mockFileList)
mockCmdTasks = Array.isArray(resolved) ? resolved : [resolved]
}

for (const [i, command] of resolvedArray.entries()) {
for (const command of resolvedArray) {
let title = isFn ? '[Function]' : command
if (isFn && mockCmdTasks[i]) {
// If command is a function, use the matching mock command as title,
// but since might include multiple [file] arguments, shorten to one
title = mockCmdTasks[i].replace(/\[file\].*\[file\]/, '[file]')

if (isFn) {
// If the function linter didn't return string | string[] it won't work
// Do the validation here instead of `validateConfig` to skip evaluating the function multiple times
if (typeof command !== 'string') {
throw new Error(
createError(
title,
'Function task should return a string or an array of strings',
resolved
)
)
}

const [startOfFn] = command.split(' ')
title += ` ${startOfFn} ...` // Append function name, like `[Function] eslint ...`
}

cmdTasks.push({
Expand Down
17 changes: 2 additions & 15 deletions lib/validateConfig.js
Expand Up @@ -80,21 +80,6 @@ module.exports = function validateConfig(config) {
)
)
}

entries.forEach(([, task]) => {
if (typeof task !== 'function') return
const resolved = task(['[filename]'])
if (typeof resolved === 'string') return
if (!Array.isArray(resolved) || resolved.some(subtask => typeof subtask !== 'string')) {
errors.push(
createError(
task,
'Function task should return a string or an array of strings',
resolved
)
)
}
})
})
}

Expand All @@ -104,3 +89,5 @@ module.exports = function validateConfig(config) {

return config
}

module.exports.createError = createError
12 changes: 0 additions & 12 deletions test/__snapshots__/validateConfig.spec.js.snap
Expand Up @@ -150,15 +150,3 @@ Please refer to https://github.com/okonet/lint-staged#configuration for more inf
`;

exports[`validateConfig should throw when detecting deprecated advanced configuration 2`] = `""`;

exports[`validateConfig should throw when function task returns incorrect values 1`] = `
"● Validation Error:
Invalid value for 'filenames => filenames.map(file => [\`eslint --fix \${file}\`, \`git add \${file}\`])'.
Function task should return a string or an array of strings.
Configured value is: [['eslint --fix [filename]', 'git add [filename]']]
Please refer to https://github.com/okonet/lint-staged#configuration for more information..."
`;
45 changes: 25 additions & 20 deletions test/makeCmdTasks.spec.js
Expand Up @@ -61,7 +61,7 @@ describe('makeCmdTasks', () => {
it('should work with function task returning a string', async () => {
const res = await makeCmdTasks({ commands: () => 'test', gitDir, files: ['test.js'] })
expect(res.length).toBe(1)
expect(res[0].title).toEqual('test')
expect(res[0].title).toEqual('[Function] test ...')
})

it('should work with function task returning array of string', async () => {
Expand All @@ -71,8 +71,8 @@ describe('makeCmdTasks', () => {
files: ['test.js']
})
expect(res.length).toBe(2)
expect(res[0].title).toEqual('test')
expect(res[1].title).toEqual('test2')
expect(res[0].title).toEqual('[Function] test ...')
expect(res[1].title).toEqual('[Function] test2 ...')
})

it('should work with function task accepting arguments', async () => {
Expand All @@ -82,8 +82,8 @@ describe('makeCmdTasks', () => {
files: ['test.js', 'test2.js']
})
expect(res.length).toBe(2)
expect(res[0].title).toEqual('test [file]')
expect(res[1].title).toEqual('test [file]')
expect(res[0].title).toEqual('[Function] test ...')
expect(res[1].title).toEqual('[Function] test ...')
})

it('should work with array of mixed string and function tasks', async () => {
Expand All @@ -93,26 +93,31 @@ describe('makeCmdTasks', () => {
files: ['test.js', 'test2.js', 'test3.js']
})
expect(res.length).toBe(5)
expect(res[0].title).toEqual('test')
expect(res[0].title).toEqual('[Function] test ...')
expect(res[1].title).toEqual('test2')
expect(res[2].title).toEqual('test [file]')
expect(res[3].title).toEqual('test [file]')
expect(res[4].title).toEqual('test [file]')
})

it('should generate short names for function tasks with long file list', async () => {
const res = await makeCmdTasks({
commands: filenames => `test ${filenames.map(file => `--file ${file}`).join(' ')}`,
gitDir,
files: Array(100).fill('file.js') // 100 times `file.js`
})
expect(res.length).toBe(1)
expect(res[0].title).toEqual('test --file [file]')
expect(res[2].title).toEqual('[Function] test ...')
expect(res[3].title).toEqual('[Function] test ...')
expect(res[4].title).toEqual('[Function] test ...')
})

it('should work with async function tasks', async () => {
const res = await makeCmdTasks({ commands: async () => 'test', gitDir, files: ['test.js'] })
expect(res.length).toBe(1)
expect(res[0].title).toEqual('test')
expect(res[0].title).toEqual('[Function] test ...')
})

it("should throw when function task doesn't return string | string[]", async () => {
await expect(makeCmdTasks({ commands: () => null, gitDir, files: ['test.js'] })).rejects
.toThrowErrorMatchingInlineSnapshot(`
"● Validation Error:
Invalid value for '[Function]'.
Function task should return a string or an array of strings.
Configured value is: null
Please refer to https://github.com/okonet/lint-staged#configuration for more information..."
`)
})
})
4 changes: 2 additions & 2 deletions test/runAll.unmocked.spec.js
Expand Up @@ -710,8 +710,8 @@ describe('runAll', () => {
LOG Preparing... [completed]
LOG Running tasks... [started]
LOG Running tasks for *.js [started]
LOG git stash drop [started]
LOG git stash drop [completed]
LOG [Function] git ... [started]
LOG [Function] git ... [completed]
LOG Running tasks for *.js [completed]
LOG Running tasks... [completed]
LOG Applying modifications... [started]
Expand Down
7 changes: 0 additions & 7 deletions test/validateConfig.spec.js
Expand Up @@ -49,13 +49,6 @@ describe('validateConfig', () => {
expect(console.printHistory()).toMatchSnapshot()
})

it('should throw when function task returns incorrect values', () => {
const invalidConfig = {
'*.js': filenames => filenames.map(file => [`eslint --fix ${file}`, `git add ${file}`])
}
expect(() => validateConfig(invalidConfig)).toThrowErrorMatchingSnapshot()
})

it('should throw when detecting deprecated advanced configuration', () => {
const advancedConfig = {
chunkSize: 10,
Expand Down

0 comments on commit abe4b92

Please sign in to comment.