stylelint-scss / stylelint-scss

A collection of SCSS specific linting rules for Stylelint
MIT License
866 stars 94 forks source link

Add lint for loud comments on nested style rules #893

Open Goodwine opened 10 months ago

Goodwine commented 10 months ago

Input

.foo {
  /* Hello! */
  .bar {
    a: b;
  }
}

Output (playground)

.foo {
  /* Hello! */      <------ Issue: User expected comment to appear above `.foo .bar`.
}
.foo .bar {
  a: b;
}

Context

Sass loud comments (/* */) within a rule declaration are not "connected" to anything. They don't "belong" to the subsequent node. So when Sass compiles nested comments and rules to CSS, it "splits" the loud comment away from the nested rule block that the author intended, as shown in the example above.

This could cause issues for libraries like Autoprefixer or RTLCSS that depend on these "loud comments" being positioned at the top of a specific line or block. For instance:

.foo {
  /* rtl:ignore */
  .bar {
    a: b;
  }

  float: left;
}

Which produces:

.foo {
  /* rtl:ignore */
  float: left;      <----- BUG: This is now ignored by RTLCSS, but that wasn't intended.
}
.foo .bar {
  a: b;
}
Goodwine commented 10 months ago

BTW I'm looking to work on this this week unless :)

kristerkari commented 10 months ago

@Goodwine Thanks for the detailed description!

Looking at what you wrote, I started wondering if the problem is maybe a bit too specific to RTLCSS to be added as a rule to this library. I understand that your problem is related to Sass, but I bet that most of the users of stylelint-scss are not using RTLCSS.

Could this idea instead be turned into a separate stylelint plugin that would be published in npm? Stylelint supports the postcss-scss customSyntax, so it's really easy to make a rule support SCSS.

Goodwine commented 10 months ago

Since this affects any loud comment (not just RTLCSS), would it make sense to still implement this lint check as part of this repo if it was made more generic?

For example:

Goodwine commented 1 month ago

I have updated the description of the issue to be more generic.