Add Stylelint about pwa HOT 8 OPEN

Hey, thanks!

For sure – 💯 I don't use this tool & so I completely forgot it was even an option.

... I think I'll add this as a new plugin (and init option) to the next minor release this weekend.

from pwa.

Hey, so this needs to wait a bit. I lost most of the day playing with Stylelint because it:

1. Doesn't support Stylus at all (stylelint/stylelint#674)

2. Doesn't work with SASS/SCSS alongside the CSS modules selectors.

I tried two approaches:

A) Adding it as a PostCSS plugin directly

This made most of SASS appear to work, although I'm not confident that it does because Option (B) shows significantly more errors & warnings per file. Also, each warning is printed twice in this mode.

B) Using the stylelint-webpack-plugin directly.

The error formatting is correct, looks nice, but this mode is the most broken & inconsistent – see below.

For (2), I have stylelint-config-recommended and stylelint-config-css-modules installed & extended. It still complains about \:root being an unknown type selector.

It will ignore \\:root entirely, but that breaks SASS. And it'll accept :root but sass-loader will not since it needs the escapes.

In fact, nearly all non-CSS preprocessors have some kind of CSS-modules escape character.

// less
&:root {
  --blue: #1E88E5;
}
&:global(.penta) {
  background-image: url(@assets/shapes/penta.svg);
}

// sass
\:root
  --blue: #1E88E5
\:global(.penta)
  background-image: url(@assets/shapes/penta.svg)

// stylus
:root
  --blue #1E88E5
:global(.penta)
  background-image url("@assets/shapes/penta.svg")

All in all, I'd be happy to add this as an official plugin, but not in its current state. There are too many holes as is – or, at least, too many inconsistencies that PWA can't solve in a single package on the user's behalf. I'd much rather wait on a good addition than put out a semi-baked solution that only works some of the time.

Unfortunately, I don't have the spare time right now to contribute towards these gaps, otherwise I most definitely would.

from pwa.

Doesn't support Stylus at all

Yeap. There is no way to support since:

1. The syntax is incredibly hard to write the parser
2. It has no active development to ask them to give AST from their parser
3. There is no big user base

Doesn't work with SASS/SCSS alongside the CSS modules selectors.

Hm. We can fix postcss-scss to parse \: as :. This syntax works in every place of SCSS?

In fact, nearly all non-CSS preprocessors have some kind of CSS-modules escape character.

Off-topic: This is why you can use PostCSS for preprocessors too ;). It works perfectly with CSS Modules, since CSS Modules is just PostCSS plugin.

from pwa.

Yeah, understood. I could live with putting a disclaimer for "no Stylelint with Stylus".

Yes, SCSS/SASS expects \: escapes – they're the same in this regard. Although, I didn't make it very clear above, but this seems to be an issue with the Webpack plugin on. When Stylelint is attached to the PostCSS chain, it accepts the escapes.

And yeah, I see that attaching stylelint as a PostCSS plugin is the way to go, but the formatting on the errors & warnings is funky and I'm not able to "normalize" it into the Webpack format.

Here are the outputs from both approaches:

Notice how there's only 1 warning via the PostCSS route but that it's duplicated.

from pwa.

The best solution, for Webpack context, would be to have a stylelint-loader that ran in the enforce: "post" mode. This would mean that it runs after all other loaders for the given file extension(s). Since PostCSS (+ SASS, + Stylus, etc) will run first, then the post-loaded Stylelint loader is always running on the transformed CSS.

This would solve the Stylus issue and prevent any inconsistencies that I've been experiencing.

I think the only thing you lose is the ability to lint the spacing on the input files, although each preprocessor might be preserving the original spacing.

from pwa.

Yes, SCSS/SASS expects : escapes – they're the same in this regard. Although, I didn't make it very clear above, but this seems to be an issue with the Webpack plugin on.

Let’s create a issue in webpack stylelint loader.

I think fixing the loader will fix all current problems.

from pwa.

The best solution, for Webpack context, would be to have a stylelint-loader that ran in the enforce: "post" mode.

I am not sure.

1. Stylelint plugin for text editor will show different mistakes
2. ESLint and Stylelint behaviour will be different

Allso, are you sure that webpack loader is a best place for linter? I see there are 2 reasons for using linter:

1. Avoid spending time for a stupid mistake
2. Prevent master from pushing mistakes. Also, teach a coding standard for junior developers.

Both cases are not really covered by webpack’s loader. Nobody really keeps webpack console in front on eyes to see a warning in the right time. Junior will see error only on CI server (and will blame theirself that everyone now see this mistake).

I think the best way of using linter: text editor plugins and lint-staged.

from pwa.

This is what the enforce: "post" does:

1. Save file foo.styl
2. PostCSS: Stylus to CSS
3. PostCSS: Autoprefixer on CSS (optional)
4. Stylelint on CSS

Compare that to the ESLint approach:

This is if you use enforce: "pre" OR if ESlint is first loader

1. Save file foo.js
2. ESLint on JS
3. Babel: Apply transforms

ESLint is able to do this because they're working with a normalized syntax (JS) whereas Stylelint is attempting to ingest everything, but it (knowingly) can't.

The more analogous example for ESLint would be linting your CoffeeScript files.

I would also say TypeScript, but it has its own Linter (due to drastic syntax differences).

1. Save file foo.coffee
2. Convert Coffee to JS
3. ESLint the JS
4. Babel: Apply any transformations

In all scenarios, you still get linter feedback per-save. The difference is just in when the linting happens.

¯_(ツ)_/¯

I know it's not great & not what you have in mind, but to me it seems like the only way to normalize the behavior is to normalize what Stylelint ingests.

from pwa.