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: allow dup vars in unrelated scopes #453

Merged
merged 2 commits into from Mar 15, 2020

Conversation

srawlins
Copy link
Contributor

@srawlins srawlins commented Feb 8, 2020

Fixes #277

let parent = decl.parent;

while (parent !== null && parent !== undefined) {
const parentKey = parent.toString();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be inefficient - I think this serializes the entire subtree below [parent]. I'm not sure what to use as unique keys to traverse down the scopes though. the non-leaf nodes are all Containers I think? I couldn't find a good common way to uniquify them...

@kristerkari
Copy link
Collaborator

Thanks! This seems like a good fix to the rule.

Copy link
Collaborator

@kristerkari kristerkari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be one additional test to check that we get a warning for .d {}, but not for the other variables:

      .a {
        $ab: 1;
      }
      .b {
        $ab: 2;
      }
      .c {
        $ab: 3;
        .d {
          $ab: 4;
        }
      }

The README of the rule should also be updated to mention that the rule considers scopes, and give an example of the scopes.

@srawlins
Copy link
Contributor Author

Done. Additional test and README text.

@kristerkari
Copy link
Collaborator

Great, thanks! Going to do a new release with the currently approved PRs.

@kristerkari kristerkari merged commit c1ab933 into stylelint-scss:master Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

no-duplicate-dollar-variables doesn't respect scopes
2 participants