Skip to content

Commit

Permalink
feat: Remove support for npm scripts (#390)
Browse files Browse the repository at this point in the history
- Update `findBin` to only resolve binaries installed locally or globally. Also
  add simple caching.
- Check pkg scripts when `findBin` fails and print useful message to aid users
  in migration to newer version config.
- Update readme.
- Remove dependency `app-root-path`.
- Update tests.

BREAKING CHANGE: Remove implicit support for running npm scripts.

Consider example `lint-staged` config:

    {
      "name": "My project",
      "version": "0.1.0",
      "scripts": {
        "my-custom-script": "linter --arg1 --arg2",
        "precommit": "lint-staged"
      },
      "lint-staged": {
        "*.js": ["my-custom-script", "git add"]
      }
    }

The list of commands should be changed to the following:

    "*.js": ["npm run my-custom-script --", "git add"]
  • Loading branch information
sudo-suhas committed Feb 21, 2018
1 parent 5a333a0 commit d8b836c
Show file tree
Hide file tree
Showing 17 changed files with 244 additions and 190 deletions.
37 changes: 26 additions & 11 deletions README.md
Expand Up @@ -6,7 +6,7 @@ Run linters against staged git files and don't let :poop: slip into your code ba

Linting makes more sense when running before committing your code. By doing that you can ensure no errors are going into repository and enforce code style. But running a lint process on a whole project is slow and linting results can be irrelevant. Ultimately you only want to lint files that will be committed.

This project contains a script that will run arbitrary npm and shell tasks with a list of staged files as an argument, filtered by a specified glob pattern.
This project contains a script that will run arbitrary shell tasks with a list of staged files as an argument, filtered by a specified glob pattern.

## Related blogs posts

Expand Down Expand Up @@ -96,11 +96,8 @@ Should be an object where each value is a command to run and its key is a glob p
#### `package.json` example:
```json
{
"scripts": {
"my-task": "your-command",
},
"lint-staged": {
"*": "my-task"
"*": "your-cmd"
}
}
```
Expand All @@ -109,15 +106,15 @@ Should be an object where each value is a command to run and its key is a glob p

```json
{
"*": "my-task"
"*": "your-cmd"
}
```

This config will execute `npm run my-task` with the list of currently staged files passed as arguments.
This config will execute `your-cmd` with the list of currently staged files passed as arguments.

So, considering you did `git add file1.ext file2.ext`, lint-staged will run the following command:

`npm run my-task -- file1.ext file2.ext`
`your-cmd file1.ext file2.ext`

### Advanced config format
To set options and keep lint-staged extensible, advanced format can be used. This should hold linters object in `linters` property.
Expand Down Expand Up @@ -164,11 +161,11 @@ Also see [How to use `lint-staged` in a multi package monorepo?](#how-to-use-lin

## What commands are supported?

Supported are both local npm scripts (`npm run-script`), or any executables installed locally or globally via `npm` as well as any executable from your $PATH.
Supported are any executables installed locally or globally via `npm` as well as any executable from your $PATH.

> Using globally installed scripts is discouraged, since lint-staged may not work for someone who doesn’t have it installed.
`lint-staged` is using [npm-which](https://github.com/timoxley/npm-which) to locate locally installed scripts, so you don't need to add `{ "eslint": "eslint" }` to the `scripts` section of your `package.json`. So in your `.lintstagedrc` you can write:
`lint-staged` is using [npm-which](https://github.com/timoxley/npm-which) to locate locally installed scripts. So in your `.lintstagedrc` you can write:

```json
{
Expand Down Expand Up @@ -210,13 +207,14 @@ All examples assuming you’ve already set up lint-staged and husky in the `pack
"name": "My project",
"version": "0.1.0",
"scripts": {
"my-custom-script": "linter --arg1 --arg2",
"precommit": "lint-staged"
},
"lint-staged": {}
}
```

*Note we don’t pass a path as an argument for the runners. This is important since lint-staged will do this for you. Please don’t reuse your tasks with paths from package.json.*
*Note we don’t pass a path as an argument for the runners. This is important since lint-staged will do this for you.*

### ESLint with default parameters for `*.js` and `*.jsx` running as a pre-commit hook

Expand All @@ -236,6 +234,23 @@ All examples assuming you’ve already set up lint-staged and husky in the `pack

This will run `eslint --fix` and automatically add changes to the commit. Please note, that it doesn’t work well with committing hunks (`git add -p`).

### Reuse npm script

If you wish to reuse a npm script defined in your package.json:

```json
{
"*.js": ["npm run my-custom-script --", "git add"]
}
```

The following is equivalent:

```json
{
"*.js": ["linter --arg1 --arg2", "git add"]
}
```

### Automatically fix code style with `prettier` for javascript + flow or typescript

Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -27,7 +27,7 @@
"test:watch": "jest --watch"
},
"dependencies": {
"app-root-path": "^2.0.0",
"app-root-path": "^2.0.1",
"chalk": "^2.1.0",
"commander": "^2.11.0",
"cosmiconfig": "^4.0.0",
Expand Down
45 changes: 45 additions & 0 deletions src/checkPkgScripts.js
@@ -0,0 +1,45 @@
'use strict'

const chalk = require('chalk')
const dedent = require('dedent')
const has = require('lodash/has')

const warn = msg => {
console.warn(chalk.yellowBright.bold(msg))
}

/**
* Checks if the given command or binary name is present in the package.json scripts. This would be
* called if and when resolving a binary fails in `findBin`.
*
* @param {Object} pkg package.json
* @param {string} cmd
* @param {string} binName
* @param {Array<string>} args
* @throws {Error} If a script is found in the pkg for the given `cmd` or `binName`.
*/
module.exports = function checkPkgScripts(pkg, cmd, binName, args) {
if (pkg && pkg.scripts) {
let scriptName
let script
if (has(pkg.scripts, cmd)) {
scriptName = cmd
script = pkg.scripts[cmd]
} else if (has(pkg.scripts, binName)) {
scriptName = binName
script = pkg.scripts[binName]
} else {
return
}

const argsStr = args && args.length ? args.join(' ') : ''
warn(dedent`
\`lint-staged\` no longer supports running scripts defined in package.json.
The same behavior can be achieved by changing the command to any of the following:
- \`npm run ${scriptName} -- ${argsStr}\`
- \`${script} ${argsStr}\`
`)
throw new Error(`Could not resolve binary for \`${cmd}\``)
}
}
87 changes: 24 additions & 63 deletions src/findBin.js
@@ -1,86 +1,47 @@
'use strict'

const appRoot = require('app-root-path')
const npmWhich = require('npm-which')(process.cwd())
const checkPkgScripts = require('./checkPkgScripts')

// Find and load the package.json at the root of the project.
const pkg = require(appRoot.resolve('package.json')) // eslint-disable-line import/no-dynamic-require
const debug = require('debug')('lint-staged:find-bin')

module.exports = function findBin(cmd, scripts, debugMode) {
const cache = new Map()

module.exports = function findBin(cmd) {
debug('Resolving binary for command `%s`', cmd)
const npmArgs = (bin, args) =>
// We always add `--` even if args are not defined. This is required
// because we pass filenames later.
['run', debugMode ? undefined : '--silent', bin, '--']
// args could be undefined but we filter that out.
.concat(args)
.filter(arg => arg !== undefined)

/*
* If package.json has script with cmd defined we want it to be executed
* first. For finding the bin from scripts defined in package.json, there
* are 2 possibilities. It's a command which does not have any arguments
* passed to it in the lint-staged config. Or it could be something like
* `kcd-scripts` which has arguments such as `format`, `lint` passed to it.
* But we always cannot assume that the cmd, which has a space in it, is of
* the format `bin argument` because it is legal to define a package.json
* script with space in it's name. So we do 2 types of lookup. First a naive
* lookup which just looks for the scripts entry with cmd. Then we split on
* space, parse the bin and args, and try again.
*
* Related:
* - https://github.com/kentcdodds/kcd-scripts/pull/3
* - https://github.com/okonet/lint-staged/issues/294
* Try to locate the binary in node_modules/.bin and if this fails, in
* $PATH.
*
* Example:
* This allows to use linters installed for the project:
*
* "scripts": {
* "my cmd": "echo deal-wth-it",
* "demo-bin": "node index.js"
* },
* "lint-staged": {
* "*.js": ["my cmd", "demo-bin hello"]
* }
* "lint-staged": {
* "*.js": "eslint"
* }
*/
if (scripts[cmd] !== undefined) {
// Support for scripts from package.json
debug('`%s` resolved to npm script - `%s`', cmd, scripts[cmd])
return { bin: 'npm', args: npmArgs(cmd) }
}

const parts = cmd.split(' ')
let bin = parts[0]
const binName = parts[0]
const args = parts.splice(1)

if (scripts[bin] !== undefined) {
debug('`%s` resolved to npm script - `%s`', bin, scripts[bin])
return { bin: 'npm', args: npmArgs(bin, args) }
if (cache.has(binName)) {
debug('Resolving binary for `%s` from cache', binName)
return { bin: cache.get(binName), args }
}

/*
* If cmd wasn't found in package.json scripts
* we'll try to locate the binary in node_modules/.bin
* and if this fails in $PATH.
*
* This is useful for shorter configs like:
*
* "lint-staged": {
* "*.js": "eslint"
* }
*
* without adding
*
* "scripts" {
* "eslint": "eslint"
* }
*/

try {
/* npm-which tries to resolve the bin in local node_modules/.bin */
/* and if this fails it look in $PATH */
bin = npmWhich.sync(bin)
const bin = npmWhich.sync(binName)
debug('Binary for `%s` resolved to `%s`', cmd, bin)
cache.set(binName, bin)
return { bin, args }
} catch (err) {
throw new Error(`${bin} could not be found. Try \`npm install ${bin}\`.`)
// throw helpful error if matching script is present in package.json
checkPkgScripts(pkg, cmd, binName, args)
throw new Error(`${binName} could not be found. Try \`npm install ${binName}\`.`)
}

debug('Binary for `%s` resolved to `%s`', cmd, bin)
return { bin, args }
}
9 changes: 1 addition & 8 deletions src/index.js
Expand Up @@ -2,7 +2,6 @@

'use strict'

const appRoot = require('app-root-path')
const dedent = require('dedent')
const cosmiconfig = require('cosmiconfig')
const stringifyObject = require('stringify-object')
Expand All @@ -13,9 +12,6 @@ const runAll = require('./runAll')

const debug = require('debug')('lint-staged')

// Find the right package.json at the root of the project
const packageJson = require(appRoot.resolve('package.json'))

// Force colors for packages that depend on https://www.npmjs.com/package/supports-color
// but do this only in TTY mode
if (process.stdout.isTTY) {
Expand Down Expand Up @@ -56,10 +52,7 @@ module.exports = function lintStaged(injectedLogger, configPath, debugMode) {
debug('Normalized config:\n%O', config)
}

const scripts = packageJson.scripts || {}
debug('Loaded scripts from package.json:\n%O', scripts)

runAll(scripts, config, debugMode)
runAll(config)
.then(() => {
debug('linters were executed successfully!')
// No errors, exiting with 0
Expand Down
5 changes: 2 additions & 3 deletions src/runAll.js
Expand Up @@ -12,11 +12,10 @@ const debug = require('debug')('lint-staged:run')

/**
* Executes all tasks and either resolves or rejects the promise
* @param scripts
* @param config {Object}
* @returns {Promise}
*/
module.exports = function runAll(scripts, config, debugMode) {
module.exports = function runAll(config) {
debug('Running all linter scripts')
// Config validation
if (!config || !has(config, 'concurrent') || !has(config, 'renderer')) {
Expand All @@ -37,7 +36,7 @@ module.exports = function runAll(scripts, config, debugMode) {
const tasks = generateTasks(config, filenames).map(task => ({
title: `Running tasks for ${task.pattern}`,
task: () =>
new Listr(runScript(task.commands, task.fileList, scripts, config, debugMode), {
new Listr(runScript(task.commands, task.fileList, config), {
// In sub-tasks we don't want to run concurrently
// and we want to abort on errors
dateFormat: false,
Expand Down
4 changes: 2 additions & 2 deletions src/runScript.js
Expand Up @@ -12,7 +12,7 @@ const resolveGitDir = require('./resolveGitDir')

const debug = require('debug')('lint-staged:run-script')

module.exports = function runScript(commands, pathsToLint, scripts, config, debugMode) {
module.exports = function runScript(commands, pathsToLint, config) {
debug('Running script with commands %o', commands)

const normalizedConfig = getConfig(config)
Expand All @@ -28,7 +28,7 @@ module.exports = function runScript(commands, pathsToLint, scripts, config, debu
title: linter,
task: () => {
try {
const res = findBin(linter, scripts, debugMode)
const res = findBin(linter)

// Only use gitDir as CWD if we are using the git binary
// e.g `npm` should run tasks in the actual CWD
Expand Down
16 changes: 10 additions & 6 deletions test/__mocks__/npm-which.js
@@ -1,10 +1,14 @@
const mockFn = jest.fn(path => {
if (path.includes('missing')) {
throw new Error(`not found: ${path}`)
}
return path
})

module.exports = function npmWhich() {
return {
sync(path) {
if (path.includes('missing')) {
throw new Error(`not found: ${path}`)
}
return path
}
sync: mockFn
}
}

module.exports.mockFn = mockFn
19 changes: 19 additions & 0 deletions test/__snapshots__/checkPkgScripts.spec.js.snap
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`checkPkgScripts throws if the binary name is defined in script 1`] = `
"
WARN \`lint-staged\` no longer supports running scripts defined in package.json.
The same behavior can be achieved by changing the command to any of the following:
- \`npm run lint -- --fix\`
- \`eslint . --fix\`"
`;

exports[`checkPkgScripts throws if the cmd is defined in script 1`] = `
"
WARN \`lint-staged\` no longer supports running scripts defined in package.json.
The same behavior can be achieved by changing the command to any of the following:
- \`npm run lint -- \`
- \`eslint . \`"
`;
10 changes: 10 additions & 0 deletions test/__snapshots__/findBin.spec.js.snap
@@ -0,0 +1,10 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`findBin should throw a helpful error if the cmd is present in pkg scripts 1`] = `
"
WARN \`lint-staged\` no longer supports running scripts defined in package.json.
The same behavior can be achieved by changing the command to any of the following:
- \`npm run lint -- \`
- \`eslint . \`"
`;

0 comments on commit d8b836c

Please sign in to comment.