Skip to content

Commit

Permalink
feat(postcss-colormin): switch to colord and solve multiple issues
Browse files Browse the repository at this point in the history
* ensure that the replacement color values are shorter than the original. Previously, the plugin returned the converted value without checking whether it was shorter than the original, we now ensure we always return the shortest color value
* rework the colomin cache so use only one cache as in #771 (without using Ramda as in that PR). The cache key is now just the declaration value. Before there were two caches, one with a CSS `property|value` key and one that cached the single colors.
* change the underlying library to [colord](https://github.com/omgovich/colord).
Using `colord` comes with some trade-offs. On the plus side, it automatically rounds up up, which fixes an [open issue](#819) without additional work, reduces transitive dependencies from 5 to 0 (although I think the previous 4 are all maintained by the same people), is smaller and claims to be faster (haven't checked but I suspect css-value-parser is more the bottle neck than color conversion here).
On the minus side, the `colord` parser is very lenient so it accepts invalid CSS colors (rgb with mixed percentages and numbers) which cssnano did not convert before, so I've added some checks to skip these values. Overall there are less lines of code.
* get rid of the JSON file with precomputed CSS color names. I guess the idea was that the JSON file only contained color names that are shorter than the hex, but the color conversion libraries include all color names anyway to be able to parse, so in fact we ship the color names twice. I don't think the extra code for generating the file, plus loading the JSON is worth it to avoid a an extra length check.

fix(postcss-colormin): do not replace original value with longer one

fix #1042

feat(postcss-colormin): switch to colord

* round color values fix #819
* reduce dependencies

perf(postcss-colormin): improve caching

Fix #771

refactor(postcss-colormin): drop the generate script
  • Loading branch information
ludofischer committed May 18, 2021
1 parent ea1ff0b commit 1d36ad1
Show file tree
Hide file tree
Showing 29 changed files with 110 additions and 232 deletions.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions packages/postcss-colormin/package.json
Expand Up @@ -9,7 +9,7 @@
],
"scripts": {
"prebuild": "del-cli dist",
"build": "cross-env BABEL_ENV=publish babel src --config-file ../../babel.config.json --out-dir dist --ignore \"**/__tests__/\" && babel-node script/generate",
"build": "cross-env BABEL_ENV=publish babel src --config-file ../../babel.config.json --out-dir dist --ignore \"**/__tests__/\"",
"prepublish": "yarn build"
},
"keywords": [
Expand All @@ -31,7 +31,7 @@
"repository": "cssnano/cssnano",
"dependencies": {
"browserslist": "^4.16.0",
"color": "^3.1.1",
"colord": "^1.7.0",
"postcss-value-parser": "^4.1.0"
},
"bugs": {
Expand Down
40 changes: 0 additions & 40 deletions packages/postcss-colormin/script/generate.js

This file was deleted.

9 changes: 7 additions & 2 deletions packages/postcss-colormin/src/__tests__/colours.js
Expand Up @@ -52,7 +52,7 @@ test(

test(
'should convert rgba to hsla when shorter',
isEqual('rgba(221, 221, 221, 0.5)', 'hsla(0, 0%, 86.7%, 0.5)')
isEqual('rgba(221, 221, 221, 0.5)', 'hsla(0, 0%, 87%, 0.5)')
);

test(
Expand Down Expand Up @@ -104,7 +104,7 @@ test(

test(
'should convert percentage based rgba values (2)',
isEqual('rgba(50%,50%,50%,0.5)', 'hsla(0, 0%, 49.8%, 0.5)')
isEqual('rgba(50%,50%,50%,0.5)', 'hsla(0, 0%, 50%, 0.5)')
);

test(
Expand Down Expand Up @@ -133,6 +133,11 @@ test('should convert 8 character hex codes', isEqual('#000000FF', '#000'));

test('should convert 4 character hex codes', isEqual('#000F', '#000'));

test(
'should pass through 8 character hex codes',
isEqual('#00000004', '#00000004')
);

test('should pass through if not recognised', () => {
expect(min('Unrecognised')).toBe('Unrecognised');
expect(min('inherit')).toBe('inherit');
Expand Down
2 changes: 1 addition & 1 deletion packages/postcss-colormin/src/__tests__/index.js
Expand Up @@ -207,7 +207,7 @@ test(
'should not mangle percentage based rgba values',
processCSS(
'h1{color:rgba(50%,50%,50%,0.5)}',
'h1{color:hsla(0, 0%, 49.8%, 0.5)}'
'h1{color:hsla(0, 0%, 50%, 0.5)}'
)
);

Expand Down
86 changes: 39 additions & 47 deletions packages/postcss-colormin/src/colours.js
@@ -1,66 +1,58 @@
import color from 'color';
import keywords from './keywords.json';
import { colord, extend, getFormat } from 'colord';
import namesPlugin from 'colord/plugins/names';
import toShorthand from './lib/toShorthand';

const shorter = (a, b) => (a && a.length < b.length ? a : b).toLowerCase();
extend([namesPlugin]);

export default (colour, isLegacy = false, cache = false) => {
const key = colour + '|' + isLegacy;

if (cache && cache[key]) {
return cache[key];
export default (colour, isLegacy = false) => {
if (getFormat(colour) === 'rgb') {
/* check that it is a valid CSS value
https://www.w3.org/TR/css-color-4/#rgb-functions */
let percentCount = 0;
for (const c of colour) {
if (c === '%') {
percentCount++;
}
}
// rgb values should either be all percentages or all numbers
if (percentCount !== 3 && percentCount !== 0) {
return colour;
}
}

try {
const parsed = color(colour.toLowerCase());
const parsed = colord(colour.toLowerCase());
if (parsed.isValid()) {
const alpha = parsed.alpha();

if (alpha === 1) {
const toHex = toShorthand(parsed.hex().toLowerCase());
const result = shorter(keywords[toHex], toHex);

if (cache) {
cache[key] = result;
const toHex = toShorthand(parsed.toHex());
const toName = parsed.toName();
if (toName && toName.length < toHex.length) {
return toName;
} else {
return toHex;
}

return result;
} else {
const rgb = parsed.rgb();
const rgb = parsed.toRgb();

if (
!isLegacy &&
!rgb.color[0] &&
!rgb.color[1] &&
!rgb.color[2] &&
!alpha
) {
const result = 'transparent';

if (cache) {
cache[key] = result;
}

return result;
if (!isLegacy && !rgb.r && !rgb.g && !rgb.b && !alpha) {
return 'transparent';
}

let hsla = parsed.hsl().string();
let rgba = rgb.string();
let result = hsla.length < rgba.length ? hsla : rgba;
let hsla = parsed.toHslString();
let rgba = parsed.toRgbString();

if (cache) {
cache[key] = result;
}
const shortestConversion = hsla.length < rgba.length ? hsla : rgba;

let result;
if (colour.length < shortestConversion.length) {
result = colour;
} else {
result = shortestConversion;
}
return result;
}
} catch (e) {
} else {
// Possibly malformed, so pass through
const result = colour;

if (cache) {
cache[key] = result;
}

return result;
return colour;
}
};
11 changes: 5 additions & 6 deletions packages/postcss-colormin/src/index.js
Expand Up @@ -31,15 +31,15 @@ function isMathFunctionNode(node) {
return ['calc', 'min', 'max', 'clamp'].includes(node.value.toLowerCase());
}

function transform(value, isLegacy, colorminCache) {
function transform(value, isLegacy) {
const parsed = valueParser(value);

walk(parsed, (node, index, parent) => {
if (node.type === 'function') {
if (/^(rgb|hsl)a?$/i.test(node.value)) {
const { value: originalValue } = node;

node.value = colormin(stringify(node), isLegacy, colorminCache);
node.value = colormin(stringify(node), isLegacy);
node.type = 'word';

const next = parent.nodes[index + 1];
Expand All @@ -58,7 +58,7 @@ function transform(value, isLegacy, colorminCache) {
return false;
}
} else if (node.type === 'word') {
node.value = colormin(node.value, isLegacy, colorminCache);
node.value = colormin(node.value, isLegacy);
}
});

Expand All @@ -77,7 +77,6 @@ function pluginCreator() {
env: resultOpts.env,
});
const isLegacy = browsers.some(hasTransparentBug);
const colorminCache = {};
const cache = {};

return {
Expand All @@ -97,15 +96,15 @@ function pluginCreator() {
return;
}

const cacheKey = `${decl.prop}|${decl.value}`;
const cacheKey = value + '|' + isLegacy;

if (cache[cacheKey]) {
decl.value = cache[cacheKey];

return;
}

const newValue = transform(value, isLegacy, colorminCache);
const newValue = transform(value, isLegacy);

decl.value = newValue;
cache[cacheKey] = newValue;
Expand Down
33 changes: 0 additions & 33 deletions packages/postcss-colormin/src/keywords.json

This file was deleted.

55 changes: 5 additions & 50 deletions packages/postcss-colormin/yarn.lock
Expand Up @@ -18,45 +18,12 @@ caniuse-lite@^1.0.30001165:
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001170.tgz#0088bfecc6a14694969e391cc29d7eb6362ca6a7"
integrity sha512-Dd4d/+0tsK0UNLrZs3CvNukqalnVTRrxb5mcQm8rHL49t7V5ZaTygwXkrq+FB+dVDf++4ri8eJnFEJAB8332PA==

color-convert@^1.9.1:
version "1.9.3"
resolved "https://registry.yarnpkg.com/color-convert/-/color-convert-1.9.3.tgz#bb71850690e1f136567de629d2d5471deda4c1e8"
integrity sha512-QfAUtd+vFdAtFQcC8CCyYt1fYWxSqAiK2cSD6zDB8N3cpsEBAvRxp9zOGg6G/SHHJYAT88/az/IuDGALsNVbGg==
dependencies:
color-name "1.1.3"

color-name@1.1.3:
version "1.1.3"
resolved "https://registry.yarnpkg.com/color-name/-/color-name-1.1.3.tgz#a7d0558bd89c42f795dd42328f740831ca53bc25"
integrity sha1-p9BVi9icQveV3UIyj3QIMcpTvCU=

color-name@^1.0.0:
version "1.1.4"
resolved "https://registry.yarnpkg.com/color-name/-/color-name-1.1.4.tgz#c2a09a87acbde69543de6f63fa3995c826c536a2"
integrity sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==
colord@^1.7.0:
version "1.7.0"
resolved "https://registry.yarnpkg.com/colord/-/colord-1.7.0.tgz#2d2b3df19ebe81d72e626c5604e8e3e5867bb5d8"
integrity sha512-/9I83NehU/HvAKGTSEZrFFEubYHknzGMpSDOee2JBTqk42qbYd+EOKfN22gmh7kysTwdpNfbJGgfUiqGkLul0A==

color-string@^1.5.4:
version "1.5.4"
resolved "https://registry.yarnpkg.com/color-string/-/color-string-1.5.4.tgz#dd51cd25cfee953d138fe4002372cc3d0e504cb6"
integrity sha512-57yF5yt8Xa3czSEW1jfQDE79Idk0+AkN/4KWad6tbdxUmAs3MvjxlWSWD4deYytcRfoZ9nhKyFl1kj5tBvidbw==
dependencies:
color-name "^1.0.0"
simple-swizzle "^0.2.2"

color@^3.1.1:
version "3.1.3"
resolved "https://registry.yarnpkg.com/color/-/color-3.1.3.tgz#ca67fb4e7b97d611dcde39eceed422067d91596e"
integrity sha512-xgXAcTHa2HeFCGLE9Xs/R82hujGtu9Jd9x4NW3T34+OMs7VoPsjwzRczKHvTAHeJwWFwX5j15+MgAppE8ztObQ==
dependencies:
color-convert "^1.9.1"
color-string "^1.5.4"

colorette@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/colorette/-/colorette-1.2.1.tgz#4d0b921325c14faf92633086a536db6e89564b1b"
integrity sha512-puCDz0CzydiSYOrnXpz/PKd69zRrribezjtE9yd4zvytoRc8+RY/KJPvtPFKZS3E3wP6neGyMe0vOTlHO5L3Pw==

colorette@^1.2.2:
colorette@^1.2.1, colorette@^1.2.2:
version "1.2.2"
resolved "https://registry.yarnpkg.com/colorette/-/colorette-1.2.2.tgz#cbcc79d5e99caea2dbf10eb3a26fd8b3e6acfa94"
integrity sha512-MKGMzyfeuutC/ZJ1cba9NqcNpfeqMUcYmyF1ZFY6/Cn7CNSAKx6a+s48sqLqyAiZuaP2TcqMhoo+dlwFnVxT9w==
Expand All @@ -71,11 +38,6 @@ escalade@^3.1.1:
resolved "https://registry.yarnpkg.com/escalade/-/escalade-3.1.1.tgz#d8cfdc7000965c5a0174b4a82eaa5c0552742e40"
integrity sha512-k0er2gUkLf8O0zKJiAhmkTnJlTvINGv7ygDNPbeIsX/TJjGJZHuh9B2UxbsaEkmlEo9MfhrSzmhIlhRlI2GXnw==

is-arrayish@^0.3.1:
version "0.3.2"
resolved "https://registry.yarnpkg.com/is-arrayish/-/is-arrayish-0.3.2.tgz#4574a2ae56f7ab206896fb431eaeed066fdf8f03"
integrity sha512-eVRqCvVlZbuw3GrM63ovNSNAeA1K16kaR/LRY/92w0zxQ5/1YzwblUX652i4Xs9RwAGjW9d9y6X88t8OaAJfWQ==

nanoid@^3.1.23:
version "3.1.23"
resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.1.23.tgz#f744086ce7c2bc47ee0a8472574d5c78e4183a81"
Expand All @@ -100,13 +62,6 @@ postcss@^8.2.15:
nanoid "^3.1.23"
source-map "^0.6.1"

simple-swizzle@^0.2.2:
version "0.2.2"
resolved "https://registry.yarnpkg.com/simple-swizzle/-/simple-swizzle-0.2.2.tgz#a4da6b635ffcccca33f70d17cb92592de95e557a"
integrity sha1-pNprY1/8zMoz9w0Xy5JZLeleVXo=
dependencies:
is-arrayish "^0.3.1"

source-map@^0.6.1:
version "0.6.1"
resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.6.1.tgz#74722af32e9614e9c287a8d0bbde48b5e2f1a263"
Expand Down

0 comments on commit 1d36ad1

Please sign in to comment.