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

feat/issue-1398: added support for custom configuration #1401

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thin-onions-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'lint-staged': minor
---

added support for custom configuration
40 changes: 40 additions & 0 deletions lib/handleCustomConfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import debug from 'debug'

const debugLog = debug('lint-staged:customTask')

import { configurationError } from './messages.js'
import { makeErr } from './resolveTaskFn.js'
import { getInitialState } from './state.js'

/**
* Handles function configuration and pushes the tasks into the task array
*
* @param {object} command
* @param {Array<string>} files
* @param {Array} cmdTasks
* @throws {Error} If the function configuration is not valid
*/
export const handleCustomConfig = (command, files, cmdTasks) => {
debugLog('Handling custom task %o', command)
if (typeof command.title === 'string' && typeof command.task === 'function') {
const task = async (ctx = getInitialState()) => {
try {
await command.task(files, ctx)
} catch (e) {
throw makeErr(command.title, e, ctx)
}
}
cmdTasks.push({
title: command.title,
task,
})
} else {
throw new Error(
configurationError(
'[Function]',
'Function task should contain `title` and `task` where `title` should be string and `task` should be function.',
command
)
)
}
}
53 changes: 29 additions & 24 deletions lib/makeCmdTasks.js
Copy link
Member

Choose a reason for hiding this comment

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

This file has started to become a bit complex... I wonder if we could split it somehow, maybe different handling for function and regular config.

Copy link
Author

Choose a reason for hiding this comment

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

done

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import debug from 'debug'

import { configurationError } from './messages.js'
import { resolveTaskFn } from './resolveTaskFn.js'
import { handleCustomConfig } from './handleCustomConfig.js'

const debugLog = debug('lint-staged:makeCmdTasks')

Expand All @@ -18,33 +19,37 @@ const debugLog = debug('lint-staged:makeCmdTasks')
*/
export const makeCmdTasks = async ({ commands, cwd, files, topLevelDir, shell, verbose }) => {
debugLog('Creating listr tasks for commands %o', commands)
const commandArray = Array.isArray(commands) ? commands : [commands]
const cmdTasks = []

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

/** Pass copy of file list to prevent mutation by function from config file. */
const resolved = isFn ? await cmd([...files]) : cmd

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

for (const command of resolvedArray) {
// 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 (isFn && typeof command !== 'string') {
throw new Error(
configurationError(
'[Function]',
'Function task should return a string or an array of strings',
resolved
if (!Array.isArray(commands) && typeof commands === 'object') {
handleCustomConfig(commands, files, cmdTasks)
} else {
const commandArray = Array.isArray(commands) ? commands : [commands]

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

/** Pass copy of file list to prevent mutation by function from config file. */
const resolved = isFn ? await cmd([...files]) : cmd

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

for (const command of resolvedArray) {
// 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 (isFn && typeof command !== 'string') {
throw new Error(
configurationError(
'[Function]',
'Function task should return a string or an array of strings',
resolved
)
)
)
}
}

const task = resolveTaskFn({ command, cwd, files, topLevelDir, isFn, shell, verbose })
cmdTasks.push({ title: command, command, task })
const task = resolveTaskFn({ command, cwd, files, gitDir, isFn, shell, verbose })
cmdTasks.push({ title: command, command, task })
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/resolveTaskFn.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const interruptExecutionOnError = (ctx, execaChildProcess) => {
* @param {Object} ctx
* @returns {Error}
*/
const makeErr = (command, result, ctx) => {
export const makeErr = (command, result, ctx) => {
ctx.errors.add(TaskError)

// https://nodejs.org/api/events.html#error-events
Expand Down
19 changes: 17 additions & 2 deletions lib/validateConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,27 @@ export const validateConfigLogic = (config, configPath, logger) => {
(!Array.isArray(task) ||
task.some((item) => typeof item !== 'string' && typeof item !== 'function')) &&
typeof task !== 'string' &&
typeof task !== 'function'
typeof task !== 'function' &&
typeof task !== 'object'
) {
errors.push(
configurationError(
pattern,
'Should be a string, a function, or an array of strings and functions.',
'Should be a string, a function, an object or an array of strings and functions.',
task
)
)
}

if (
!Array.isArray(task) &&
typeof task === 'object' &&
(typeof task.title !== 'string' || typeof task.task !== 'function')
) {
errors.push(
configurationError(
pattern,
'Custom task should contain `title` and `task` fields, where `title` should be a string and `task` should be a function.',
task
)
)
Expand Down
10 changes: 9 additions & 1 deletion test/unit/__snapshots__/validateConfig.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@ exports[`validateConfig should throw and should print validation errors for inva

Invalid value for 'foo': false

Should be a string, a function, or an array of strings and functions."
Should be a string, a function, an object or an array of strings and functions."
`;

exports[`validateConfig should throw and should print validation errors for invalid config 1 1`] = `"Configuration should be an object or a function"`;

exports[`validateConfig should throw and should print validation errors for invalid object config 1`] = `
"✖ Validation Error:

Invalid value for '*.js': { title: 'Running a custom task' }

Custom task should contain \`title\` and \`task\` fields, where \`title\` should be a string and \`task\` should be a function."
`;

exports[`validateConfig should throw for empty config 1`] = `"Configuration should not be empty"`;

exports[`validateConfig should throw when detecting deprecated advanced configuration 1`] = `
Expand Down
26 changes: 26 additions & 0 deletions test/unit/makeCmdTasks.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,30 @@ describe('makeCmdTasks', () => {
/** ...but the original file list was not mutated */
expect(files).toEqual(['test.js'])
})

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

it('should throw error when function task fails', async () => {
const failingTask = () => {
throw new Error('Task failed')
}

const res = await makeCmdTasks({
commands: { title: 'test', task: failingTask },
gitDir,
files: ['test.js'],
})

const [linter] = res
await expect(linter.task()).rejects.toThrow()
})
})
24 changes: 24 additions & 0 deletions test/unit/validateConfig.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,30 @@ describe('validateConfig', () => {
expect(() => validateConfig(invalidConfig, configPath, logger)).toThrowErrorMatchingSnapshot()
})

it('should throw and should print validation errors for invalid object config', () => {
const invalidConfig = {
'*.js': {
title: 'Running a custom task',
},
}

expect(() => validateConfig(invalidConfig, configPath, logger)).toThrowErrorMatchingSnapshot()
})

it('should not throw and should print nothing for valid object config', () => {
const validObjectConfig = {
'*.js': {
title: 'Running a custom task',
task: async (files) => {
console.log(files)
},
},
}

expect(() => validateConfig(validObjectConfig, configPath, logger)).not.toThrow()
expect(logger.printHistory()).toEqual('')
})

it('should throw for empty config', () => {
expect(() => validateConfig({}, configPath, logger)).toThrowErrorMatchingSnapshot()
})
Expand Down