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

Add postcss-sorting integration #161

Merged

Conversation

nkt
Copy link
Contributor

@nkt nkt commented Jun 30, 2016

No description provided.

@matype
Copy link
Owner

matype commented Jul 1, 2016

@nkt Thank you for you PR! Good stuff :)

But, I have a question. Are you taking into account using with @include and @extend in SCSS code, or @apply rules?

There is a possibility to change source code destructive by sorting declarations. Please see also #3 (comment) .

Thanks.


function formatOrder(root, params) {
var sortOrder = params.stylelint['declaration-block-properties-order'];
if (!Array.isArray(sortOrder)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@morishitter maybe I should add check for sass right here?

if (params.isSass || !Array.isArray(sortOrder)) {
  return;
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to consider with at-rules e.g. @apply, @include of SCSS and @mixin of postcs-mixins. I think, at-rules in declaration nodes of PostCSS should be on the top of its declaration.

.selector {
  font-size: 12px;
  max-width: 280px;
  @include truncate();
}

The above code will be formatted to the following code.

.selector {
  @include truncate();
  max-width: 280px;
  font-size: 12px;
}

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already supported by postcss-sorting. I've updated tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

postcss-sorting also supports grouping, but this feature not supported by stylelint 😞

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, nice! I wish stylelint will support it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkt nkt force-pushed the feature/declaration-block-properties-order branch from fe27387 to d9c2100 Compare July 1, 2016 15:53
@matype
Copy link
Owner

matype commented Jul 1, 2016

@nkt Thank you so much ;)

@matype matype merged commit 15d6f59 into matype:master Jul 1, 2016
@nkt nkt deleted the feature/declaration-block-properties-order branch July 1, 2016 16:34
@jeddy3
Copy link

jeddy3 commented Jul 1, 2016

This is fantastic! Great job :)

You might be interested in keeping an eye on this proposal for a companion plugin to declaration-block-properties-order called stylelint-block-non-properties-order. The hope is that it'll work alongside declaration-block-properties-order to lint the order of non-property things like nested at-rules, nested rules and variables. It's early days yet, but hopefully something will come of it :)

@jasonlong
Copy link

Thanks @nkt!

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

Successfully merging this pull request may close these issues.

4 participants