stylelint-validator icon indicating copy to clipboard operation
stylelint-validator copied to clipboard

Make @import rules require a semicolon

Open trainiac opened this issue 8 years ago • 7 comments
trafficstars

I created an issue in the stylelint repo, but they believed this repository would be a better candidate for this type of feature.

The problem is, if you forget to include a semicolon the import on the following line will not get exectued which is pretty annoying not to have a warning for when developing.

trainiac avatar Feb 15 '17 00:02 trainiac

@lahmatiy Also, thanks for 1.1.0 release. Both Sass and CSS Modules syntax work well in my repository.

trainiac avatar Feb 15 '17 00:02 trainiac

PostCSS's AST doesn't store information about semicolons. So @import 'foo' and @import 'foo'; looks the same. No clue how to detect a problem. Moreover when you omit a semicolon it includes following until next semicolon or curly bracket to at-rule expression, i.e.

@import url('styles/elements')
@import url('styles/ui/base');

represents as single AST node:

{
  "type": "atrule",
  "name": "import",
  "params": "url('styles/elements')\n@import url('styles/ui/base')",
  ..
}

It's a quite complicated issue. Actually there should be a parse error.

Currently CSSTree lexer (that using by this plugin) tests declaration values only. It's going to support validation for at-rules and selectors a little bit later. Then it will detect that something wrong is going after url. But we need parse an at-rule expression before validation and parsing will fail (on @import inside at-rule expression).

Summarise, the best we can get is warning about parsing on second @import. It can help to figure out that's something wrong nearby. But this is not exactly what you'd expect.

lahmatiy avatar Feb 15 '17 00:02 lahmatiy

Makes sense. Thanks for the explanation.

As mentioned in the other issue, I do get an indentation error warning from stylelint that at least lets me know something is wrong with the imports, but during development I only have errors enabled that lead to severe errors in the application being able to perform. Indentation errors only frustrate me while I'm developing, so I leave it as a warning until committing code. However leaving out a semicolon will fail to include the next line import and results in what I would say a pretty severe error that I would want to know about right away.

trainiac avatar Feb 15 '17 01:02 trainiac

looking forward to the updates ;)

trainiac avatar Feb 15 '17 01:02 trainiac

Hi, I found that semicolon really matters.

Sample file:

@import './base.css'

.abc, .def {
  position: absolute;
}

.abc {
  display: flex;
}

When compiled by postcss & css-loader, .abc, .def { ... } is just gone, without any warnings or errors. The same for stylelint (tried stylelint-webpack-plugin).

It took me a long time debugging (and founded in css-loader-parser of css-loader/lib/processCss.js).

It might be a postcss AST problem, but hopefully if there will be a parse error :)

lemori avatar Sep 19 '17 07:09 lemori

CSSTree is not ready for a full validation checking of at-rules. Anyway it's going that direction and very close to it. The main problem with stylelint & postcss that non-CSS syntax may to be used as input. In this case any CSS syntax extensions used in a at-rule prelude would lead to unwanted error. I think about a workaround. Although maybe I do not need to worry about it ;)

lahmatiy avatar Sep 19 '17 09:09 lahmatiy

Oh, what's the workaround? Maybe we just have to use it with caution. Anyway, it's easy to identify.

lemori avatar Sep 20 '17 03:09 lemori