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

Properties-alphabetical-order mixin error within styled-components #63

Closed
ekfuhrmann opened this issue Mar 26, 2018 · 33 comments
Closed

Comments

@ekfuhrmann
Copy link

ekfuhrmann commented Mar 26, 2018

With the following component:

const Foo = styled.p`
   margin: 0;
   text-transform: uppercase;

  ${font("sans-serif", 14, 600, #000)};
`;

I receive this error:

Expected -styled-mixin0 to come before text-transform (order/properties-alphabetical-order)

The mixin I have is simply creating some font style properties based off of the parameters. It seems like the alphabetical ordering is using the word mixin and thus wants it placed above text-transform but after margin.

@hudochenkov
Copy link
Owner

Sorry for a late response.

This plugin doesn't know anything about styled components. From what I see in your report, you're probably using stylelint-processor-styled-components. Please, refer to this issue styled-components/stylelint-processor-styled-components#117 for solution.

@ostenning
Copy link

ostenning commented Jul 2, 2018

I'm facing the same issue. Would it make sense to add an option to ignore the sorting when ${font('sans-serif', 14, 600, #000)}; is detected? E.g. ignore ${ or something similar?

Imo it would be handy

@nickmccurdy
Copy link

Can we please reopen this issue for a workaround, since stylelint-processor-styled-components can't be changed to improve this?

I think the best we can do is automatically ignore rules starting with -styled-.

@hudochenkov
Copy link
Owner

@nickmccurdy stylelint supports CSS-in-JS out of the box now. Have you tried it without using stylelint-processor-styled-components?

@nickmccurdy
Copy link

I removed "processors": "stylelint-processor-styled-components", and my linter errors are no longer working on Stylelint 9.6.0

@taylon
Copy link

taylon commented Nov 27, 2018

It is weird that every Stylelint rule I tested so far works in CSS-in-JS (including ---fix) out of the box. Except for stylelint-order. The order rules don't report any errors in CSS-in-JS.

@hudochenkov
Copy link
Owner

@taylon please, create a separate issue with a description of your problem.

@markcellus
Copy link

markcellus commented Apr 27, 2019

Definitely having a similar problem as @nickmccurdy here. I realize this may not be a direct issue in this repository but it does seem odd that this lib can't ignore styled-component mixins. See code below:

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

export const buttonReset = css`
    background: none;
    border-bottom: 0;
    font: inherit;
    padding: 0;
`;

export const StyledButton = styled.button`
    ${buttonReset}
    border: 1px solid red;
}

And I get this in the console:

Expected border to come before -styled-mixin0   order/properties-alphabetical-order

Looks like its expecting the mixin variable ${buttonReset} to come after the border property, but I need for the mixin variable to be first in order to override it.

@markcellus
Copy link

sorry I meant that issue is the same one that @ekfuhrmann originally had. Can we reopen this issue and explore a possible fix?

@hudochenkov
Copy link
Owner

stylelint-order works great with Styled Components. I use it every day. Don't use stylelint-processor-styled-components and you won't see any warnings like above.

@markcellus
Copy link

markcellus commented Apr 28, 2019

Sure, but as @nickmccurdy has already pointed out, stylelint-processor-styled-components can't be removed because linting stops working altogether. Using latest version of stylelint: 10.0.1.

Also, just to clarify. These are not warnings, they're actual errors that fail the lint tests.

@hudochenkov
Copy link
Owner

hudochenkov commented Apr 28, 2019

These are not warnings, they're actual errors that fail the lint tests.

Please, create an issue in stylelint if this happens. Help us fix these errors.

I have no intention to support stylelint-processor-styled-components as this plugin work without any stylelint processors.

@markcellus
Copy link

😕 This is the same error that was originally brought up in the thread. Why not just reopen this?

@markcellus
Copy link

markcellus commented Apr 28, 2019

I have no intention to support stylelint-processor-styled-components as this plugin work without any stylelint processors.

Hmm, don't think anyone is asking for you to support stylelint-processor-styled-components. We're asking that you support use of ${} expressions and ignore them. You don't technically need stylelint-processor-styled-components to use ${} expressions in your css-in-js.

@hudochenkov
Copy link
Owner

The original error is caused by using stylelint-processor-styled-components. It won't be fixed.

I asked you to not use this processor, and you said that there are other errors. Please, report these errors to stylelint, so we can fix it.

@markcellus
Copy link

markcellus commented Apr 28, 2019

The original error is caused by using stylelint-processor-styled-components. It won't be fixed.

See my previous comment. It's not caused by the use of stylelint-processor-styled-components. This is an issue caused by using ${} expressions in your styled-components when using this plugin.

I asked you to not use this processor, and you said that there are other errors.

😕 never said there are any other errors than those that are already mentioned here.

@hudochenkov
Copy link
Owner

I think there is misunderstanding from both sides.

Warning like this:

Expected -styled-mixin0 to come before text-transform (order/properties-alphabetical-order)

Is definitely caused by stylelint-processor-styled-components because -styled-mixin0 created from this processor. It's not from stylelint itself.

Try to remove stylelint-processor-styled-components from your stylelint config and see what happens. It should work as expected. I just tried your example and didn't get any warnings, because order is correct. Then I changed order in the first component and got warnings, because order was incorrect.

@markcellus
Copy link

Then I changed order in the first component and got warnings, because order was incorrect.

This will be my last comment about this since I'm pretty busy. Your statement above is incorrect. It looks like everything is fine because stylelint stops discovering other errors altogether and no longer works properly. Use my example again, remove stylelint-processor-styled-components, and put some other error there (maybe like duplicate selector) and you will see that that won't produce any error also.

@drwpow
Copy link

drwpow commented Jun 18, 2019

I have a dumb hack for this, that doesn’t really fix the problem but at least suppresses the error in many scenarios. This works because the list from css-known-properties out-of-the-box comes alphabetized with vendor prefixes, as you’d expect.

// .stylelintrc.js

const properties = require('known-css-properties').all;

module.exports = {
  processors: ['stylelint-processor-styled-components'],
  plugins: ['stylelint-order'],
  rules: {
    'order/properties-order': [...properties, '-styled-mixin0'], // alphabetize, but put Styled Components last

The real problem is that certain properties used by Styled Components, et al shouldn’t be alphabetized at all—they either need to come first or last depending on the situation. Anyway, what I’d like to see with this plugin would be an ignore option that would let you whitelist things like -styled-mixin0, and in the sorting algorithm would just leave this as-is wherever it’s found.

I could open a PR for this if this is desirable to others.

@hudochenkov
Copy link
Owner

@DangoDev why stylelint-processor-styled-components is used? stylelint supports styled components out of the box without the need for processor.

@drwpow
Copy link

drwpow commented Jun 18, 2019

@hudochenkov I’m sorry—I’ve read this entire thread, and I know you keep saying that, and feel like you’re being ignored. But that doesn’t seem to be the case. As of stylelint 10.1.0 and Styled Components 5.0.0-beta.3 it does not work without the processor. This is what I see:

Screen Shot 2019-06-18 at 17 10 58

Expected empty line before declaration (declaration-empty-line-before)stylelint(declaration-empty-line-before)

Not only does that error appear in every Styled Components block, but without the processor, all the other lint rules stop working (such as alphabetization). That’s trying to run it without stylelint-processor-styled-components.

What’s more, the Styled Components documentation still recommends using the processor, and the repo is still actively maintained. It’s hard to see how to use Styled Components in Stylelint without this.

@hudochenkov
Copy link
Owner

@DangoDev would you mind opening an issue in stylelint repository about declaration-empty-line-before issue, please? It's either bug or incorrect config.

If you have examples of other core rules, please, open issues in stylelint repository as well. Help us make stylelint better :)

@markcellus
Copy link

@DangoDev either he's just not getting it or all of us are just doing a very very bad job explaining this lol.

@hudochenkov
Copy link
Owner

I'm getting it. styled-components processor were introduced long before stylelint was supporting CSS-in-JS. Now there are less reasons to use processor. There are some problems which developers of stylelint don't know about, because users don't report them.

What I see from this thread, that users facing problems with built-in support, so they choose quick solution — use processor, and users happy unless process backfires sometimes (like this issue). It's a workaround to a problem and a short term solution. I'm asking to invest a little bit of time to long term solution — report issues you're encountering to stylelint repository. Help us to help you and make better tool for everyone.

@drwpow
Copy link

drwpow commented Jun 21, 2019

What confused us was the messaging here. I see issues like this one that are working toward deprecating processors. And I’m all for it! Love the idea of Stylelint supporting CSS-in-JS natively 🎉!

We can (and will) open up tickets, for sure. But when you say “it supports,“ that’s not accurate because for (arguably) the most popular CSS-in-JS library, Styled Components, it is clearly and obviously broken. It is lacking the most basic of support. Let’s just level with the fact that it doesn’t work, and the official Styled Components documentation still recommending you to use the processor agrees with this.

Saying “it works now” isn’t correct. But saying “we’re moving toward deprecating processors, and want to be able to support Styled Components without it” is awesome! 💯 All in favor of working for that solution. Hopefully that clarifies the disconnect here.

@hudochenkov
Copy link
Owner

For a note: I use styled components for half a year now with stylelint and autofixing. I use stylelint-config-recommended and order/order and order/properties-order with autofixing.

That where from some of my confusion comes. I use both and don't have problems (almost :)).

@hudochenkov
Copy link
Owner

While fixing everything in stylelint could take a while, users want to continue using processor in the meanwhile.

Consider contributing a fix to ignore -styled-* things.

@ekfuhrmann
Copy link
Author

@hudochenkov would you mind sharing your stylelint config that you use for React?

@hudochenkov
Copy link
Owner

@ekfuhrmann I use two configs. One default, which is also picked up by editor plugin — .stylelintrc.js. And the second which extends default config and add more things that executed on precommit hook — .stylelintrc-fix.js.

stylelint "src/**/*.styled.js" --config .stylelint-fix.rc.js
// .stylelintrc.js
module.exports = {
	extends: ['stylelint-config-recommended'],
	plugins: ['stylelint-order'],
	rules: {
		'no-empty-source': null,
		'order/order': [['declarations', 'rules']],
	},
};
.stylelintrc-fix.js (click to expand)
// .stylelintrc-fix.js
module.exports = {
	extends: './.stylelintrc.js',
	rules: {
		'order/properties-order': [
			[
				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'content',
						'position',
						'top',
						'right',
						'bottom',
						'left',
						'z-index',
						'display',
						'vertical-align',
						'visibility',
						'flex',
						'flex-grow',
						'flex-shrink',
						'flex-basis',
						'flex-direction',
						'flex-flow',
						'flex-wrap',
						'grid',
						'grid-area',
						'grid-template',
						'grid-template-areas',
						'grid-template-rows',
						'grid-template-columns',
						'grid-row',
						'grid-row-start',
						'grid-row-end',
						'grid-column',
						'grid-column-start',
						'grid-column-end',
						'grid-auto-rows',
						'grid-auto-columns',
						'grid-auto-flow',
						'grid-gap',
						'grid-row-gap',
						'grid-column-gap',
						'align-content',
						'align-items',
						'align-self',
						'justify-content',
						'order',
						'float',
						'clear',
						'overflow',
						'overflow-x',
						'overflow-y',
						'overflow-scrolling',
						'clip',
						'box-sizing',
					],
				},

				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'width',
						'min-width',
						'max-width',
						'height',
						'min-height',
						'max-height',
						'margin',
						'margin-top',
						'margin-right',
						'margin-bottom',
						'margin-left',
						'padding',
						'padding-top',
						'padding-right',
						'padding-bottom',
						'padding-left',
						'border',
						'border-spacing',
						'border-collapse',
						'border-width',
						'border-style',
						'border-color',
						'border-top',
						'border-top-width',
						'border-top-style',
						'border-top-color',
						'border-right',
						'border-right-width',
						'border-right-style',
						'border-right-color',
						'border-bottom',
						'border-bottom-width',
						'border-bottom-style',
						'border-bottom-color',
						'border-left',
						'border-left-width',
						'border-left-style',
						'border-left-color',
						'border-radius',
						'border-top-left-radius',
						'border-top-right-radius',
						'border-bottom-right-radius',
						'border-bottom-left-radius',
						'border-image',
						'border-image-source',
						'border-image-slice',
						'border-image-width',
						'border-image-outset',
						'border-image-repeat',
						'border-top-image',
						'border-right-image',
						'border-bottom-image',
						'border-left-image',
						'border-corner-image',
						'border-top-left-image',
						'border-top-right-image',
						'border-bottom-right-image',
						'border-bottom-left-image',
					],
				},

				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'background',
						'background-color',
						'background-image',
						'background-attachment',
						'background-position',
						'background-position-x',
						'background-position-y',
						'background-clip',
						'background-origin',
						'background-size',
						'background-repeat',
						'color',
						'box-decoration-break',
						'box-shadow',
						'outline',
						'outline-width',
						'outline-style',
						'outline-color',
						'outline-offset',
						'table-layout',
						'caption-side',
						'empty-cells',
						'list-style',
						'list-style-position',
						'list-style-type',
						'list-style-image',
					],
				},

				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'font',
						'font-weight',
						'font-style',
						'font-variant',
						'font-size-adjust',
						'font-stretch',
						'font-size',
						'font-family',
						'src',
						'line-height',
						'letter-spacing',
						'quotes',
						'counter-increment',
						'counter-reset',
						'-ms-writing-mode',
						'text-align',
						'text-align-last',
						'text-decoration',
						'text-emphasis',
						'text-emphasis-position',
						'text-emphasis-style',
						'text-emphasis-color',
						'text-indent',
						'text-justify',
						'text-outline',
						'text-transform',
						'text-wrap',
						'text-overflow',
						'text-overflow-ellipsis',
						'text-overflow-mode',
						'text-shadow',
						'white-space',
						'word-spacing',
						'word-wrap',
						'word-break',
						'overflow-wrap',
						'tab-size',
						'hyphens',
						'interpolation-mode',
					],
				},

				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'opacity',
						'filter',
						'resize',
						'cursor',
						'pointer-events',
						'user-select',
					],
				},

				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'unicode-bidi',
						'direction',
						'columns',
						'column-span',
						'column-width',
						'column-count',
						'column-fill',
						'column-gap',
						'column-rule',
						'column-rule-width',
						'column-rule-style',
						'column-rule-color',
						'break-before',
						'break-inside',
						'break-after',
						'page-break-before',
						'page-break-inside',
						'page-break-after',
						'orphans',
						'widows',
						'zoom',
						'max-zoom',
						'min-zoom',
						'user-zoom',
						'orientation',
						'fill',
						'stroke',
					],
				},

				{
					emptyLineBefore: 'always',
					noEmptyLineBetween: true,
					properties: [
						'transition',
						'transition-delay',
						'transition-timing-function',
						'transition-duration',
						'transition-property',
						'transform',
						'transform-origin',
						'animation',
						'animation-name',
						'animation-duration',
						'animation-play-state',
						'animation-timing-function',
						'animation-delay',
						'animation-iteration-count',
						'animation-direction',
						'animation-fill-mode',
					],
				},
			],
			{
				unspecified: 'bottom',
			},
		],
	},
};

@mulholo
Copy link

mulholo commented Nov 19, 2019

Has anyone got a hack/solution for globally ignoring errors which contain -styled-x? I don't want to do an ignore comment every time.

e.g.

[stylelint order/properties-alphabetical-order] [E] Expected -styled-mixin1 to come before transform (order/properties-alphabetical-order)

@saiichihashimoto
Copy link

It's been close to 2 years since @hudochenkov last commented on this. Is there a way we can move forward? Currently, using styled components and using this rule are mutually exclusive.

@ghost
Copy link

ghost commented Aug 14, 2021

I had the same issue, disabling "stylelint-processor-styled-components" make this plugins works but in other hand indentation rule starting to had inconsistent behavior, with "stylelint-processor-styled-components" order/properties-alphabetical-order doesn't had the issue related with mixing, so in the end I prefer have the indentation working properly and I use https://github.com/tinloof/eslint-plugin-better-styled-components to get alphabetical-order rule until the indentation be fixed without require the processor

@hudochenkov
Copy link
Owner

Closing as stylelint-processor-styled-components was deprecated. Use postcss-styled-syntax custom syntax instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants