Skip to content

Commit

Permalink
fix(parallel): remove --parallel in favor of concurrently (#94)
Browse files Browse the repository at this point in the history
**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.
  • Loading branch information
Kent C. Dodds committed Feb 18, 2017
1 parent 3566a15 commit acb7917
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 247 deletions.
45 changes: 12 additions & 33 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,20 @@ 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',
description: 'run in the amazingly intelligent Jest watch mode'
}
},
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"',
},
}
```
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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 <script-name1,script-name2> Scripts to run in parallel (comma seprated)
-c, --config <filepath> Config file to use (defaults to nearest package-scripts.yml or package-scripts.js)
-l, --log-level <level> The log level to use (error, warn, info [default])
-r, --require <module> Module to preload
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]):
Expand Down
25 changes: 12 additions & 13 deletions cli-test/__snapshots__/cli.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
exports[`test with --require 1`] = `
Object {
"stderr": "",
"stdout": "nps executing: echo \"log\"
"stdout": "[90mnps executing: [39m[32mecho \"log\"[39m
log
",
}
Expand All @@ -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 \"<projectRootDir>/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": "[31mUnable to find JS config at \"./something-that-does-not-exist.js\". Attempted to require as \"<projectRootDir>/cli-test/fixtures/something-that-does-not-exist.js\"[39m https://github.com/kentcdodds/p-s/blob/v0.0.0-semantically-released/other/ERRORS_AND_WARNINGS.md#unable-to-find-config
",
"stdout": "",
}
Expand All @@ -26,7 +26,7 @@ Object {
exports[`test with config with default script 1`] = `
Object {
"stderr": "",
"stdout": "nps executing: echo \"default script\"
"stdout": "[90mnps executing: [39m[32mecho \"default script\"[39m
default script
",
}
Expand All @@ -40,19 +40,18 @@ Object {
Options:
-h, --help output usage information
-V, --version output the version number
-s, --silent Silent nps output
-p, --parallel <script-name1,script-name2> Scripts to run in parallel (comma seprated)
-c, --config <filepath> Config file to use (defaults to nearest package-scripts.yml or package-scripts.js)
-l, --log-level <level> The log level to use (error, warn, info [default])
-r, --require <module> Module to preload
-h, --help output usage information
-V, --version output the version number
-s, --silent Silent nps output
-c, --config <filepath> Config file to use (defaults to nearest package-scripts.yml or package-scripts.js)
-l, --log-level <level> The log level to use (error, warn, info [default])
-r, --require <module> 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\"
[32mtest[39m - [90mecho \"test script\"[39m
[32mlint[39m - [90mecho \"lint.default\"[39m
[32mlint.sub.thing[39m - [37mthis is a description[39m - [90mecho \"deeply nested thing\"[39m
",
}
`;
45 changes: 43 additions & 2 deletions other/EXAMPLES.md
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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:

Expand All @@ -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
Expand Down
9 changes: 8 additions & 1 deletion package-scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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}`
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
28 changes: 12 additions & 16 deletions src/bin-utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -20,15 +20,15 @@ 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
})

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
Expand Down Expand Up @@ -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.',
),
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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]
}
Expand All @@ -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')
}
}

Expand Down
Loading

0 comments on commit acb7917

Please sign in to comment.