Skip to content

Commit

Permalink
feat: replace listr with listr2 and print errors inline
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Apr 21, 2020
1 parent c9adca5 commit 8f32a3e
Show file tree
Hide file tree
Showing 15 changed files with 353 additions and 594 deletions.
2 changes: 0 additions & 2 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const dedent = require('dedent')
const { cosmiconfig } = require('cosmiconfig')
const stringifyObject = require('stringify-object')
const printErrors = require('./printErrors')
const runAll = require('./runAll')
const validateConfig = require('./validateConfig')

Expand Down Expand Up @@ -105,7 +104,6 @@ module.exports = async function lintStaged(
debugLog('tasks were executed successfully!')
return true
} catch (runAllError) {
printErrors(runAllError, logger)
return false
}
} catch (lintStagedError) {
Expand Down
15 changes: 0 additions & 15 deletions lib/printErrors.js

This file was deleted.

23 changes: 4 additions & 19 deletions lib/resolveTaskFn.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,6 @@ const debug = require('debug')('lint-staged:task')

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

/**
* Create and returns an error instance with a given message.
* If we set the message on the error instance, it gets logged multiple times(see #142).
* So we set the actual error message in a private field and extract it later,
* log only once.
*
* @param {string} message
* @returns {Error}
*/
function throwError(message) {
const err = new Error()
err.privateMsg = `\n\n\n${message}`
return err
}

/**
* Create a failure message dependding on process result.
*
Expand All @@ -41,12 +26,12 @@ function throwError(message) {
function makeErr(linter, result, context = {}) {
context.taskError = true
const { stdout, stderr, killed, signal } = result

if (killed || (signal && signal !== '')) {
return throwError(
`${symbols.warning} ${chalk.yellow(`${linter} was terminated with ${signal}`)}`
)
throw new Error(`${chalk.yellow(`${linter} was terminated with ${signal}`)}`)
}
return throwError(dedent`${symbols.error} ${chalk.redBright(

throw new Error(dedent`${chalk.redBright(
`${linter} found some errors. Please fix them and try committing again.`
)}
${stdout}
Expand Down
34 changes: 21 additions & 13 deletions lib/runAll.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/** @typedef {import('./index').Logger} Logger */

const chalk = require('chalk')
const Listr = require('listr')
const { Listr } = require('listr2')
const symbols = require('log-symbols')

const chunkFiles = require('./chunkFiles')
Expand Down Expand Up @@ -58,6 +58,13 @@ const shouldSkipCleanup = (ctx) => {
}
}

const wasFailed = (context) => {
const itemCount = Object.keys(context).length
if (itemCount === 0) return false
if ('hasPartiallyStagedFiles' in context && itemCount === 1) return false
return true
}

/**
* Executes all tasks and either resolves or rejects the promise
*
Expand Down Expand Up @@ -161,8 +168,9 @@ const runAll = async (
new Listr(subTasks, {
// In sub-tasks we don't want to run concurrently
// and we want to abort on errors
dateFormat: false,
collapse: false,
concurrent: false,
dateFormat: false,
exitOnError: true
}),
skip: () => {
Expand All @@ -179,7 +187,7 @@ const runAll = async (
// No need to show number of task chunks when there's only one
title:
chunkCount > 1 ? `Running tasks (chunk ${index + 1}/${chunkCount})...` : 'Running tasks...',
task: () => new Listr(chunkListrTasks, { ...listrOptions, concurrent }),
task: (ctx) => new Listr(chunkListrTasks, { ...listrOptions, ctx, concurrent }),
skip: (ctx = {}) => {
// Skip if the first step (backup) failed
if (ctx.gitError) return MESSAGES.GIT_ERROR
Expand Down Expand Up @@ -248,28 +256,28 @@ const runAll = async (
listrOptions
)

try {
await runner.run({})
} catch (error) {
if (error.context.gitApplyEmptyCommitError) {
const context = await runner.run()

if (wasFailed(context)) {
if (context.gitApplyEmptyCommitError) {
logger.warn(`
${symbols.warning} ${chalk.yellow(`lint-staged prevented an empty git commit.
Use the --allow-empty option to continue, or check your task configuration`)}
`)
} else if (error.context.gitError && !error.context.gitGetBackupStashError) {
Use the --allow-empty option to continue, or check your task configuration`)}
`)
} else if (context.gitError && !context.gitGetBackupStashError) {
logger.error(`\n ${symbols.error} ${chalk.red(`lint-staged failed due to a git error.`)}`)

if (shouldBackup) {
// No sense to show this if the backup stash itself is missing.
logger.error(` Any lost modifications can be restored from a git stash:
> git stash list
stash@{0}: On master: automatic lint-staged backup
stash@{0}: automatic lint-staged backup
> git stash apply --index stash@{0}\n`)
}
}

throw error
const error = new Error('lint-staged failed')
throw Object.assign(error, context)
}
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"debug": "^4.1.1",
"dedent": "^0.7.0",
"execa": "^4.0.0",
"listr": "^0.14.3",
"listr2": "1.3.7",
"log-symbols": "^3.0.0",
"micromatch": "^4.0.2",
"normalize-path": "^3.0.0",
Expand Down
33 changes: 8 additions & 25 deletions test/__snapshots__/index.spec.js.snap
Original file line number Diff line number Diff line change
@@ -1,43 +1,31 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`lintStaged should exit with code 1 on linter errors 1`] = `
"
ERROR
× node found some errors. Please fix them and try committing again."
`;
exports[`lintStaged should exit with code 1 on linter errors 1`] = `""`;

exports[`lintStaged should load an npm config package when specified 1`] = `
"
LOG Running lint-staged with the following config:
LOG {
'*': 'mytask'
}
ERROR Unable to get staged files!"
}"
`;

exports[`lintStaged should load config file when specified 1`] = `
"
LOG Running lint-staged with the following config:
LOG {
'*': 'mytask'
}
ERROR Unable to get staged files!"
}"
`;

exports[`lintStaged should not output config in normal mode 1`] = `
"
ERROR Unable to get staged files!"
`;
exports[`lintStaged should not output config in normal mode 1`] = `""`;

exports[`lintStaged should output config in debug mode 1`] = `
"
LOG Running lint-staged with the following config:
LOG {
'*': 'mytask'
}
ERROR Unable to get staged files!"
}"
`;

exports[`lintStaged should parse function linter from js config 1`] = `
Expand All @@ -46,8 +34,7 @@ LOG Running lint-staged with the following config:
LOG {
'*.css': filenames => \`echo \${filenames.join(' ')}\`,
'*.js': filenames => filenames.map(filename => \`echo \${filename}\`)
}
ERROR Unable to get staged files!"
}"
`;
exports[`lintStaged should print helpful error message when config file is not found 2`] = `
Expand Down Expand Up @@ -83,14 +70,10 @@ exports[`lintStaged should use config object 1`] = `
LOG Running lint-staged with the following config:
LOG {
'*': 'node -e \\"process.exit(1)\\"'
}
ERROR Unable to get staged files!"
}"
`;
exports[`lintStaged should use cosmiconfig if no params are passed 1`] = `
"
ERROR Unable to get staged files!"
`;
exports[`lintStaged should use cosmiconfig if no params are passed 1`] = `""`;
exports[`lintStaged should use use the console if no logger is passed 1`] = `
"
Expand Down
12 changes: 0 additions & 12 deletions test/__snapshots__/printErrors.spec.js.snap

This file was deleted.

10 changes: 10 additions & 0 deletions test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ describe('lintStaged', () => {
expect(logger.printHistory()).toMatchSnapshot()
})

it('should return true when passed', async () => {
expect.assertions(1)
const config = {
'*': 'node -e "process.exit(0)"'
}
getStagedFiles.mockImplementationOnce(async () => ['sample.java'])
const passed = await lintStaged({ config, quiet: true }, logger)
expect(passed).toEqual(true)
})

it('should use use the console if no logger is passed', async () => {
expect.assertions(1)
mockCosmiconfigWith({ config: {} })
Expand Down
4 changes: 2 additions & 2 deletions test/index2.spec.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import Listr from 'listr'
import { Listr } from 'listr2'
import makeConsoleMock from 'consolemock'
import path from 'path'

jest.mock('listr')
jest.mock('listr2')
jest.mock('../lib/resolveGitRepo')

// eslint-disable-next-line import/first
Expand Down
58 changes: 0 additions & 58 deletions test/printErrors.spec.js

This file was deleted.

23 changes: 2 additions & 21 deletions test/resolveTaskFn.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,7 @@ describe('resolveTaskFn', () => {
})

const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-fail-linter' })
try {
await taskFn()
} catch (err) {
expect(err.privateMsg).toMatchInlineSnapshot(`
"
× mock-fail-linter found some errors. Please fix them and try committing again.
Mock error"
`)
}
await expect(taskFn()).rejects.toThrow('mock-fail-linter found some errors')
})

it('should throw error for killed processes', async () => {
Expand All @@ -163,16 +153,7 @@ describe('resolveTaskFn', () => {
})

const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-killed-linter' })
try {
await taskFn()
} catch (err) {
expect(err.privateMsg).toMatchInlineSnapshot(`
"
‼ mock-killed-linter was terminated with SIGINT"
`)
}
await expect(taskFn()).rejects.toThrow('mock-killed-linter was terminated with SIGINT')
})

it('should not set taskError on context if no error occur', async () => {
Expand Down
Loading

0 comments on commit 8f32a3e

Please sign in to comment.