styled-components / stylelint-processor-styled-components

Lint your styled components with stylelint!
https://styled-components.com
MIT License
656 stars 61 forks source link

Support styled-components-breakpoint #182

Open daviddelusenet opened 6 years ago

daviddelusenet commented 6 years ago

I just installed styled-components-breakpoint inside of my project and noticed the following: CSS inside of breakpoint declarations doesn't get linted.

This is my code:

screen shot 2018-05-14 at 14 06 11 ]

When I run Stylelint inside of my terminal I get the following output:

src/app/components/ProductOverview/ProductOverview.sc.js
  7:3  ✖  Expected "padding" to come before "flex-wrap"   order/properties-order
 12:3  ✖  Expected "margin" to come before "padding"      order/properties-order
 14:3  ✖  Expected "width" to come before "flex"          order/properties-order

So all of the code inside of the ${breakpoint('medium')``} statements gets ignored. Can this be solved?

Thanks in advance!

chinesedfan commented 6 years ago

Sorry, now all interpolations will be treated as black boxes during the linting. Because the processor is a static code analyzer, which can't know meanings of interpolations.

daviddelusenet commented 6 years ago

@chinesedfan what do you mean with 'treated as black boxes'?

chinesedfan commented 6 years ago

@daviddelusenet That means the processor doesn't know contents of interpolations. So ${breakpoint('xx')/* css */} is like /* black boxes */.

daviddelusenet commented 6 years ago

Ah ok, thanks for your explanation!

However, don't you think stuff like this needs to be supported? As the popularity and usage of Styled Components grows a lot more people will start using tools like styled-components-breakpoint. Not knowing the contents of interpolations means that literally no tool EVER will be supported.

I don't think this is the way the go since you'll be basically killing this processor. Maybe it's a solution to implement something like this (based on how Eslint handles inline configuration):

export const ProductPreviewWrapper = styled.div`
  flex: 0 0 auto;
  margin: 0 0 70px;
  padding: 0 20px;
  width: 100%;

  /* stylelint start-linting */
  ${breakpoint('medium')`
    width: 50%;
    margin: 0 0 90px;
  `}

  ${breakpoint('large')`
    width: 33.333333333%;
  `}

  ${breakpoint('xxlarge')`
    width: 25%;
  `}
  /* stylelint end-linting */
`;

Quite simple: you add some comments arounds your interpolations which don't get linted by default. If Stylelint encounters these comments it will just assume that everything in between these comments and in interpolations is CSS and will lint it.

What do you think about this solution?

emilgoldsmith commented 6 years ago

@daviddelusenet it's not by choice that they are black boxes, it's because interpolations are JS, and we only do static code analysis, which I'm pretty sure is also what ESlint does, unless we evaluate the javascript (which very often would include evaluating imported variables from other libraries and files etc. which really quickly becomes close to an impossible task) we don't know what to lint. What a processor does is basically take a file, extract the CSS and hand the CSS to Stylelint that then does the actual linting (and we give it sourcemaps so it can give errors on the correct lines). If you can come up with something brilliant to work around this, I'd love to hear it, but it's not as simple as you think

fabb commented 5 years ago

It's way easier to just define the breakpoint medias as strings rather than template functions:

export const theme = {
    media: {
        medium: '@media only screen and (max-width: 767px)',
        large: '@media only screen and (min-width: 768px)',
        xxlarge: '@media only screen and (min-width: 970px)',
    },
}

export const ProductPreviewWrapper = styled.div`
  flex: 0 0 auto;
  margin: 0 0 70px;
  padding: 0 20px;
  width: 100%;

  ${props => props.theme.media.medium} {
    width: 50%;
    margin: 0 0 90px;
  }

  ${props => props.theme.media.large} {
    width: 33.333333333%;
  }

  ${props => props.theme.media.medium} {
    width: 25%;
  }
`;

This lints correctly, and also syntax highlighting in VSCode works as expected.