Lint-staged: Duplicated errors

[dargullin]

@okonet thank you for the package.

I got duplicates in output while trying to use link-staged with husky, here's my config:

 "scripts": {
    "precommit": "lint-staged"
  },
  "lint-staged": {
    "gitDir": "../../",
    "linters": {
      "app/front-end/src/**/*.{js,jsx}": "eslint --quiet"
    }
  },

Output:

> husky - npm run -s precommit

[?25l[13:18:41] Running tasks for app/front-end/src/**/*.{js,jsx} [started]
[13:18:41] eslint --quiet [started]
[13:18:42] eslint --quiet [failed]
[13:18:42] → 🚫 eslint --quiet found some errors. Please fix them and try committing again.


/path/app/front-end/src/redux/actions/facebook.js
  32:24  error  'createAcffgtion' is not defined  no-undef

✖ 1 problem (1 error, 0 warnings)


[13:18:42] Running tasks for app/front-end/src/**/*.{js,jsx} [failed]
[13:18:42] → 🚫 eslint --quiet found some errors. Please fix them and try committing again.


/path/app/front-end/src/redux/actions/facebook.js
  32:24  error  'createAcffgtion' is not defined  no-undef

✖ 1 problem (1 error, 0 warnings)


[?25h🚫 eslint --quiet found some errors. Please fix them and try committing again.


/path/app/front-end/src/redux/actions/facebook.js
  32:24  error  'createAcffgtion' is not defined  no-undef

✖ 1 problem (1 error, 0 warnings)


[?25h
> husky - pre-commit hook failed (add --no-verify to bypass)
> husky - to debug, use 'npm run precommit'

Looks like lint-staged and husky both output the same error.
Here I got 2 errors from lint-staged and one from husky.

Can we add some --silent option for lint-staged? So it would not output the errors, but throw them forward to husky (I also use husky for another lintings).

[dargullin]

I've also tried "precommit": "lint-staged -s", it doesn't work now.

[okonet]

It looks like you're running lint-staged with verbose renderer but I'm not sure why this is happening. Are you using GUI and not CLI?

[dargullin]

@okonet I'm using a latest version of Git Tower for Mac https://www.git-tower.com/.

[okonet]

I see. I don't use git GUIs so this isn't a priority for me but you can write a custom renderer based on https://github.com/SamVerschueren/listr-verbose-renderer that would suppress the intermediate errors and only show them in the end and submit a PR.

[dargullin]

@okonet as many other people do, I use GUI to visualize changes.

I hope we don't need a different renderer for this issue, we just need to support a silent logLevel https://docs.npmjs.com/misc/config#loglevel So it won't output the errors but will keep them for Husky, which would print them inside GUI.

[okonet]

npm scripts are running in the silent mode by default when used with lint-staged: https://github.com/okonet/lint-staged/blob/master/src/findBin.js#L17

Again, feel free to explore what works best for you and submit a PR.

[julec]

I'm using CLI and have the same issue with stylelint.
It works fine when linting starts on pre-commit for all the files.
But when I include the lint-staged into the process, the output is tripled.

[okonet]

@julec please show me your config

[julec]

@okonet
```
"scripts": {
"precommit": "lint-staged",
"stylelint-staged": "stylelint --syntax scss"
},
"lint-staged": {
"*.scss": "stylelint-staged"
}

[julec]

@okonet
The output differs a little bit if to run the linter in Webstorm Terminal:

> Running tasks for *.scss
   →  53:1  ×  Expected empty line before at-rule   at-rule-empty-line-before
   × stylelint
     →  53:1  ×  Expected empty line before at-rule   at-rule-empty-line-before
� stylelint found some errors. Please fix them and try committing again.


components/buttons/_buttons.scss
 53:1  ×  Expected empty line before at-rule   at-rule-empty-line-before

Could the problem be in listr package?

[okonet]

Yeah, it seems like Listr displaying errors multiple time. We could try a custom renderer to fix this.

[julec]

@okonet Could you help me with that? I'm not sure I understand what to do.

[okonet]

@SamVerschueren do you think this can be switched off somehow? Or do we need to write a custom renderer for that?

[SamVerschueren]

Just to understand what is happening. stylelint is a subtask of Running tasks for *.scss right? And the problem here is that the error is shown in the subtask as well as in the root task.

> Running tasks for *.scss
   →  53:1  ×  Expected empty line before at-rule   at-rule-empty-line-before
   × stylelint
     →  53:1  ×  Expected empty line before at-rule   at-rule-empty-line-before

This is already fixed in master but wanted to land something else first before I released the next version. Let me look at it tomorrow and I'll see if I can get it in, otherwise I will release a new version.

[okonet]

@SamVerschueren thanks for chiming in. Yes, exactly. Additionally, since I'm displaying all errors from all linters after they all are done running once again, I'm not even sure I want to show any of them during the run. Is that possible?

[SamVerschueren]

Currently not. Though this might indeed be nice. What do you think about adding an option to listr-update-renderer like supressOutput which supresses all the output. Would that work for you or are you using other output features in combination with Observables for instance?

[okonet]

Right now we're using it for skipped files as well:

So this might be a problem... I could try, though.

[SamVerschueren]

Let me check later today and I'll see what I can come up with. supressOutput might not be ideally.

[SamVerschueren]

The duplicate error thingy should be fixed now in [email protected].

[okonet]

Did I say already how awesome you are? 🥇

[SamVerschueren]

Did I say already how awesome you are? 🥇

Don't need to, but thanks :) :beers:

[julec]

Guys, I've updated lint-staged but nothing have changed in CLI results.
Some changes are in Webstorm:

```

Running tasks for *.scss
× stylelint-staged
→ 27:2 × Expected newline before "}" of a multi-line block block-closing-brace-newline-before
� stylelint-staged found some errors. Please fix them and try committing again.

docs/scss/_buttons.scss
26:32 × Expected a trailing semicolon declaration-block-trailing-semicolon
27:2 × Expected newline before "}" of a multi-line block block-closing-brace-newline-before

[okonet]

Well, I think there is 1 less. We'll still need to do something about the duplication, it seems. 🤔

[SamVerschueren]

It's because the Listr output is separate from the lint-staged output. So Listr just works as intended. I'm open to suggestions. Something like supressErrorOutput maybe? That, or lint-staged should create it's own renderer. I'm willing to do it myself or help out to do it.

[okonet]

If you think such option would be beneficial for Listr, I'd prefer it. Otherwise I could try making a custom renderer but it seems it will be a lot code duplication. Your call.

[SamVerschueren]

I think an option could make sense. I'll have a look at it in a couple of days. Quite busy lately. I'll keep you posted.

[justinhp]

Has a resolution to this been found? Just wondering bc it has some of the less-experienced/new devs confused when they see the error output and depending on how many errors it can get really difficult to read.

[okonet]

Does anyone wants to stab this one? I'd be happy about a PR

[JonKrone]

I took a look at this problem yesterday as a team I work with has been a little frustrated by this, too. Some of our script output can really blow up your screen when printed a few times.

I came across an option in Listr, nonTTYrenderer, which isn't set by lint-staged and defaults to verbose. When set to silent, error output prints correctly.

I see this issue when I use Git Bash but not on a standard Windows terminal and as OP @fronteed also noted he was running lint-staged in a Git Tower application, I think this might be the problem - nonTTY environments just use a verbose renderer.

I'm not sure this is the root cause and so want to run this by @okonet before submitting a PR. Thinking we can either just set the Listr nonTTYRenderer option to 'silent' or set it based on a flag.

[okonet]

@JonKrone If it solves it please go for it. Lint-staged can't be used on CI, so we won't have any problem with overriding it I think. The only place where we use verbose renderer is tests but we must set it explicitly I think.

[JonKrone]

Great, I'll work on a PR with tests and all that this weekend. I want to know more about the TTY distinction and why it thinks my output channel isn't a TTY.