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 styled-components in no-descending-specificity #3862

Closed
hudochenkov opened this issue Dec 22, 2018 · 9 comments · Fixed by #3875
Closed

Fix false positives for styled-components in no-descending-specificity #3862

hudochenkov opened this issue Dec 22, 2018 · 9 comments · Fixed by #3875
Labels
status: ready to implement is ready to be worked on by someone syntax: css-in-js relates to JS objects & template literals type: bug a problem with a feature or rule

Comments

@hudochenkov
Copy link
Member

Clearly describe the bug

False positives for no-descending-specificity in Styled Components. The rule looks into two different components which is not correct. Every styled component should be treated as a separate file if we speak in CSS terms. Link and LinkIcon are two different components, and stylelint can't know if their rules collapse or not.

This is most likely has to do with how postcss-styled parses a file.

Test repository: https://github.com/hudochenkov/styled-component-stylelint-bugs/tree/master/false-positive-no-descending-specificity

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

no-descending-specificity

What CSS is needed to reproduce the bug?

import styled from 'styled-components';
import { NavLink } from 'react-router-dom';

export const Link = styled(NavLink)`
  font-size: 15px;
  font-weight: 700;

  &:hover,
  &.active {
    color: red;

    svg {
      fill: red;
    }
  }
`;

export const LinkIcon = styled.div`
  flex: none;

  svg {
    width: 15px;
    height: 15px;
  }
`;

What stylelint configuration is needed to reproduce the bug?

{
    "extends": ["stylelint-config-recommended", "stylelint-config-styled-components"]
}

Which version of stylelint are you using?

9.9.0

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

CLI with stylelint "**/*.styled.js" --syntax styled

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

Yes, it's related to styled components.

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:

Sidebar.styled.js
 21:3  ✖  Expected selector "svg" to come before selector "&:hover svg"    no-descending-specificity
 21:3  ✖  Expected selector "svg" to come before selector "&.active svg"   no-descending-specificity
@hudochenkov hudochenkov added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule syntax: css-in-js relates to JS objects & template literals labels Dec 22, 2018
@jeddy3
Copy link
Member

jeddy3 commented Dec 27, 2018

Yes, I agree.

@jeddy3 jeddy3 changed the title False positives for no-descending-specificity in Styled Components Fix false positives for styled-components in no-descending-specificity Dec 27, 2018
@cdauth
Copy link

cdauth commented Feb 26, 2019

As far as I can see, the fix for this should be contained in v9.10.1? I'm still experiencing this problem with that version. I'm using stylelint with --syntax scss (--syntax styled doesn't seem to be documented anywhere and doesn't seem to find any errors for me).

@alexander-akait
Copy link
Member

@cdauth don't use syntax, we automatically detect syntax, it is very rare use case when you need set syntax

@cdauth
Copy link

cdauth commented Feb 26, 2019

@evilebottnawi I don't know why everyone keeps saying that. When I used it without, it was reporting an error on every inline comment, until I specified --syntax scss as recommended here: styled-components/stylelint-processor-styled-components#195

@jeddy3
Copy link
Member

jeddy3 commented Feb 27, 2019

I don't know why everyone keeps saying that.

I think there is confusion because the function of the --syntax option has changed over time.

stylelint now supports two ways of extracting styles from containers (like template literals or <style> tags):

The latter came first, but it is being replaced with the former because stylelint processors don't support autofix.

When a processor isn't used, the --syntax option is usually unnecessary because stylelint will autodetect what PostCSS syntax is needed to:

  • parse a language like SCSS directly
  • extract CSS from a container like <style> or JavaScript template literals

However, it's a different story with stylelint processors. The --syntax option sets the parser to use after the processor has extracted the styles from the container.

What is also muddying the water is that styled-components doesn't use SCSS. The docs say it uses a processor called stylis, which is an SCSS-like preprepressor that supports inline comments.

The default PostCSS parser supports everything stylis does except inline comments, which is probably why this issue is only being detected now. It only appears when inline comments are used.

it was reporting an error on every inline comment

Because styled-components uses stylis behind the scenes, I think the parser stylelint uses to extract the template literal styles from JavaScript files (postcss-styled) should support inline comments. I've raised an issue upstream about this.

As far as I can see, the fix for this should be contained in v9.10.1?

I believe this bug was fixed if you use the built-in support for styled-components. It might not be fixed if you use the processor. Due to the way processors and PostCSS syntax differ, it might not be possible to fix this when using the processors.


Long story short, it's all a bit confusing sorry.

I believe processors will be replaced by the PostCSS syntaxes going forward, so you might want to try your luck not using the processor. At the moment it doesn't look like the built-in template literal support can parse inline comments, so you'll need to use standard CSS comment (/* */) for now. This could be a good thing, as it feels like CSS-like-languages-in-JS is perhaps a step too far when CSS-in-JS should do everything you need.

@fabb
Copy link

fabb commented Mar 19, 2019

I get a similar warning when I use > & and nested &:last-child, even when I use "syntax": "scss":

Screenshot 2019-03-19 at 07 34 23

Expected selector ".selector2110 > *" to come before selector ".selector2109 > *:last-child" (no-descending-specificity)stylelint(no-descending-specificity)

@the-J
Copy link

the-J commented Apr 16, 2019

@fabb did you find any walk around? I have same issue in project except for a simple, small, tiny fact that it occurs only on windows machines.

@fabb
Copy link

fabb commented Apr 16, 2019

Unfortunately not, apart from putting the components to different files, or deactivating linting for that line.

@the-J
Copy link

the-J commented Apr 16, 2019

Uh, thanks, I'll try to look into it.

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 syntax: css-in-js relates to JS objects & template literals type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

6 participants