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 false positives for functional pseudo-classes in no-descending-specificity #4010

Closed
vegawong opened this issue Apr 7, 2019 · 11 comments · Fixed by #6195
Closed

Fix false positives for functional pseudo-classes in no-descending-specificity #4010

vegawong opened this issue Apr 7, 2019 · 11 comments · Fixed by #6195
Assignees
Labels
status: wip is being worked on by someone syntax: postcss relates PostCSS language extensions type: bug a problem with a feature or rule

Comments

@vegawong
Copy link

vegawong commented Apr 7, 2019

Clearly describe the bug

There are some error tips in my selectors was not expected..
It flagged an error about rule no-descending-specificity when i use css-module presudo function like :global() with a selector.
Thought the selectors had not any relations ,it always flagged an error.
If i rewrite the same selectors without :global, then it works.

Which rule, if any, is the bug related to?

no-descending-specificity

What CSS is needed to reproduce the bug?

.componentName :global(.ant-modal) :global(.ant-btn) {
    color: #fff;
}
.componentName :global(.ant-menu) {
    color: #fff;
}

What stylelint configuration is needed to reproduce the bug?

e.g.

module.exports = {
    extends: ['stylelint-config-standard', 'stylelint-config-css-modules', 'stylelint-config-prettier'],
    rules: {
        'at-rule-no-unknown': [true, { ignoreAtRules: ['function', 'return'] }]
    }
}

Which version of stylelint are you using?

9.10.1

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

CLI with stylelint "**/*.css" --config .stylelintrc.js

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

Yes, it's related to css module pre-sudo function like :global()

What did you expect to happen?

No warnings to be flagged.

What actually happened (e.g. what warnings or errors did you get)?

The following warnings were flagged:

test.css
4:1  ✖  Expected selector ".componentName :global(.ant-menu)" to come before selector ".componentName :global(.ant-modal) :global(.ant-btn)"   no-descending-specificity
@jeddy3 jeddy3 changed the title no-descending-specificity with css module pre-sudo functions selectors works is not expected Fix false positives for functional pseudo-classes in no-descending-specificity Apr 7, 2019
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone syntax: postcss relates PostCSS language extensions type: bug a problem with a feature or rule labels Apr 7, 2019
@jeddy3
Copy link
Member

jeddy3 commented Apr 7, 2019

@vegawong Thanks for the report and for using the template.

It looks like the rule struggles with functional pseudo-classes.

I can reproduce this with:

:fsc1() :fsc2() {}
:fsc3() {}

As the issue has been labelled "help wanted", please consider contributing a fix.

@vegawong
Copy link
Author

vegawong commented Apr 7, 2019

@jeddy3 , Thanks for reply! I am tring to understand how to calculate the specificity by stylelint. I found that the specificity calculate result was not equal to the vscode tips.

@vegawong
Copy link
Author

vegawong commented Apr 7, 2019

Hey guys! Maybe i had found the problem.

const referenceSelectorNode = lastCompoundSelectorWithoutPseudoClasses(

In this line of code, :global is treated as a pseudo selector so that all the last nodes that use :global are treated as the same comparisonContext. Right?
:global and :local are predefined keywords in css-module. I suggest adding these words to keywordSets.pseudoElements to compare them as separate items in comparisonContext.

@yoyo837
Copy link
Contributor

yoyo837 commented Feb 12, 2020

@import '../../variable.less';

@imgSize: 32px;
@gutter: (@menu-collapsed-width - @imgSize) / 2;

.sider,
.logo,
.logo h1 {
  transition: all 0.2s;
}

.sider {
  padding-top: @layout-header-height;
  position: fixed;
  height: 100%;
  left: 0;
  top: 0;
  :global(.ant-layout-sider-children) {
    padding-right: 20px;
    width: calc(~'100% + 20px');
    height: calc(~'100vh - @{layout-header-height}');
    overflow-x: hidden;
    overflow-y: auto;
    -ms-overflow-style: none; // overflow: -moz-scrollbars-none;
    &::-webkit-scrollbar {
      display: none;
    }
  }
}

.logo {
  position: fixed;
  top: 0;
  left: 0;
  height: @layout-header-height;
  line-height: @layout-header-height;
  padding-left: @gutter;
  background: @header-background;
  width: 200px;
  color: @header-color;
  overflow: hidden;
  z-index: 1;
  img,
  h1 {
    display: inline-block;
    vertical-align: middle;
  }
  img {
    height: @imgSize;
  }
  h1 {
    color: #fff;
    font-size: 20px;
    font-family: Avenir, 'Helvetica Neue', Arial, Helvetica, sans-serif;
    font-weight: 600;
    margin: 0 0 0 @gutter;
  }
}

.siderCollapsed {
  .logo {
    padding-left: 24px;
    padding-right: 24px;
    width: auto;
    h1 {
      display: none;
    }
  }
 // here
 // `Expected selector ".siderCollapsed :global(.ant-menu-item-group-title)" to come before selector ".sider :global(.ant-layout-sider-children)::-webkit-scrollbar" (no-descending-specificity)stylelint(no-descending-specificity)`
  :global(.ant-menu-item-group-title) {
    text-align: center;
  }
}

@hazxy
Copy link

hazxy commented Jun 15, 2022

same problem

@ybiquitous
Copy link
Member

ybiquitous commented Jun 15, 2022

The no-descending-specificity rule's implementation has now changed and improved by using heavily the @csstools/selector-specificity package. See #6131 and the following code for details.

const { selectorSpecificity: calculate, compare } = require('@csstools/selector-specificity');

The false positive reported by the issue appears still on the demo, but I doubt if it's a false positive. 🤔
Because it seems @csstools/selector-specificity properly calculates specificities as below:

const parser = require('postcss-selector-parser');
const { selectorSpecificity } = require('@csstools/selector-specificity');

console.log(selectorSpecificity(parser().astSync('.componentName :global(.ant-modal) :global(.ant-btn)')));
//=> { a: 0, b: 3, c: 0 }

console.log(selectorSpecificity(parser().astSync('.componentName :global(.ant-menu)')));
//=> { a: 0, b: 2, c: 0 }

See also the @csstools/selector-specificity implementation.

What do you think?

@jeddy3
Copy link
Member

jeddy3 commented Jun 20, 2022

The false positive reported by the issue appears still on the demo, but I doubt if it's a false positive. 🤔

I agree. Let's close.

@jeddy3 jeddy3 closed this as completed Jun 20, 2022
@richex-cn
Copy link

richex-cn commented Jul 7, 2022

Same error for me, How should fix it ?

@ybiquitous
Copy link
Member

@richex-cn Could you reproduce an error on the demo and paste the demo URL here, please?

@richex-cn
Copy link

@ybiquitous Sure, It's demo.

@ybiquitous
Copy link
Member

@richex-cn Thanks. I'll look into it. 👍🏼

@ybiquitous ybiquitous reopened this Jul 7, 2022
@ybiquitous ybiquitous added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Jul 7, 2022
@ybiquitous ybiquitous self-assigned this Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone syntax: postcss relates PostCSS language extensions type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

6 participants