hudochenkov / stylelint-order

A plugin pack of order related linting rules for Stylelint.
MIT License
916 stars 61 forks source link

CSS not linted inside a Styled Component `css` tag #93

Closed caribou-code closed 5 years ago

caribou-code commented 5 years ago

Description

stylelint-order ignores CSS for Styled Components when in a css template literal wrapper.

Example

.stylelintrc

{
  "processors": [
    "stylelint-processor-styled-components"
  ],
  "extends": [
    "stylelint-config-standard",
    "stylelint-config-styled-components",
    "stylelint-config-rational-order"
  ]
}

components/Button/index.style.ts

import styled, { css } from 'styled-components';

interface ButtonStyledProps {
  isDisabled?: boolean;
  isLoading?: boolean;
  variant?: 'dark' | 'light';
}

export const ButtonStyled = styled.button<ButtonStyledProps>`
  ${p => css`
    padding: 0 ${p.theme.button.padding};
    color: ${p.variant === 'dark' && p.theme.button.colorDark};
    color: ${p.variant === 'light' && p.theme.button.colorLight};
    color: ${p.isLoading && 'transparent'};
    font-weight: ${p.theme.button.fontWeight};
    font-size: ${p.theme.button.fontSize};
    font-family: ${p.theme.button.fontFamily};
    line-height: ${p.theme.button.height};
    text-transform: ${p.theme.button.textTransform};
    border-radius: ${p.theme.button.borderRadius};
    cursor: ${(p.isLoading || p.isDisabled) && 'default'};
    opacity: ${p.isDisabled && 0.2};
    pointer-events: ${(p.isLoading || p.isDisabled) && 'none'};
    height: ${p.theme.button.height};
  `}
`;

When props are accessed so many times in a styled component, it makes sense to use the wrapping css tag so only 1 function is created. Everything inside there is valid CSS so should be linted, but stylelint-order is not correctly flagging the height property as being in the wrong place. This bug may be a general stylelint bug because other non ordering rules are also not linting styles inside the css tag.

hudochenkov commented 5 years ago

What output do you get?

Do other stylelint rules work? I. e. declaration-empty-line-before: "always"?

It might be the case that stylelint just don't parse your file, because you're using TypeScript and stylelint doesn't support it.

caribou-code commented 5 years ago

Stylelint and the property ordering works fine if I don't use the css tag wrapper e.g. if I use inline props like so:

export const ButtonStyled = styled.button<ButtonStyledProps>`
    padding: 0 ${p => p.theme.button.padding};
    color: ${p => p.variant === 'dark' && p.theme.button.colorDark};
    font-weight: ${p => p.theme.button.fontWeight};
`;

There is a note in the README that says:

CSS-in-JS styles with template interpolation could be ignored by autofixing to avoid styles corruption.

Not sure if that has anything to do with it?

hudochenkov commented 5 years ago

I'll take a look at the problem within the next week or two.

caribou-code commented 5 years ago

Thanks. I'm starting to think the issue is with stylelint rather than this specific ordering package.

hudochenkov commented 5 years ago

Could you create a repo with reproducible example, please? I can't reproduce the problem.

Evalon commented 5 years ago

It's will be fixed by: https://github.com/gucong3000/postcss-jsx/pull/58

caribou-code commented 5 years ago

@Evalon Maybe I'm misunderstanding something but I don't see how that PR will fix this problem...

@hudochenkov I'm happy to create an example, but thinking I should maybe create an issue on the main stylelint repo instead, since I think it's all linting that doesn't work in this scenario, not just ordering

Evalon commented 5 years ago

@caribou-code This helps greatly if you create reproduction repository. Also, if you use latest version of stylelint you don't need "stylelint-processor-styled-components" because styled-components supported out of the box or maybe some features missing?

caribou-code commented 5 years ago

@hudochenkov @Evalon Sorry for the delay. I've created a public repo with the most basic reproduction of the issue I can do: https://github.com/caribou-code/stylelint-test

Clone it, npm install then run npm run stylelint.

There are 3 styled components in test/index.style.ts. All have the same properties with display: block in the wrong order. Stylelint correctly flags the first 2 as being in the wrong order but not the 3rd. The 3rd one is potentially a technique that could be used to write a substantial amount of CSS.

hudochenkov commented 5 years ago

As soon as stylelint-processor-styled-components disabled everything works. Stylelint supports CSS-in-JS out of the box, no need for this processor.

caribou-code commented 5 years ago

Right you are! I've discovered a completely different bug with the main stylelint package as a result of this, but this specific issue can be closed.