hudochenkov / postcss-sorting

PostCSS plugin to keep rules and at-rules content in order.
MIT License
517 stars 31 forks source link

Alphabetical Sorting Order Non-Stable and Broken for some properties #68

Closed dwighthouse closed 7 years ago

dwighthouse commented 7 years ago

Using postcss-sorting 3.0.0, the most recent version.

I have found a case where properties are sorted into a non-sensical order when using the option 'properties-order': 'alphabetical'. Sorting causes some vendor-prefixed properties to sort below non-prefixed variations. There are other issues with the algorithm incorrectly sorting vendor-prefixed properties when the non-prefixed string doesn't match the string created by removing the vendor prefix. Finally, I have also found that non-significant changes to the input can result in significant changes to the sorted output, which makes me believe the sorting algorithm is inherently non-stable.

Example case, tests, and full details here: https://github.com/dwighthouse/postcss-sorting-flex-issue

hudochenkov commented 7 years ago

Thank you for detailed explanation and providing test repo!

PostCSS Sorting intended to use on source files, not on output files. So developer has predictable order in her style sheets.

Finally, I have also found that non-significant changes to the input can result in significant changes to the sorted output, which makes me believe the sorting algorithm is inherently non-stable.

This is unacceptable. I'll dig into the problem.

  1. The align-items property should be sorted below both the -webkit-box-align and -ms-flex-align properties.

Technically, no. Because if you remove prefixes, align-items fill be first. postcss-sorting doesn't know about different flexbox spec implementations. It only knows about standard way of prefixing. I'm afraid this problem won't be addressed.

  1. The flex-wrap property should be sorted below the -ms-flex-wrap property.

Yes. This is weird, that it doesn't happens some times.

  1. The entry for display: flex; should be below both display: -webkit-box; and display: -ms-flexbox;.

Yes. But only if they was in the same way in input file. Properties with same name should preserve the same order as in input.


I'll fix inconsistency problem. Which leads to issues 2. and 3..

Overall, I suggest you to use postcss-sorting on source files. Or at least run it before cssnext. autoprefixer inside cssnext will apply flexbox prefixes, but postcss-sorting wouldn't know about them.

dwighthouse commented 7 years ago

Technically, no. Because if you remove prefixes, align-items fill be first. postcss-sorting doesn't know about different flexbox spec implementations. It only knows about standard way of prefixing. I'm afraid this problem won't be addressed.

I disagree with the reasoning here. There is no "standard way of prefixing". By definition, prefixing is working with pre-release modes that will change in the future. There is no reason to expect that the property names of the prefixed variations to be textually equivalent (or functionally equivalent for that matter) to the unprefixed versions, as my case clearly demonstrates. If you won't make the prefix handling aware of the similar prefixed properties, at least put a warning somewhere that to that effect. Or recommend (in the docs) that users sort their properties before adding prefixed versions. However, if one is using browser specific values for prefixed variations (to address browser-differing behavior, for example) that are different from the unprefixed version, this solution won't work.

Thank you for looking into the other issues. I tried to go through the algorithm myself yesterday, but I didn't see anything that looked clearly wrong.

hudochenkov commented 7 years ago

I've fixed inconsistent sorting. Please, upgrade to 3.0.1.

I'll add description about prefixed properties, which have completely different name, later.

hudochenkov commented 7 years ago

Description added in https://github.com/hudochenkov/postcss-sorting/commit/96810aaa346896674614052ba14b887f22872f80.