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

group.forEach is not a function #32

Closed
gajus opened this issue Jul 2, 2016 · 8 comments
Closed

group.forEach is not a function #32

gajus opened this issue Jul 2, 2016 · 8 comments

Comments

@gajus
Copy link

gajus commented Jul 2, 2016

Trying to format this CSS:

.upload-requirements {
    margin: 10px;
}

Using https://github.com/gajus/stylelint-config-canonical (v1.0.2) config, gives an error:

TypeError: group.forEach is not a function
    at /Users/gajus/Documents/dev/applaudience/static-upload-app/static-upload-app-web/src/client/node_modules/postcss-sorting/index.js:44:10
    at Array.forEach (native)
    at getSortOrderFromOptions (/Users/gajus/Documents/dev/applaudience/static-upload-app/static-upload-app-web/src/client/node_modules/postcss-sorting/index.js:43:4)
    at /Users/gajus/Documents/dev/applaudience/static-upload-app/static-upload-app-web/src/client/node_modules/postcss-sorting/index.js:231:15
    at formatOrder (/Users/gajus/Documents/dev/applaudience/static-upload-app/static-upload-app-web/src/client/node_modules/stylefmt/lib/formatOrder.js:13:3)
    at /Users/gajus/Documents/dev/applaudience/static-upload-app/static-upload-app-web/src/client/node_modules/stylefmt/index.js:19:7

Upon inspection, sortOrder is:

[{"properties":["composes"]},{"properties":["font","font-family","font-size","font-weight","font-style","font-variant","font-size-adjust","font-stretch","font-effect","font-emphasize","font-emphasize-position","font-emphasize-style","font-smooth","line-height"]},{"properties":["position","z-index","top","right","bottom","left"]},{"properties":["display","visibility","float","clear","overflow","overflow-x","overflow-y","clip","zoom","flex-direction","flex-order","flex-pack","flex-align"]},{"properties":["box-sizing","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"]},{"properties":["table-layout","empty-cells","caption-side","border-spacing","border-collapse","list-style","list-style-position","list-style-type","list-style-image"]},{"properties":["content","quotes","counter-reset","counter-increment","resize","cursor","user-select","nav-index","nav-up","nav-right","nav-down","nav-left","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","text-align","text-align-last","vertical-align","white-space","text-decoration","text-emphasis","text-emphasis-color","text-emphasis-style","text-emphasis-position","text-indent","text-justify","letter-spacing","word-spacing","text-outline","text-transform","text-wrap","text-overflow","text-overflow-ellipsis","text-overflow-mode","word-wrap","word-break","tab-size","hyphens","pointer-events"]},{"properties":["opacity","color","border","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","outline","outline-width","outline-style","outline-color","outline-offset","background","background-color","background-image","background-repeat","background-attachment","background-position","background-position-x","background-position-y","background-clip","background-origin","background-size","box-decoration-break","box-shadow","text-shadow"]}]

This does not look like an expected format.

Regardless, the input format meeds the schema defined in https://github.com/stylelint/stylelint/blob/master/src/rules/declaration-block-properties-order/README.md

@hudochenkov
Copy link
Owner

Stylelint config is not fully compatible with this plugin yet. gajus/stylelint-config-canonical@2c58f19#diff-33d8a40ae0050661239a87463149dcafR4 array of arrays will be a correct config for PostCSS Sorting.

@gajus
Copy link
Author

gajus commented Jul 2, 2016

@hudochenkov Would you accept a PR that makes this change? or is there a reason this is on hold (since it does not look like a big change)

@hudochenkov
Copy link
Owner

It's a big change because it would broke plugin backwards compatibility. Grouping in this plugin allows to group declarations, nested rules, at-rules. See default config. Stylelint only support grouping for declarations only.

We are working now on a plugin for stylelint which will check sort order for non declarations (nested rules, at-rules). After this plugin will be ready I will think about how to integrate postcss-sorting in stylefmt, tool which format style sheets based on stylelint config. Now stylefmt support stylelint's declaration-block-properties-order, but only with config like this: ["array", "of", "unprefixed", "property", "names"].

@gajus
Copy link
Author

gajus commented Jul 2, 2016

It's a big change because it would broke plugin backwards compatibility.

I disagree. This plugin could support both syntaxes without introducing backwards breaking changes.

@hudochenkov
Copy link
Owner

I'll be glad if you right because it would be a great feature. Feel free to make a PR if it only adds new syntax and won't break anything.

@jeddy3
Copy link

jeddy3 commented Jul 2, 2016

After this plugin will be ready I will think about how to integrate postcss-sorting in stylefmt, tool which format style sheets based on stylelint config.

This is an excellent idea! It looks like stylefmt added support for declaration-block-properties-order yesterday using postcss-sorting. Is there a chicken and egg thing going on here? Either way, it's feels like things are coming together nicely and that your stylelint plugin will be the missing piece of the puzzle :)

@hudochenkov
Copy link
Owner

@jeddy3 Yeah, I found out about this support from PostCSS twitter :) And as I can see, it's very limited support. I'm going to fix that too :) It's won't be a full support, because stylelint have “flexible” order option, and postcss-sorting do strict order only. But I will change stylefmt's part of declaration-block-properties-order to support stylelint's rule as close as possible.

@jeddy3
Copy link

jeddy3 commented Jul 2, 2016

It's won't be a full support, because stylelint have “flexible” order option, and postcss-sorting do strict order only.

That makes sense to me. I'm not sure how many people use the "flexible" order option, but I suspect it's a much smaller number than the combined total of people who use alphabetical or define a strict custom order. If someone does use the "flexible" order option, then they can always PR the support themselves if they want it :)

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

No branches or pull requests

3 participants