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

Fix declaration-block-no-duplicate-properties autofix for !important #6523

Closed
sidx1024 opened this issue Dec 14, 2022 · 3 comments · Fixed by #6528
Closed

Fix declaration-block-no-duplicate-properties autofix for !important #6523

sidx1024 opened this issue Dec 14, 2022 · 3 comments · Fixed by #6528
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@sidx1024
Copy link
Member

What steps are needed to reproduce the bug?

  1. Enable declaration-block-no-duplicate-properties rule
  2. Run stylelint --fix against a file like this:
.box {
  display: flex !important;
  display: inline-flex;
}

The css fixed by stylelint becomes:

.box {
  display: inline-flex !important;
  display: inline-flex;
}

The fix is incorrect because before fixing the css the effective value for display would be flex and after the fix it is inline-flex.

What Stylelint configuration is needed to reproduce the bug?

{
  "plugins": [
    "stylelint-scss",
  ],
  "rules": {
    "declaration-block-no-duplicate-properties": true
  }
}

How did you run Stylelint?

stylelint file.scss --fix

Which version of Stylelint are you using?

13.12.0

What did you expect to happen?

The expected file after auto fix should be:

.box {
  display: flex !important;
}

If the case is difficult to handle the issue should not be auto-fixed.

What actually happened?

The issue was auto-fixed and the file after the fix became:

.box {
  display: inline-flex !important;
  display: inline-flex;
}

Does the bug relate to non-standard syntax?

No response

Proposal to fix the bug

  1. Avoid auto-fixing if one of the duplicates have !important.
  2. Apply the effective value for the property when !important is present.
@ybiquitous
Copy link
Member

@sidx1024 Thanks for the report. As you commented, the current declaration-block-no-duplicate-properties rule doesn't consider !important, and I agree that the rule auto-fix should remain only one effective property.

I guess there are the following cases for !important:

Case 1

When there is one !important:

a {
  color: red !important;
  color: green;
}

should become ⬇️

a {
  color: red !important;
}

Case 2

When there are multiple !important:

a {
  color: red !important;
  color: green !important;
}

should become ⬇️

a {
  color: green !important;
}

In other words, only the last !important should be kept.

What do you think?

@ybiquitous ybiquitous added status: needs discussion triage needs further discussion type: bug a problem with a feature or rule labels Dec 14, 2022
@jeddy3 jeddy3 changed the title declaration-block-no-duplicate-properties should not auto-fix properties with !important Fix declaration-block-no-duplicate-properties autofix for !important Dec 15, 2022
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone and removed status: needs discussion triage needs further discussion labels Dec 15, 2022
@jeddy3
Copy link
Member

jeddy3 commented Dec 15, 2022

In other words, only the last !important should be kept.

SGTM.

I've labelled the issue as ready to implement. @sidx1024 Please consider contributing if you have time.

There are steps on how to fix a bug in a rule in the Developer guide.

@sidx1024
Copy link
Member Author

Sure. Raised a PR #6528

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

3 participants