From acb7917efdcb1222c578686673ea965f34f6b9f0 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Thu, 9 Feb 2017 11:37:12 -0700 Subject: [PATCH] fix(parallel): remove `--parallel` in favor of `concurrently` (#94) **What**: This removes the `--parallel` capabities from `p-s` in favor of encouraging the use of the `concurrently` module. **Why**: DOTADIW. We want `p-s` to stay as small and focused as possible **How**: Refactoring and documenting BREAKING CHANGE: you must now use `concurrently` to have scripts run in parallel. Check the docs for an example of how to do this. --- README.md | 45 +++------- cli-test/__snapshots__/cli.test.js.snap | 25 +++--- other/EXAMPLES.md | 45 +++++++++- package-scripts.js | 9 +- package.json | 3 +- src/bin-utils/index.js | 28 +++---- src/bin-utils/index.test.js | 36 ++++---- src/bin/nps.js | 16 ++-- src/index.js | 67 +++------------ src/index.test.js | 105 ++++-------------------- yarn.lock | 75 ++++++++++++++++- 11 files changed, 207 insertions(+), 247 deletions(-) diff --git a/README.md b/README.md index 7306ad8..489d3db 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ module.exports = { default: 'node index.js', lint: 'eslint .', test: { - // learn more about Jest here: https://kcd.im/egghead-jest + // learn more about Jest here: https://facebook.github.io/jest default: 'jest', watch: { script: 'jest --watch', @@ -52,10 +52,12 @@ module.exports = { } }, build: { + // learn more about Webpack here: https://webpack.js.org/ default: 'webpack', prod: 'webpack -p', }, - validate: 'nps --parallel lint,test,build', + // learn more about concurrently here: https://npm.im/concurrently + validate: 'concurrently "nps lint" "nps test" "nps build"', }, } ``` @@ -75,7 +77,7 @@ scripts: build: default: webpack prod: webpack -p - validate: nps --parallel lint,test,build + validate: concurrently "nps lint" "nps test" "nps build" ``` To use `p-s`, it's recommended that you either install it globally (`npm i -g p-s`) or add `./node_modules/bin` to your @@ -84,7 +86,7 @@ To use `p-s`, it's recommended that you either install it globally (`npm i -g p- Then you can run: ```console -p-s --help +nps --help ``` Which will output: @@ -97,7 +99,6 @@ Which will output: -h, --help output usage information -V, --version output the version number -s, --silent Silent nps output - -p, --parallel Scripts to run in parallel (comma seprated) -c, --config Config file to use (defaults to nearest package-scripts.yml or package-scripts.js) -l, --log-level The log level to use (error, warn, info [default]) -r, --require Module to preload @@ -109,7 +110,7 @@ test - jest test.watch - run in the amazingly intelligent Jest watch mode - jest --watch build - webpack build.prod - webpack -p -validate - nps --parallel lint,test,build +validate - concurrently "nps lint" "nps test" "nps build" ``` **Because `p-s` is harder to type, it is recommended that you use the alias `nps` to interact with `p-s`, which is much @@ -243,14 +244,6 @@ config). By default, `nps` will log out to the console before running the command. You can add `-s` to your command to silence this. -##### -p, --parallel - -Run the given scripts in parallel. This enables handy workflows like this: - -```console -nps -p lint,build,cover && nps check-coverage && nps report-coverage -``` - ##### -c, --config Use a different config @@ -283,7 +276,7 @@ nps lint --fix # --fix will be passed on to the lint script ##### scripts -If you don't use `-p` (because you don't need parallelism) then you can simply provide the name of the script like so: +To run a script, you simply provide the name of the script like so: ```console nps cover @@ -377,6 +370,10 @@ Log levels available: ## FAQ +### How do I do ___ ? + +Have you looked at the examples in [other/EXAMPLES.md][examples]? + ### Why `npm start`? _Just to be clear:_ You do **not** have to use the `start` script. You can use whatever you like. But I recommend using @@ -404,24 +401,6 @@ This was inspired by [a tweet][tweet] by [@sindresorhus][sindre]. a line for every script (one of the pains I'm trying to solve) and a each script requires its own file (one of the benefits of npm scripts I wanted to keep). -## In the wild - -- [react-component-template](https://github.com/nkbt/react-component-template) uses `p-s` to implement shareable npm -scripts. See then how dependent [react-swap](https://github.com/nkbt/react-swap) can reuse them. - -GOTCHAS: - - use `process.cwd()` as the base for all paths - -- [Hypercubed/EventsSpeedTests](https://github.com/Hypercubed/EventsSpeedTests) uses `p-s` to automate benchmark running -and reporting in node and the browser. `package-scripts.js` enables us to keep our scripts DRY. Combined with -[grunion](https://github.com/Hypercubed/grunion) allows benchmarks to be run, serially or concurrently, on glob -patterns. - -- [SmithersAssistant/Smithers](https://github.com/SmithersAssistant/smithers) is an [electron](https://electron.atom.io) -based personal assistant. Smithers works on multiple platforms. Smithers uses `p-s` to dynamically find the current -platform and execute the dev environment. Now we don't have to manually update the `package.json` scripts when you are -on a different platform! - ## Contributors Thanks goes to these people ([emoji key][emojis]): diff --git a/cli-test/__snapshots__/cli.test.js.snap b/cli-test/__snapshots__/cli.test.js.snap index 8146090..50a6fa8 100644 --- a/cli-test/__snapshots__/cli.test.js.snap +++ b/cli-test/__snapshots__/cli.test.js.snap @@ -1,7 +1,7 @@ exports[`test with --require 1`] = ` Object { "stderr": "", - "stdout": "nps executing: echo \"log\" + "stdout": "nps executing: echo \"log\" log ", } @@ -17,7 +17,7 @@ Object { exports[`test with a missing config 1`] = ` Object { - "stderr": "Unable to find JS config at \"./something-that-does-not-exist.js\". Attempted to require as \"/cli-test/fixtures/something-that-does-not-exist.js\" https://github.com/kentcdodds/p-s/blob/v0.0.0-semantically-released/other/ERRORS_AND_WARNINGS.md#unable-to-find-config + "stderr": "Unable to find JS config at \"./something-that-does-not-exist.js\". Attempted to require as \"/cli-test/fixtures/something-that-does-not-exist.js\" https://github.com/kentcdodds/p-s/blob/v0.0.0-semantically-released/other/ERRORS_AND_WARNINGS.md#unable-to-find-config ", "stdout": "", } @@ -26,7 +26,7 @@ Object { exports[`test with config with default script 1`] = ` Object { "stderr": "", - "stdout": "nps executing: echo \"default script\" + "stdout": "nps executing: echo \"default script\" default script ", } @@ -40,19 +40,18 @@ Object { Options: - -h, --help output usage information - -V, --version output the version number - -s, --silent Silent nps output - -p, --parallel Scripts to run in parallel (comma seprated) - -c, --config Config file to use (defaults to nearest package-scripts.yml or package-scripts.js) - -l, --log-level The log level to use (error, warn, info [default]) - -r, --require Module to preload + -h, --help output usage information + -V, --version output the version number + -s, --silent Silent nps output + -c, --config Config file to use (defaults to nearest package-scripts.yml or package-scripts.js) + -l, --log-level The log level to use (error, warn, info [default]) + -r, --require Module to preload Available scripts (camel or kebab case accepted) -test - echo \"test script\" -lint - echo \"lint.default\" -lint.sub.thing - this is a description - echo \"deeply nested thing\" +test - echo \"test script\" +lint - echo \"lint.default\" +lint.sub.thing - this is a description - echo \"deeply nested thing\" ", } `; diff --git a/other/EXAMPLES.md b/other/EXAMPLES.md index b424079..5f61232 100644 --- a/other/EXAMPLES.md +++ b/other/EXAMPLES.md @@ -1,11 +1,23 @@ # package-scripts examples ## Links to projects + Examples of how people us `p-s`: - **[p-s](https://github.com/kentcdodds/p-s):** [scripts](https://github.com/kentcdodds/p-s/blob/7a00d750611f08e8a95f24e78dd1cdc0a2213e51/package.json#L6-L10), [`package-scripts.js`](https://github.com/kentcdodds/p-s/blob/7a00d750611f08e8a95f24e78dd1cdc0a2213e51/package-scripts.js) -- **[Smithers](https://github.com/SmithersAssistant/smithers):** - [scripts](https://github.com/SmithersAssistant/smithers/blob/0732fed616d64ff4696110574e51c300cd409d4c/package.json#L67-L70), [`package-scripts.js`](https://github.com/SmithersAssistant/smithers/blob/0732fed616d64ff4696110574e51c300cd409d4c/package-scripts.js) +- **[react-component-template](https://github.com/nkbt/react-component-template)** uses `p-s` to implement shareable npm +scripts. See then how dependent [react-swap](https://github.com/nkbt/react-swap) can reuse them. (Gotcha: - use +`process.cwd()` as the base for all paths). +- **[Hypercubed/EventsSpeedTests](https://github.com/Hypercubed/EventsSpeedTests)** uses `p-s` to automate benchmark +running and reporting in node and the browser. `package-scripts.js` enables us to keep our scripts DRY. Combined with +[grunion](https://github.com/Hypercubed/grunion) allows benchmarks to be run, serially or concurrently, on glob +patterns. +- **[SmithersAssistant/Smithers](https://github.com/SmithersAssistant/smithers)** is an +[electron](https://electron.atom.io) based personal assistant. Smithers works on multiple platforms. Smithers uses `p-s` +to dynamically find the current platform and execute the dev environment. Now we don't have to manually update the +`package.json` scripts when you are on a different platform! +[scripts](https://github.com/SmithersAssistant/smithers/blob/0732fed616d64ff4696110574e51c300cd409d4c/package.json#L67-L70), +[`package-scripts.js`](https://github.com/SmithersAssistant/smithers/blob/0732fed616d64ff4696110574e51c300cd409d4c/package-scripts.js) ## Inline Examples @@ -16,6 +28,9 @@ can't determine the platform in the `package.json`, you have to either have to d `build:windows` and `build:unix` script), find CLIs that are cross platform (like [`cross-env`](http://npm.im/cross-env)), or write your logic in a separate file to handle the platform. +You can also use [`cross-var`](http://npm.im/cross-var) in basically the same way to do the same for using environment +variables in your scripts so it works on both windows and mac/linux + With `package-scripts`, you can really easily have a single script that uses the platform to determine what should be run. For example: @@ -35,6 +50,32 @@ module.exports = { Note, in this specific scenario, I'd recommend that you actually use [`rimraf`](http://npm.im/rimraf), but I think you get the idea 😄. This is a pretty nice win over traditional npm scripts 👍 +### parallel scripts + +Often, scripts can run concurrently because they are not interdependent. We recommend +[`concurrently`](http://npm.im/concurrently) for this: + +```javascript +module.exports = { + scripts: { + validate: concurrent([ + 'build', + 'lint', + 'test', + 'order.sandwich', + ]), + // etc... + } +} + +function concurrent(scripts) { + process.env.FORCE_COLOR = true // this is necessary until https://github.com/kimmobrunfeldt/concurrently/issues/86 + const names = scripts.join(',') + const quotedScripts = `"nps ${scripts.join('" "nps ')}"` + return `concurrently --prefix "[{name}]" --names "${names}" ${quotedScripts}` +} +``` + ## Instructions Thanks for using `p-s`! I'm glad/I hope it's been helpful to you. Please add a link to your example here. If you're diff --git a/package-scripts.js b/package-scripts.js index a254a32..c02a7f3 100644 --- a/package-scripts.js +++ b/package-scripts.js @@ -43,7 +43,7 @@ module.exports = { }, validate: { description: 'This runs several scripts to make sure things look good before committing or on clean install', - script: `nps -p ${validate.join(',')}`, + script: concurrent(validate), }, addContributor: { description: 'When new people contribute to the project, run this', @@ -58,3 +58,10 @@ module.exports = { silent: false, }, } + +function concurrent(scripts) { + process.env.FORCE_COLOR = true // this is required until https://github.com/kimmobrunfeldt/concurrently/issues/86 + const names = scripts.join(',') + const quotedScripts = `"nps ${scripts.join('" "nps ')}"` + return `concurrently --prefix "[{name}]" --names "${names}" ${quotedScripts}` +} diff --git a/package.json b/package.json index 6613722..f709ee1 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "dependencies": { "arrify": "^1.0.1", "bluebird": "^3.4.7", - "colors": "^1.1.2", + "chalk": "^1.1.3", "commander": "^2.9.0", "find-up": "^2.1.0", "js-yaml": "^3.7.0", @@ -56,6 +56,7 @@ "husky": "^0.13.1", "jest-cli": "^18.1.0", "opt-cli": "^1.5.1", + "concurrently": "^3.1.0", "p-s": "*", "rimraf": "^2.5.4", "semantic-release": "^6.3.6", diff --git a/src/bin-utils/index.js b/src/bin-utils/index.js index 72a1301..b97df1d 100644 --- a/src/bin-utils/index.js +++ b/src/bin-utils/index.js @@ -3,7 +3,7 @@ import {readFileSync} from 'fs' import {remove, includes, isPlainObject, isUndefined, isEmpty, isFunction} from 'lodash' import typeOf from 'type-detect' import shellEscape from 'shell-escape' -import colors from 'colors/safe' +import chalk from 'chalk' import {safeLoad} from 'js-yaml' import getLogger from '../get-logger' @@ -20,7 +20,7 @@ const log = getLogger() */ const preloadModule = getAttemptModuleRequireFn((moduleName, requirePath) => { log.warn({ - message: colors.yellow(`Unable to preload "${moduleName}". Attempted to require as "${requirePath}"`), + message: chalk.yellow(`Unable to preload "${moduleName}". Attempted to require as "${requirePath}"`), ref: 'unable-to-preload-module', }) return undefined @@ -28,7 +28,7 @@ const preloadModule = getAttemptModuleRequireFn((moduleName, requirePath) => { const loadJSConfig = getAttemptModuleRequireFn(function onFail(configPath, requirePath) { log.error({ - message: colors.red(`Unable to find JS config at "${configPath}". Attempted to require as "${requirePath}"`), + message: chalk.red(`Unable to find JS config at "${configPath}". Attempted to require as "${requirePath}"`), ref: 'unable-to-find-config', }) return undefined @@ -59,7 +59,7 @@ function loadConfig(configPath, input) { } if (!isPlainObject(config)) { log.error({ - message: colors.red( + message: chalk.red( `The package-scripts configuration ("${configPath}") ` + 'must be an object or a function that returns an object.', ), @@ -86,7 +86,7 @@ function loadYAMLConfig(configPath) { throw e } log.error({ - message: colors.red(`Unable to find YML config at "${configPath}".`), + message: chalk.red(`Unable to find YML config at "${configPath}".`), ref: 'unable-to-find-config', }) return undefined @@ -97,22 +97,18 @@ function getScriptsAndArgs(program) { let scripts = [] let args = '' const {rawArgs} = program - const parallel = !isEmpty(program.parallel) - if (parallel) { - scripts = program.parallel.split(',') - args = getArgs(program.args, program.rawArgs, scripts) - } else if (!isEmpty(program.args)) { + if (!isEmpty(program.args)) { const [scriptsString] = program.args scripts = scriptsString.split(',') remove(rawArgs, arg => arg === scriptsString) args = getArgs(program.args.slice(1), rawArgs, scripts) } - return {scripts, args, parallel} + return {scripts, args} } function getArgs(args, rawArgs, scripts) { const allArgs = rawArgs.slice(2) - const psArgs = ['-p', '--parallel', '-c', '--config', '-r', '--require'] + const psArgs = ['-c', '--config', '-r', '--require'] const psFlags = ['-s', '--silent'] const cleanedArgs = remove(allArgs, (item, index, array) => { const isArgOrFlag = includes(psArgs, item) || includes(psFlags, item) @@ -165,11 +161,11 @@ function help({scripts}) { const availableScripts = getAvailableScripts(scripts) const filteredScripts = availableScripts.filter(script => !script.hiddenFromHelp) const scriptLines = filteredScripts.map(({name, description, script}) => { - const coloredName = colors.green(name) - const coloredScript = colors.gray(script) + const coloredName = chalk.green(name) + const coloredScript = chalk.gray(script) let line if (description) { - line = [coloredName, colors.white(description), coloredScript] + line = [coloredName, chalk.white(description), coloredScript] } else { line = [coloredName, coloredScript] } @@ -180,7 +176,7 @@ function help({scripts}) { const message = `${topMessage}\n\n${scriptLines.join('\n')}` return message } else { - return colors.yellow('There are no scripts available') + return chalk.yellow('There are no scripts available') } } diff --git a/src/bin-utils/index.test.js b/src/bin-utils/index.test.js index eef8af4..223f159 100644 --- a/src/bin-utils/index.test.js +++ b/src/bin-utils/index.test.js @@ -1,6 +1,6 @@ /* eslint import/newline-after-import:0, global-require:0 */ import path from 'path' -import colors from 'colors/safe' +import chalk from 'chalk' import {spy} from 'sinon' import {getScriptsAndArgs, help, preloadModule, loadConfig} from './index' @@ -21,14 +21,6 @@ test('getScriptsAndArgs: gets scripts in series', () => { expect(args).toEqual('') }) -test('getScriptsAndArgs: gets parallel scripts', () => { - const {scripts} = getScriptsAndArgs({ - parallel: 'boo,baz', - rawArgs: ['node', 'p-s', '-p', 'boo,baz'], - }) - expect(scripts).toEqual(['boo', 'baz']) -}) - test('getScriptsAndArgs: passes args to scripts', () => { const {args, scripts} = getScriptsAndArgs({ args: ['boo'], @@ -38,7 +30,7 @@ test('getScriptsAndArgs: passes args to scripts', () => { expect(args).toBe('--watch --verbose') }) -test('getScriptsAndArgs: returns empty scripts and args if not parallel and no args', () => { +test('getScriptsAndArgs: returns empty scripts and args if no args', () => { const {args, scripts} = getScriptsAndArgs({ args: [], rawArgs: ['node', 'p-s'], @@ -62,8 +54,8 @@ test('preloadModule: resolves an absolute path', () => { }) test('preloadModule: resolves a node_module', () => { - const val = preloadModule('colors/safe') - expect(val).toBe(colors) + const val = preloadModule('chalk') + expect(val).toBe(chalk) }) test('preloadModule: logs a warning when the module cannot be required', () => { @@ -221,14 +213,14 @@ test('help: formats a nice message', () => { const expected = ` Available scripts (camel or kebab case accepted) -${colors.green('foo')} - ${colors.white('the foo script')} - ${colors.gray('echo "foo"')} -${colors.green('bar')} - ${colors.white('stuff')} - ${colors.gray('echo "bar default"')} -${colors.green('bar.baz')} - ${colors.gray('echo "baz"')} -${colors.green('bar.barBub')} - ${colors.gray('echo "barBub"')} -${colors.green('build')} - ${colors.gray('webpack')} -${colors.green('build.x')} - ${colors.white('webpack with x env')} - ${colors.gray('webpack --env.x')} -${colors.green('build.x.y')} - ${colors.white('build X-Y')} - ${colors.gray('echo "build x-y"')} -${colors.green('foobar')} - ${colors.gray('echo "foobar"')} +${chalk.green('foo')} - ${chalk.white('the foo script')} - ${chalk.gray('echo "foo"')} +${chalk.green('bar')} - ${chalk.white('stuff')} - ${chalk.gray('echo "bar default"')} +${chalk.green('bar.baz')} - ${chalk.gray('echo "baz"')} +${chalk.green('bar.barBub')} - ${chalk.gray('echo "barBub"')} +${chalk.green('build')} - ${chalk.gray('webpack')} +${chalk.green('build.x')} - ${chalk.white('webpack with x env')} - ${chalk.gray('webpack --env.x')} +${chalk.green('build.x.y')} - ${chalk.white('build X-Y')} - ${chalk.gray('echo "build x-y"')} +${chalk.green('foobar')} - ${chalk.gray('echo "foobar"')} `.trim() expect(message).toBe(expected) @@ -237,7 +229,7 @@ ${colors.green('foobar')} - ${colors.gray('echo "foobar"')} test('help: returns no scripts available', () => { const config = {scripts: {}} const message = help(config) - const expected = colors.yellow('There are no scripts available') + const expected = chalk.yellow('There are no scripts available') expect(message).toBe(expected) }) @@ -252,7 +244,7 @@ test('help: do not display scripts with flag hiddenFromHelp set to true', () => }, } const message = help(config) - const expected = colors.yellow('There are no scripts available') + const expected = chalk.yellow('There are no scripts available') expect(message).toBe(expected) }) diff --git a/src/bin/nps.js b/src/bin/nps.js index 4cd1680..9976da2 100644 --- a/src/bin/nps.js +++ b/src/bin/nps.js @@ -3,7 +3,7 @@ import findUp from 'find-up' import {merge, includes, indexOf} from 'lodash' import program from 'commander' -import colors from 'colors/safe' +import chalk from 'chalk' import {keyInYN} from 'readline-sync' import runPackageScript from '../index' import { @@ -23,7 +23,6 @@ program .version(version) .allowUnknownOption() .option('-s, --silent', 'Silent nps output') - .option('-p, --parallel ', 'Scripts to run in parallel (comma seprated)') .option('-c, --config ', 'Config file to use (defaults to nearest package-scripts.yml or package-scripts.js)') .option('-l, --log-level ', 'The log level to use (error, warn, info [default])') .option('-r, --require ', 'Module to preload') @@ -58,7 +57,6 @@ function loadAndRun(psConfig) { args: scriptsAndArgs.args, options: merge(psConfig.options, { silent: program.silent, - parallel: scriptsAndArgs.parallel, logLevel: program.logLevel, }), }).catch(error => { @@ -73,7 +71,7 @@ function getPSConfig() { } const configFilepath = getPSConfigFilepath() if (!configFilepath) { - log.warn(colors.yellow('Unable to find a config file and none was specified.')) + log.warn(chalk.yellow('Unable to find a config file and none was specified.')) return {scripts: {}} // empty config } const config = loadConfig(configFilepath, scriptsAndArgs) @@ -89,18 +87,18 @@ function getPSConfigFilepath() { function onInit() { if (getPSConfigFilepath()) { - if (!keyInYN(colors.yellow(`Do you want to overwrite your existing config file?`))) { + if (!keyInYN(chalk.yellow(`Do you want to overwrite your existing config file?`))) { process.exit(FAIL_CODE) } } shouldRun = false const {packageScriptsPath} = initialize(getConfigType()) - log.info(`Your scripts have been saved at ${colors.green(packageScriptsPath)}`) - log.info(colors.gray( + log.info(`Your scripts have been saved at ${chalk.green(packageScriptsPath)}`) + log.info(chalk.gray( 'Check out your scripts in there. Go ahead and update them and add descriptions to the ones that need it', )) - log.info(colors.gray('Your package.json scripts have also been updated. Run `npm start help` for help')) - log.info(colors.gray( + log.info(chalk.gray('Your package.json scripts have also been updated. Run `npm start help` for help')) + log.info(chalk.gray( 'You may also want to install the package globally and installing autocomplete script. You can do so by running\n' + ' npm install --global p-s\n' + ' nps completion \n' + diff --git a/src/index.js b/src/index.js index db9bce3..ae2a0d3 100644 --- a/src/index.js +++ b/src/index.js @@ -1,6 +1,6 @@ import spawn from 'spawn-command-with-kill' import Promise from 'bluebird' -import colors from 'colors/safe' +import chalk from 'chalk' import {isString, clone} from 'lodash' import {sync as findUpSync} from 'find-up' import managePath from 'manage-path' @@ -18,50 +18,12 @@ function runPackageScripts({scriptConfig, scripts, args, options = {}}) { scripts = ['default'] } const scriptNames = arrify(scripts) - if (options.parallel) { - return runParallel() - } else { - return runSeries() - } - - function runSeries() { - return scriptNames.reduce((res, scriptName) => { - return res.then(() => ( - runPackageScript({scriptConfig, options, scriptName, args}) - )) - }, Promise.resolve()) - } - - function runParallel() { - const results = scriptNames.map(script => ({script, code: undefined})) - let aborted = false - - const promises = scriptNames.map(scriptName => { - return runPackageScript({scriptConfig, options, scriptName, args}) - }) - - const allPromise = Promise.all(promises.map((promise, index) => { - return promise.then(code => { - results[index].code = code - }) - })).then(() => results) - allPromise.catch(() => { - /* istanbul ignore if */ - if (aborted) { - // this is very unlikely to happen - } else { - abortAll() - } - }) - - return allPromise - - function abortAll() { - aborted = true - promises.forEach(p => p.abort()) - } - } + return scriptNames.reduce((res, scriptName) => { + return res.then(() => ( + runPackageScript({scriptConfig, options, scriptName, args}) + )) + }, Promise.resolve()) } @@ -70,7 +32,7 @@ function runPackageScript({scriptConfig, options, scriptName, args}) { const script = getScriptToRun(scripts, scriptName) if (!isString(script)) { return Promise.reject({ - message: colors.red( + message: chalk.red( `Scripts must resolve to strings. There is no script that can be resolved from "${scriptName}"`, ), ref: 'missing-script', @@ -78,15 +40,14 @@ function runPackageScript({scriptConfig, options, scriptName, args}) { } const command = [script, args].join(' ').trim() const log = getLogger(getLogLevel(options)) - log.info(colors.gray('nps executing: ') + colors.green(command)) + log.info(chalk.gray('nps executing: ') + chalk.green(command)) let child const promise = new Promise((resolve, reject) => { child = spawn(command, {stdio: 'inherit', env: getEnv()}) child.on('error', error => { - child = null reject({ - message: colors.red( + message: chalk.red( `The script called "${scriptName}" which runs "${command}" emitted an error`, ), ref: 'emitted-an-error', @@ -95,12 +56,11 @@ function runPackageScript({scriptConfig, options, scriptName, args}) { }) child.on('close', code => { - child = null if (code === NON_ERROR) { resolve(code) } else { reject({ - message: colors.red( + message: chalk.red( `The script called "${scriptName}" which runs "${command}" failed with exit code ${code}`, ), ref: 'failed-with-exit-code', @@ -110,13 +70,6 @@ function runPackageScript({scriptConfig, options, scriptName, args}) { }) }) - promise.abort = function abort() { - if (child !== null) { - child.kill() - child = null - } - } - return promise } diff --git a/src/index.test.js b/src/index.test.js index a3cbf6c..a57cdf1 100644 --- a/src/index.test.js +++ b/src/index.test.js @@ -2,7 +2,7 @@ import {resolve} from 'path' import Promise from 'bluebird' import {spy} from 'sinon' -import color from 'colors/safe' +import chalk from 'chalk' import managePath from 'manage-path' test('spawn called with the parent process.env + npm path', () => { @@ -39,7 +39,7 @@ test('returns a log object when no script is found', () => { const scriptConfig = {lint: {script: 42}} return runPackageScript({scriptConfig, scripts: ['lint']}).catch(error => { expect(error).toEqual({ - message: color.red('Scripts must resolve to strings. There is no script that can be resolved from "lint"'), + message: chalk.red('Scripts must resolve to strings. There is no script that can be resolved from "lint"'), ref: 'missing-script', }) }) @@ -69,7 +69,7 @@ test('passes on additional arguments', () => { }) }) -test('runs scripts serially if given an array of input without parallel', () => { +test('runs scripts serially if given an array of input', () => { const lintCommand = 'eslint' const buildCommand = 'babel' const args = 'src/ scripts/' @@ -126,116 +126,43 @@ test('stops running scripts when running serially if any given script fails', () }) }) -test('runs scripts in parallel if given an array of input', () => { - const lintCommand = 'eslint' - const buildCommand = 'babel' - const args = 'src/ scripts/' - const {runPackageScript, mockSpawnStubSpy} = setup() - const scripts = ['lint', 'build'] - const scriptConfig = {build: buildCommand, lint: lintCommand} - const options = {parallel: true} - return runPackageScript({scriptConfig, scripts, args, options}).then(() => { - expect(mockSpawnStubSpy.calledTwice) - const [command1] = mockSpawnStubSpy.firstCall.args - const [command2] = mockSpawnStubSpy.secondCall.args - expect(command1).toBe('eslint src/ scripts/') - expect(command2).toBe('babel src/ scripts/') - }) -}) - test('runs the default script if no scripts provided', () => { return testSpawnCall({default: 'echo foo'}, []).then(({command}) => { expect(command).toBe(`echo foo`) }) }) -test('an non-zero exit code from a script will abort other scripts that are still running', () => { - const FAIL_CODE = 1 - const goodCommand = 'goodButLong' +test('an error from the child process logs an error', () => { + const ERROR = {message: 'there was an error', code: 2} const badCommand = 'bad' - const longCommand = 'long' - const scripts = ['goodS', 'badS', 'longS'] - const scriptConfig = { - goodS: goodCommand, - badS: badCommand, - longS: longCommand, - } - const killSpy = spy() - function spawnStub(cmd) { - return { - on(event, cb) { - if (event === 'close') { - if (cmd === longCommand) { - // verifies that the promise callback wont be invoked when it's been aborted - setTimeout(() => cb(0)) - } else if (cmd === badCommand) { - cb(FAIL_CODE) - } else { - cb(0) - } - } - }, - kill() { - killSpy(cmd) - }, - } - } - const {runPackageScript} = setup(spawnStub) - const options = {parallel: true} - return runPackageScript({scriptConfig, scripts, options}).then(() => { - throw new Error('the promise should be rejected') - }, ({code, ref, message}) => { - expect(code).toBe(FAIL_CODE) - expect(typeof ref === 'string') - expect(typeof message === 'string') - expect(killSpy.calledOnce) // and only once, just for the longCommand - expect(killSpy.firstCall.args[0]).toBe(longCommand) - }) -}) - -test('an error event from a script will abort other scripts', () => { - const ERROR_STRING = 'error string' const goodCommand = 'good' - const badCommand = 'bad' - const longCommand = 'long' - const scripts = ['goodS', 'badS', 'longS'] + const scripts = ['badS', 'goodS'] const scriptConfig = { goodS: goodCommand, badS: badCommand, - longS: longCommand, } - const killSpy = spy() function spawnStub(cmd) { return { on(event, cb) { - if (event === 'close') { - if (cmd === longCommand) { - // never call the callback - // it'll get aborted anyway. - } else if (cmd === badCommand) { - // do nothing in this case because we're going to call the error cb - } else { - cb(0) - } - } else if (event === 'error' && cmd === badCommand) { - cb(ERROR_STRING) + if (event === 'error' && cmd === badCommand) { + cb(ERROR) } }, kill() { - killSpy(cmd) }, } } - const {runPackageScript} = setup(spawnStub) - const options = {parallel: true} - return runPackageScript({scriptConfig, scripts, options}).then(() => { + const mockSpawnStubSpy = jest.fn(spawnStub) + const {runPackageScript} = setup(mockSpawnStubSpy) + return runPackageScript({scriptConfig, scripts}).then(() => { throw new Error('the promise should be rejected') - }, ({error, ref, message}) => { - expect(error).toBe(ERROR_STRING) + }, ({message, ref, error}) => { + expect(error).toBe(ERROR) expect(typeof ref === 'string') expect(typeof message === 'string') - expect(killSpy.calledOnce) // and only once, just for the longCommand - expect(killSpy.firstCall.args[0]).toBe(longCommand) + expect(mockSpawnStubSpy).toHaveBeenCalledTimes(1) + const [[command1]] = mockSpawnStubSpy.mock.calls + expect(command1).toBe(badCommand) }) }) diff --git a/yarn.lock b/yarn.lock index 68280a9..84fe92f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -107,10 +107,18 @@ ansi-escapes@^1.1.0, ansi-escapes@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/ansi-escapes/-/ansi-escapes-1.4.0.tgz#d3a8a83b319aa67793662b13e761c7911422306e" +ansi-regex@^0.2.0, ansi-regex@^0.2.1: + version "0.2.1" + resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-0.2.1.tgz#0d8e946967a3d8143f93e24e298525fc1b2235f9" + ansi-regex@^2.0.0: version "2.1.1" resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-2.1.1.tgz#c3b33ab5ee360d86e0e628f0468ae7ef27d654df" +ansi-styles@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-1.1.0.tgz#eaecbf66cd706882760b2f4691582b8f55d7a7de" + ansi-styles@^2.2.1: version "2.2.1" resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-2.2.1.tgz#b432dd3358b634cf75e1e4664368240533c1ddbe" @@ -924,6 +932,10 @@ block-stream@*: dependencies: inherits "~2.0.0" +bluebird@2.9.6: + version "2.9.6" + resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-2.9.6.tgz#1fc3a6b1685267dc121b5ec89b32ce069d81ab7d" + bluebird@3.4.7, bluebird@^3.4.1, bluebird@^3.4.6, bluebird@^3.4.7: version "3.4.7" resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.4.7.tgz#f72d760be09b7f76d08ed8fae98b289a8d05fab3" @@ -1107,6 +1119,16 @@ center-align@^0.1.1: align-text "^0.1.3" lazy-cache "^1.0.3" +chalk@0.5.1: + version "0.5.1" + resolved "https://registry.yarnpkg.com/chalk/-/chalk-0.5.1.tgz#663b3a648b68b55d04690d49167aa837858f2174" + dependencies: + ansi-styles "^1.1.0" + escape-string-regexp "^1.0.0" + has-ansi "^0.1.0" + strip-ansi "^0.3.0" + supports-color "^0.2.0" + chalk@1.1.3, chalk@^1.0.0, chalk@^1.1.0, chalk@^1.1.1, chalk@^1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/chalk/-/chalk-1.1.3.tgz#a8115c55e4a702fe4d150abd3872822a7e09fc98" @@ -1214,7 +1236,7 @@ colors@1.0.3: version "1.0.3" resolved "https://registry.yarnpkg.com/colors/-/colors-1.0.3.tgz#0433f44d809680fdeb60ed260f1b0c262e82a40b" -colors@1.1.2, colors@^1.1.2: +colors@1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/colors/-/colors-1.1.2.tgz#168a4701756b6a7f51a12ce0c97bfa28c084ed63" @@ -1228,6 +1250,10 @@ combined-stream@^1.0.5, combined-stream@~1.0.5: dependencies: delayed-stream "~1.0.0" +commander@2.6.0: + version "2.6.0" + resolved "https://registry.yarnpkg.com/commander/-/commander-2.6.0.tgz#9df7e52fb2a0cb0fb89058ee80c3104225f37e1d" + commander@2.9.0, commander@^2.8.1, commander@^2.9.0: version "2.9.0" resolved "https://registry.yarnpkg.com/commander/-/commander-2.9.0.tgz#9c99094176e12240cb22d6c5146098400fe0f7d4" @@ -1270,6 +1296,19 @@ concat-stream@^1.4.6, concat-stream@^1.4.7, concat-stream@^1.5.2: readable-stream "^2.2.2" typedarray "^0.0.6" +concurrently@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/concurrently/-/concurrently-3.1.0.tgz#dc5ef0459090012604756668894c04b434ef90d1" + dependencies: + bluebird "2.9.6" + chalk "0.5.1" + commander "2.6.0" + lodash "^4.5.1" + moment "^2.11.2" + rx "2.3.24" + spawn-default-shell "^1.1.0" + tree-kill "^1.1.0" + condition-node-version@^1.3.0: version "1.3.0" resolved "https://registry.yarnpkg.com/condition-node-version/-/condition-node-version-1.3.0.tgz#bd3284b204cf715fe21b948cde7e373a373cb717" @@ -1673,7 +1712,7 @@ es6-weak-map@^2.0.1: es6-iterator "2" es6-symbol "3" -escape-string-regexp@^1.0.2, escape-string-regexp@^1.0.5: +escape-string-regexp@^1.0.0, escape-string-regexp@^1.0.2, escape-string-regexp@^1.0.5: version "1.0.5" resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4" @@ -2334,6 +2373,12 @@ har-validator@~2.0.6: is-my-json-valid "^2.12.4" pinkie-promise "^2.0.0" +has-ansi@^0.1.0: + version "0.1.0" + resolved "https://registry.yarnpkg.com/has-ansi/-/has-ansi-0.1.0.tgz#84f265aae8c0e6a88a12d7022894b7568894c62e" + dependencies: + ansi-regex "^0.2.0" + has-ansi@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/has-ansi/-/has-ansi-2.0.0.tgz#34f5049ce1ecdf2b0649af3ef24e45ed35416d91" @@ -2643,7 +2688,7 @@ is-path-inside@^1.0.0: is-posix-bracket@^0.1.0: version "0.1.1" - resolved "http://registry.npmjs.org/is-posix-bracket/-/is-posix-bracket-0.1.1.tgz#3334dc79774368e92f016e6fbc0a88f5cd6e6bc4" + resolved "https://registry.yarnpkg.com/is-posix-bracket/-/is-posix-bracket-0.1.1.tgz#3334dc79774368e92f016e6fbc0a88f5cd6e6bc4" is-primitive@^2.0.0: version "2.0.0" @@ -3262,7 +3307,7 @@ lodash@4.17.2: version "4.17.2" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.2.tgz#34a3055babe04ce42467b607d700072c7ff6bf42" -lodash@4.17.4, lodash@^4.0.0, lodash@^4.11.2, lodash@^4.13.1, lodash@^4.14.0, lodash@^4.17.4, lodash@^4.2.0, lodash@^4.3.0: +lodash@4.17.4, lodash@^4.0.0, lodash@^4.11.2, lodash@^4.13.1, lodash@^4.14.0, lodash@^4.17.4, lodash@^4.2.0, lodash@^4.3.0, lodash@^4.5.1: version "4.17.4" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.4.tgz#78203a4d1c328ae1d86dca6460e369b57f4055ae" @@ -3427,6 +3472,10 @@ minimist@1.2.0, minimist@^1.1.1, minimist@^1.1.3, minimist@^1.2.0: dependencies: minimist "0.0.8" +moment@^2.11.2: + version "2.17.1" + resolved "https://registry.yarnpkg.com/moment/-/moment-2.17.1.tgz#fed9506063f36b10f066c8b59a144d7faebe1d82" + ms@0.7.1: version "0.7.1" resolved "https://registry.yarnpkg.com/ms/-/ms-0.7.1.tgz#9cd13c03adbff25b65effde7ce864ee952017098" @@ -4348,6 +4397,10 @@ rx-lite@^3.1.2: version "3.1.2" resolved "https://registry.yarnpkg.com/rx-lite/-/rx-lite-3.1.2.tgz#19ce502ca572665f3b647b10939f97fd1615f102" +rx@2.3.24: + version "2.3.24" + resolved "https://registry.yarnpkg.com/rx/-/rx-2.3.24.tgz#14f950a4217d7e35daa71bbcbe58eff68ea4b2b7" + rx@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/rx/-/rx-4.1.0.tgz#a5f13ff79ef3b740fe30aa803fb09f98805d4782" @@ -4524,6 +4577,10 @@ spawn-command@^0.0.2-1: version "0.0.2" resolved "https://registry.yarnpkg.com/spawn-command/-/spawn-command-0.0.2.tgz#9544e1a43ca045f8531aac1a48cb29bdae62338e" +spawn-default-shell@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/spawn-default-shell/-/spawn-default-shell-1.1.0.tgz#095439d44c4b7c0aff56a53929fbaab87878e7c6" + spawn-sync@^1.0.15: version "1.0.15" resolved "https://registry.yarnpkg.com/spawn-sync/-/spawn-sync-1.0.15.tgz#b00799557eb7fb0c8376c29d44e8a1ea67e57476" @@ -4632,6 +4689,12 @@ stringstream@~0.0.4: version "0.0.5" resolved "https://registry.yarnpkg.com/stringstream/-/stringstream-0.0.5.tgz#4e484cd4de5a0bbbee18e46307710a8a81621878" +strip-ansi@^0.3.0: + version "0.3.0" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-0.3.0.tgz#25f48ea22ca79187f3174a4db8759347bb126220" + dependencies: + ansi-regex "^0.2.1" + strip-ansi@^3.0.0, strip-ansi@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-3.0.1.tgz#6a385fb8853d952d5ff05d0e8aaf94278dc63dcf" @@ -4799,6 +4862,10 @@ travis-deploy-once@1.0.0-node-0.10-support: request-promise "^4.1.1" travis-ci "^2.1.1" +tree-kill@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/tree-kill/-/tree-kill-1.1.0.tgz#c963dcf03722892ec59cba569e940b71954d1729" + trim-newlines@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/trim-newlines/-/trim-newlines-1.0.0.tgz#5887966bb582a4503a41eb524f7d35011815a613"