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

mixins-before-declarations Errors #230

Closed
kamalbennani opened this issue Sep 28, 2015 · 3 comments
Closed

mixins-before-declarations Errors #230

kamalbennani opened this issue Sep 28, 2015 · 3 comments

Comments

@kamalbennani
Copy link

Hello,
When i used you sass lint, i found this error which i found little bit confused, normally it's shouldn't cause this error.
Could you please explain to me the problem ?
Thanks

.area-current {
  $active: #00acac;
  $background: #2d353c;

  @include area-menu($active, $background);
}

.area-new {
  $active: #348fe2;
  $background: #2e475d;

  @include area-menu($active, $background);
}

.area-disappeared {
  $active: #727cb6;
  $background: #424a61;

  @include area-menu($active, $background);
}
botify-report/sass/_themes.scss
   96:3  error  Mixins should come before declarations  mixins-before-declarations
  103:3  error  Mixins should come before declarations  mixins-before-declarations
  110:3  error  Mixins should come before declarations  mixins-before-declarations
@DanPurdy
Copy link
Member

I'm gonna label this is as a bug but I think it needs further discussion too. Is there any reason you are declaring variables in each block like that? There's not really any benefit to using them in that code.

Technically a variable declaration is classed as a declaration and so should come after a mixin, now obviously in the case you have this wouldn't be practical. Is there a lot more going on that Is using those variables within that block?

So my basic question to @bgriffith and @Snugug and anyone else with an opinion is should the above case flag as an error or not.

@bgriffith
Copy link
Member

No. Should only cause an error if it's a propert being set in my opinion.

@BPScott
Copy link

BPScott commented Oct 1, 2015

I'm gonna label this is as a bug but I think it needs further discussion too. Is there any reason you are declaring variables in each block like that? There's not really any benefit to using them in that code.

Variables defined within a block are useful as they are scoped to that block.

I agree with @bgriffith: this should only be an error when a mixin appears before a property or a nested selector by default.

Potentially in the future we could have a rule to define the expected order of: {variable declarations, extends, mixins, properties, nested selectors} (and anything else I've missed) to superseed and unify the mixins-before-declarations, extends-before-declarations and extends-before-mixins rules but that might end up being that little bit too complicated.

donabrams pushed a commit to donabrams/sass-lint that referenced this issue Nov 19, 2015
Add check for declaration node content type
Fixes sasstools#227
Fixes sasstools#230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants