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

no-duplicate-dollar-variables doesn't respect scopes #277

Closed
robertjk opened this issue Oct 17, 2018 · 5 comments · Fixed by #453
Closed

no-duplicate-dollar-variables doesn't respect scopes #277

robertjk opened this issue Oct 17, 2018 · 5 comments · Fixed by #453
Labels
Enhancement ✨ New feature or request Help wanted 🙋 Help is needed

Comments

@robertjk
Copy link

robertjk commented Oct 17, 2018

no-duplicate-dollar-variables rule reports variables with the same name but declared in different scopes. The snippet below produces error Unexpected duplicate dollar variable $padding-x scss/no-duplicate-dollar-variables.

.FormControl-label {
  $padding-x: $padding-small-x;

  @include input-overlay;

  position: relative;
  top: 0.40em;
  margin-left: $margin-medium-x;
  padding-left: $padding-x;
  padding-right: $padding-x;
  font-size: $font-size-small;
}


.FormControl-input {
  $padding-x: $padding-medium-x;

  width: 100%;
  border: 1px solid $color-grey-light;
  border-radius: 0;
  padding-left: $padding-x;
  padding-right: $padding-x;
  height: $row-height;
}

For me it's pretty obvious that this shouldn't be reported. Am I wrong?

@kristerkari
Copy link
Collaborator

Thanks! The way the rule was created is that it looks for variables in the whole stylesheet's scope. I think that the rule could be changed to be a bit more clever to not warn for your use case.

So, this would warn (overwriting a variable from outer scope):

$a: 1;

.foo {
  $a: 2;
} 

.bar {
  $a: 3;
}

...but this would not warn (the variable is not defined in the outer scope):

.foo {
  $a: 2;
} 

.bar {
  $a: 3;
}

@kristerkari kristerkari added Enhancement ✨ New feature or request Help wanted 🙋 Help is needed labels Oct 17, 2018
@kristerkari kristerkari pinned this issue Jan 22, 2019
@ChrisMBarr
Copy link

ChrisMBarr commented Feb 14, 2019

Another false positive example I ran across today that might be a different use case

@if $value == $light {
    $ring-color: $dark;
} @else {
    $ring-color: $value;
//  ^^^^^^^^^^^ Unexpected duplicate dollar variable $ring-color (scss/no-duplicate-dollar-variables)  
}

and

$ring-color: $value !default;
@if $value == $light {
    $ring-color: $dark;
//  ^^^^^^^^^^^ Unexpected duplicate dollar variable $ring-color (scss/no-duplicate-dollar-variables) 
}

@kristerkari
Copy link
Collaborator

@ChrisMBarr those are not actually false positives, to avoid that you can use the ignore options of the rule:
https://github.com/kristerkari/stylelint-scss/blob/master/src/rules/no-duplicate-dollar-variables/README.md#ignoreinsideatrules-array-of-at-rules

@kristerkari
Copy link
Collaborator

This is now fixed in v3.15.0
https://github.com/kristerkari/stylelint-scss/releases/tag/3.15.0

Thanks everyone for helping out!

@kristerkari kristerkari unpinned this issue Mar 15, 2020
@masi
Copy link

masi commented May 4, 2021

WIth 3.18 the rule triggers:

.foobar {
  $gapWidth: 5px;

  @media (min-width: 600px) {
    $gapWidth: 15px;
  }
}

I had assumed that the media query defines a scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ New feature or request Help wanted 🙋 Help is needed
Development

Successfully merging a pull request may close this issue.

4 participants