Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task lintCss always succeeds #4

Open
brendanfalkowski opened this issue Mar 23, 2019 · 5 comments
Open

Task lintCss always succeeds #4

brendanfalkowski opened this issue Mar 23, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@brendanfalkowski
Copy link
Member

Problem

When the lintCss task encounters an error it also fires a success notification (it should not).

Steps to reproduce

  • Run gulp watch
  • Edit the file: /css/src/module/_thing.scss
  • Type x on the last line + save
  • This will fail to compile and trigger the linter

Expected outcome

  1. Task css fails + fires an error OS notification
  2. Task lintCss fails + fires an error OS notification

Actual outcome

  1. Task css fails + fires an error OS notification
  2. Task lintCss fails + fires an error OS notification
  3. Task lintCss fires a success OS notification ❌

Video

video

@brendanfalkowski brendanfalkowski added the bug Something isn't working label Mar 23, 2019
@brendanfalkowski
Copy link
Member Author

brendanfalkowski commented Mar 23, 2019

See: https://github.com/olegskl/gulp-stylelint

Tried these plugin options:

  • failAfterError: true — no effect
  • { reporters: [{ formatter: errorFormatter }] } — Using this repo's errorFormatter() as the formatter messes up the notify plugin
  • fix: true — didn't fix the issue, but doesn't work in this task flow because the lintCss task always writes via gulp.dest() which triggers the css task (recursive loop)

@phated
Copy link
Contributor

phated commented Mar 23, 2019

@brendanfalkowski I think we need to submit some PRs to that repository to split the linting and reporting into separate streams (similar to how gulp-eslint works). I'm really disappointed in the quality of gulp-stylelint 😭

@phated
Copy link
Contributor

phated commented Mar 23, 2019

Is there a reason gulp-stylelint is needed over gulp-csslint? The csslint plugin looks to follow gulp patterns better (even though their formatter stream is a Through stream, they at least .resume it). I should actually submit a PR over to them to clean up that.

@brendanfalkowski
Copy link
Member Author

brendanfalkowski commented Mar 25, 2019

@phated — Hmm, at the time, Stylelint seemed like the best choice (still does), even though the Gulp wrapper isn't top notch. I wrote a full ruleset for Stylelint, and appreciate how robust it is:

https://github.com/gravitydepartment/frontend-starter/blob/master/stylelint.config.js

I don't think CSS Lint is a viable choice anymore, except as-is.

Thanks for the insight on the Gulp side of it, but I'd rather keep an incorrect notifier than migrate back out of Stylelint. I'm happy to contribute the use case for a PR to gulp-stylelint, but I'd be wasted on the refactoring job.

Why I chose Stylelint (over CSS Lint)

As I remember it, Stylelint leap-frogged the work of CSS Lint and improved it across the board. So much so that the CSS Lint team decided to merge their work into Stylelint + consolidate the space (back in 2016, but didn't complete): CSSLint/csslint#668

Fast-forward to 2019, Stylelint still seems the better choice:

  1. More rules — with more options.
  2. Better docs — with examples of approving/negating.
  3. SCSS rules — from my past colleague @kristerkari: https://github.com/kristerkari/stylelint-scss
  4. Bigger community + more corporate use (FB + GitHub + WordPress).
  5. More traction: https://www.npmtrends.com/css-lint-vs-csscomb-vs-csslint-vs-stylelint

Screen Shot 2019-03-25 at 3 21 00 PM

Reference: the packages in question

CSS Lint

Stylelint

@phated
Copy link
Contributor

phated commented Mar 25, 2019

That makes a lot of sense! I think it's probably worthwile to upgrade gulp-stylelint to the proper patterns. I wonder if the owner would want to contribute it to https://github.com/gulp-community and then maybe we could spend an "office hours" refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants