Exit code from "snazzy" is ignored

[jsumners]

I'm not sure if this is a bug with pre-commit, a bug with snazzy or both. So I'm filing it here and copying @feross

The following "project" will commit without error despite failing the linting step:

package.json

{
  "scripts": {
    "lint": "snazzy",
    "foo": "exit 0"
  },
  "precommit": [
    "lint",
    "foo"
  ],
  "devDependencies": {
    "pre-commit": "^1.2.2",
    "snazzy": "^6.0.0",
    "standard": "^10.0.0"
  }
}

index.js

'use strict'

function invalid() {
  return 1
} // note missing newline EOF

If you replace "lint": "snazzy" with "lint": "standard" then the commit will fail as it should.

[feross]

@jsumners I can't reproduce the issue. Can you try a few things:

1. Delete node_modules/ and run npm install, just to make sure it's not a silly npm issue. (Don't use yarn or alternate package managers, please)

2. What versions of node and npm? Please share the output of node -v and npm -v.

[feross]

Actually, I can reproduce the issue when I make a commit, so it points to this being a pre-commit issue.

Running snazzy directly via npm run lint or ./node_modules/.bin/snazzy produces the correct exit code of 1.

[jsumners]

Glad you were able to reproduce it @feross . I was going to say that I created the test case from nothing and I don't use anything other than npm.

[feross]

@jsumners I suppose one other useful question is: did this ever used to work in the past? That is, did a recent version of pre-commit or snazzy cause this issue to start happening?

[jsumners]

@feross I do not know the answer to that question.

[feross]

@jsumners I see, so you just tried it for the first time and it didn't work for you.

Could this have something to do with the way that we set the exit code in the process.on('exit') event?
https://github.com/feross/snazzy/blob/f2172ffeeecc3d0d8efdad8b7149fa953c8a2327/bin/cmd.js#L20-L24

@jsumners In the meantime, you can try piping from standard to snazzy instead, like this:

standard --verbose | snazzy

[jsumners]

@feross piping with --verbose produced the desired result:

% git commit -m 'test 2'                                                                                                                                     [s:0 l:8172]
standard: Use JavaScript Standard Style (https://standardjs.com)
standard: Run `standard --fix` to automatically fix some problems.

/private/tmp/06/precommit-test/index.js
  3:10  error  'invalid' is defined but never used            no-unused-vars
  3:17  error  Missing space before function parentheses      space-before-function-paren
  5:2   error  Newline required at end of file but not found  eol-last

✖ 3 problems
pre-commit:
pre-commit: We've failed to pass the specified git pre-commit hooks as the `lint`
pre-commit: hook returned an exit code (1). If you're feeling adventurous you can
pre-commit: skip the git pre-commit hooks by adding the following flags to your commit:
pre-commit:
pre-commit:   git commit -n (or --no-verify)
pre-commit:
pre-commit: This is ill-advised since the commit is broken.
pre-commit:

As did simply piping standard into snazzy.

[feross]

@jsumners Good to hear. I'm seriously considering removing the ability to run snazzy directly anyway, since in addition to this issue there are other issues it has caused. Specifically, this one recently: standard/snazzy#16

Here's the PR that removes the ability to run snazzy directly: standard/snazzy#17