feat: disable css-declaration-sorter in default preset

[ludofischer]

Could not detect any size reduction with css-declaration-sorter on,
it introduces bugs in certain conditions and it is hard to upgrade
without breaking Node.js 10 compatibility.

Fix #1054

In the test below with css-size, it looks like css-declaration-sorterincreases the gzip size from 22.8kB to 22.9kB

master:

node dist/cli.js ../../frameworks/bootstrap-v4.2.1.css
┌────────────┬──────────────┬─────────┬─────────┐
│            │ Uncompressed │ Gzip    │ Brotli  │
├────────────┼──────────────┼─────────┼─────────┤
│ Original   │ 204 kB       │ 25.2 kB │ 18.5 kB │
├────────────┼──────────────┼─────────┼─────────┤
│ Processed  │ 152 kB       │ 22.9 kB │ 17 kB   │
├────────────┼──────────────┼─────────┼─────────┤
│ Difference │ 51.8 kB      │ 2.27 kB │ 1.41 kB │
├────────────┼──────────────┼─────────┼─────────┤
│ Percent    │ 74.57%       │ 90.98%  │ 92.37%  │
└────────────┴──────────────┴─────────┴─────────┘

in this PR:

node dist/cli.js ../../frameworks/bootstrap-v4.2.1.css
┌────────────┬──────────────┬─────────┬─────────┐
│            │ Uncompressed │ Gzip    │ Brotli  │
├────────────┼──────────────┼─────────┼─────────┤
│ Original   │ 204 kB       │ 25.2 kB │ 18.5 kB │
├────────────┼──────────────┼─────────┼─────────┤
│ Processed  │ 152 kB       │ 22.8 kB │ 17 kB   │
├────────────┼──────────────┼─────────┼─────────┤
│ Difference │ 51.8 kB      │ 2.33 kB │ 1.46 kB │
├────────────┼──────────────┼─────────┼─────────┤
│ Percent    │ 74.57%       │ 90.76%  │ 92.11%  │
└────────────┴──────────────┴─────────┴─────────┘

[alexander-akait]

hm, will be great to test it on popular frameworks, like bootstrap/tailwind

[ludofischer]

hm, will be great to test it on popular frameworks, like bootstrap/tailwind

Definitely should test more. But from my (possibly wrong) understanding of compression I am not surprised it does not reduce the size.
If the properties are in the same order but the values are different, they will not form an identical string, so sorting does not help gzip. If the values are the same, postcss-merge-rules will probably merge the rules, so there will be no duplicate string anyway.

I guess there's very small number of cases where sorting could improve the size (same declarations with same properties, but rules cannot be merged for some reason).

[alexander-akait]

Make sense, maybe we just set it to false right now, because removing it look like breaking change

[ludofischer]

Make sense, maybe we just set it to false right now, because removing it look like breaking change

I've updated this PRI, disabled in css-declaration-sorter in the default preset instead of removing. But I think this exposes a different bug. I find it's actually pretty strange:

Without css-declaration-sorter,

a{border-top:1px solid;border-color:purple}

turns into

a{border-color:currentcolor purple purple;border-top:1px solid purple}

Might be related to #1044

[Siilwyn]

👋 chiming in, it should reduce the gzip size indeed but I can imagine that it has a negative effect on some stylesheets. I did some testing initially when adding the sorter to cssnano but have no concrete numbers. Would be good to look into, I'll also see if I can find time soon for this.

Regarding the upgrade path:

hard to upgrade without breaking Node.js 10 compatibility

I can backport bug fixes to the 5.x branch, is that something you want?

[ludofischer]

I can backport bug fixes to the 5.x branch, is that something you want?

The current cssnano uses PostCSS 8, so we would need a css-declaration-sorter release with the PostCSS 8 API compatible with Node.js 10. Right now cssnano is using css-declaration-sorter pinned at 6.0. . Ideally, I would like to bring back Node.js 10 support to the current postcss-declaration-sorter 6.0 series so it does not unexpectedly break on people using a semantic version range on Node.js 10. see Node.js 12 is required to use the native stable sort instead of the timsort package so maybe it's also easier for you to add back the timsort dependency rather than backporting fixes, as I see the code layout also changed since 5.0

[ludofischer]

wave chiming in, it should reduce the gzip size indeed but I can imagine that it has a negative effect on some stylesheets. I did some testing initially when adding the sorter to cssnano but have no concrete numbers. Would be good to look into, I'll also see if I can find time soon for this.

The gzip compression results surprise me, because I cannot understand how sorting makes the results slightly worse (I could understand if there was no change at all).
Maybe sorting causes other plugins (like merge-longhand) to generate worse results?
Here's some results of running css-size with some more recent frameworks versions:

Input	Compressed size wiith sorting (as % of original)	Without sorting	Gzip size difference with sorting
tailwind 2.1.2	96.14%	96.37%	-0.7kB
bootstrap 4.2.1	90.98%	90.76%	+0.06kB
basscss 8.0.2	90.36%	90.24%	+0.003kB
bulma 0.7.2	91.8%	91.76%	+0.01kB
tachyons 4.12	63.5%	63.44%	+0.01kB

The only framework where sorting improves the gzipped output is tailwind. I'm also surprised of the drastic improvements that minification makes on tachyons, as I expected tachyons and tailwind to have a similar structure, but apparently that's not the case. Of course most people would not ship the whole of tailwind so in absolute number of kilobytes, the improvements would me smaller for practical situations. In all other cases there is basically no change, it only gets worse by a minuscule amount.

[alexander-akait]

What do you think disable this for default preset and set it for advanced and add Notes in docs - advisable for the developer to check it manually to make sure it is required or not

[Siilwyn]

I do wonder what the effects are on actual CSS bundles instead of framework bundles. 🤔 Could get even complexer with Brotli but not sure about that since it is based on the same principles as gzip afaik.

[alexander-akait]

@Siilwyn another idea - calculate original gzip/brotli size then order them, calculate gzip/brotli size of ordered and return what is best, it may take some time, but it will be safe to use

[ludofischer]

I do wonder what the effects are on actual CSS bundles instead of framework bundles. thinking Could get even complexer with Brotli but not sure about that since it is based on the same principles as gzip afaik.

It would be interesting to check with custom CSS, but I think these days the framework code is a big part of the total CSS and it's also time consuming to scrape enough CSS that's not already minified.

In my measurements, there's no difference with brotli either. One of the easiest methods to check is running this benchmark https://github.com/GoalSmashers/css-minification-benchmark and disable css-declaration-sorter on this line https://github.com/ludofischer/css-minification-benchmark/blob/88ccbd4c875099614b8216957efdde63de6cb763/lib/minify.js#L19 (you can use my fork to run cssnano 5). It's

[Siilwyn]

I can backport bug fixes to the 5.x branch, is that something you want?

The current cssnano uses PostCSS 8, so we would need a css-declaration-sorter release with the PostCSS 8 API compatible with Node.js 10. Right now cssnano is using css-declaration-sorter pinned at 6.0. . Ideally, I would like to bring back Node.js 10 support to the current postcss-declaration-sorter 6.0 series so it does not unexpectedly break on people using a semantic version range on Node.js 10. see Node.js 12 is required to use the native stable sort instead of the timsort package so maybe it's also easier for you to add back the timsort dependency rather than backporting fixes, as I see the code layout also changed since 5.0

Alright, that should be do-able. I remember 6.0.0, it was published by mistake coming from the next version tag. My intention was not to publish v6 with Node.js 10 support but I guess I screwed that up at that moment.

[alexander-akait]

@ludofischer let's puts benchmark here, it is good work, we should not lost it

[Siilwyn]

@Siilwyn another idea - calculate original gzip/brotli size then order them, calculate gzip/brotli size of ordered and return what is best, it may take some time, but it will be safe to use

Sounds like a nice weekend task for me. Where would you put this logic in cssnano or css-declaration-sorter?

It would be interesting to check with custom CSS, but I think these days the framework code is a big part of the total CSS and it's also time consuming to scrape enough CSS that's not already minified.

Definitely, thanks for the investigation good to see! I understand getting unminified CSS is more time consuming, thinking of trying your benchmark out on open source websites like PostCSS and Deno.

[alexander-akait]

Sounds like a nice weekend task for me. Where would you put this logic in cssnano or css-declaration-sorter?

Make sense to put it here, because your module sort of declarations only 😄

Also I think css-declaration-sorter can order @media queries too, unfortunately postcss don't have ability to parse them without additional libraries, but it is other great task for future and out of scope these tasks.

[Siilwyn]

Thinking of creating a comparison using stylesheets used in real websites.

A quick experiment using the benchmark comparsion you linked @ludofischer gives the following:

• Where 'no order' means the sorter is disabled, the results are quite mixed
• The first stylesheet is from https://deno.land/

I'll also look into @alexander-akait's idea, I am afraid of the performance impact on cssnano though since we'd need to run something like gzip-size on it.

[alexander-akait]

Good research, can you share example (gist/git)?

[Siilwyn]

@alexander-akait yeah sure: https://github.com/Siilwyn/temp-css-minification-benchmark-dump

[alexander-akait]

Sometimes I think we should convert all postcss plugin into one package, it will improve perf very well...

@ludofischer What do you think?

[ludofischer]

Sometimes I think we should convert all postcss plugin into one package, it will improve perf very well...

@ludofischer What do you think?

I think we've talked about it before. I am in favour, but I thought to try to fix some of the remaining bugs, like the calc() ones.

[alexander-akait]

Regarding to calc - I think we can keep content as is, if parser can't parse content - I do it postcss/postcss-calc#137, maybe we can merge it