Add stylelint

[karlhorky]

Hi there! First of all, thanks to all the contributors for this awesome project. Having all the tools, configuration and setup in one place helps tons.

create-react-app doesn't have any CSS linting yet. I noticed that there was tentative agreement for further consideration in #636 (comment), so I decided to open a PR for this, since I need it for a project anyway.

I've included the standard config because I don't have a full opinionated stylelint config yet, but this can of course be changed.

Note: This depends on my fork of @vieron's stylelint webpack plugin, because the errors were not displaying correctly using watch mode in CRA. I've opened a pull request on the plugin here: webpack/stylelint-webpack-plugin#43

Edit: Pull request has been merged!

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[cr101]

Or you could use a Sass linter for both SASS and SCSS syntax!

[karlhorky]

stylelint also supports CSS-like syntaxes like SCSS, SugarSS and Less. This is however not relevant for create-react-app, which currently only supports vanilla CSS.

The sasslint-webpack-plugin is also deprecated in favor of stylelint: https://github.com/alleyinteractive/sasslint-webpack-plugin/blob/master/README.md

[karlhorky]

webpack/stylelint-webpack-plugin#43 has been merged and released as 0.4.1! I've updated the dependency accordingly.

[JaKXz]

@karlhorky I recommend another update here to capture our fixes from this morning (v0.4.2).

[karlhorky]

Done!

[gaearon]

Can you provide some details about node_modules disk size before and after this change please?

Also, would this work well for people compiling Sass/Less into CSS, and importing the compiled CSS?

Does this plugin work well with Webpack 2?

[karlhorky]

some details about node_modules disk size before and after

du -sh node_modules before and after (13M difference):

109M node_modules # before
122M node_modules # after

would this work well for people compiling Sass/Less into CSS

It could lint the original source Sass/Less if I changed the file extensions that are matched by the plugin. I however only enable linting of source files (disabling the linting of any built CSS files) because the built output will be optimized and is thus not a good candidate for linting.

Does this plugin work well with Webpack 2?

I tested this with a simple application and the plugin works. As for working well, I haven't tested extensively yet - maybe @JaKXz can offer some insight here?

[karlhorky]

I've modified the eject script to create a stylelint configuration entry as well, although webpack/stylelint-webpack-plugin#67 may interfere until it's resolved.

Edit: This appears to be resolved in 0.6.0. Bumping the version.

[JaKXz]

Does this plugin work well with Webpack 2?

The plugin is actively tested against webpack 2 as of webpack/stylelint-webpack-plugin#46, so I have confidence in the logic against webpack's compiler [from v0.5.0 on].

[gaearon]

It could lint the original source Sass/Less if I changed the file extensions that are matched by the plugin. I however only enable linting of source files (disabling the linting of any built CSS files) because the built output will be optimized and is thus not a good candidate for linting.

The scenario I meant is when people compile Sass to CSS but keep it in src next to the components. That seemed like the most straightforward way to use a preprocessor to me.

[karlhorky]

compile Sass to CSS but keep it in src next to the components

Ah yeah, that would also be supported by changing the file extensions.

[JaKXz]

one thing to consider is that stylelint can handle .sass/.scss files, and the default glob for stylelint-webpack-plugin supports both so you could drop this line.

[karlhorky]

Ah, nice idea. If @gaearon agrees we want to go this direction, I will make this change.

[karlhorky]

@gaearon any feedback to the files config option? Should I just remove it as @JaKXz suggested to allow Stylelint to lint all supported file extensions?

[shai32]

Is this PR still active?, will it be merged?

[karlhorky]

If there's any interest I can rebase and fix conflicts (and upgrade to webpack 2 if necessary).

[roytz]

There is much interest :) (and to the upgrade to webpack 2 too)

[cr101]

Handling .sass/.scss files is essential

[shai32]

@karlhorky please upgrade to webpack 2

[shai32]

Is this pr active?

[karlhorky]

@gaearon The answer I gave before to that question is above. If there's still interest in this PR I could rebase and update all dependencies and check the size again.

[gaearon]

Lol I'm sorry 😛
Yes, please do rebase.

[karlhorky]

Ok, I'll get to this in the next day or so.

[gaearon]

I'd also like to better understand the performance implications of this. If the project has e.g. 500 300-line CSS files, how much does this plugin affect the development and production build times?

People already complain about the builds being super slow so I'm not really sure we have the capacity to add any more build-time work.

[JaKXz]

Perhaps this should be opt-in like some others in #3815? I think the lintDirtyModulesOnly option will help wrt build-time work, so if you have 500 files but only a few have changed then only those will be linted. The separate lint task (#2729) can just call stylelint directly for all files.

FYI v0.10.1 of the plugin was released recently as well as stylelint v8.

[gaearon]

Perhaps this should be opt-in like some others in #3815?

This seems a bit trickier. We won't make those opt-in by some configuration flag. Instead if you import .sass, we'll ask you to install node-sass. If you install it, Sass will "just work". Similarly, if you import .module.css instead of .css, we will treat it as CSS Modules. But with Stylelint we can't "guess" whether you want it or not. We also don't know if you're writing CSS or compiling to it. I'm also not sure if it can handle CSS Modules. So it seems like this is potentially more involved than it seems. Could the user just install and run Stylelint themselves? I'm not sure it needs to be integrated with the system in the same way that e.g. Sass needs to be for a good experience.

[JaKXz]

I think the intention here is to have stylelint errors side by side with eslint errors in the build, and assuming CRA works with the better errors webpack plugin that should be achievable :) but I understand your point, I agree it's much harder to pinpoint exactly what the user would want from the outset, and that could change with a lot more volatility.

[gaearon]

Retagging as 2.x because I don't think this should block the 2.0 release.
If we can get the numbers that show it's not expensive, sure, let's get it in.

If it slows down our already slow builds I don't think we can do this (but you can always use it separately in your IDE etc).

[caghand]

Yes, you can use stylelint in your IDE, but since the errors do not appear in the build output (like eslint), you won't be able to implement a check-in policy that prevents style-violating CSS from being pushed in. :)

-Caghan

[sstruct]

I'd prefer Prettier

[oriSomething]

Wouldn't be better to use declaration-block-no-duplicate-properties with ignore: ["consecutive-duplicates"]?

Moreover, is this rule blocks the pattern of (Added example)?

margin: 1rem;
margin-bottom: 2rem;

[ai]

If we have ESLint for JS, we definitely should have Stylelint for CSS.