Skip to content

Commit

Permalink
See #290 - removes aggressive merging.
Browse files Browse the repository at this point in the history
Why:

* Aggressive merging was replaced by override merging;
* it supports much more properties now so aggressive merging won't
  be missed;
* it also overrides based on understandability not merely a
  position in output file.

Side notes:

* This is a safe change as if a property is not configured to
  be overridable it won't be;
* overhauls `canOverride` functionality to prevent features like
  vendor prefixes or variables from overriding widely supported
  values;
* overhauls validating values by allowing known values, which
  strictly doesn't make it a full validator but rather a way
  of telling if a value *may* be valid.
  • Loading branch information
jakubpawlowicz committed Jan 18, 2017
1 parent 04b4958 commit 11efd42
Show file tree
Hide file tree
Showing 32 changed files with 1,727 additions and 741 deletions.
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -13,6 +13,7 @@
* Simplifies URL rebasing with a single `rebaseTo` option in API or inferred from `--output` in CLI.
* Splits `inliner` option into `inlineRequest` and `inlineTimeout`.
* Fixed issue [#209](https://github.com/jakubpawlowicz/clean-css/issues/209) - adds output formatting via `format` flag.
* Fixed issue [#290](https://github.com/jakubpawlowicz/clean-css/issues/290) - removes aggressive merging.
* Fixed issue [#432](https://github.com/jakubpawlowicz/clean-css/issues/432) - adds URLs normalization.
* Fixed issue [#460](https://github.com/jakubpawlowicz/clean-css/issues/460) - unescaped semicolon in selector.
* Fixed issue [#657](https://github.com/jakubpawlowicz/clean-css/issues/657) - adds property name validation.
Expand Down
3 changes: 1 addition & 2 deletions README.md
Expand Up @@ -42,6 +42,7 @@ There will be some breaking changes:
* level 1 optimizations are the new default, up to 3.x it was level 2;
* `--keep-line-breaks` / `keepBreaks` option is replaced with `--format keep-breaks` / `{ format: 'keep-breaks' }` to ease transition.
* `sourceMap` option is API has to be a boolean from now on. If you want to specify an input source map pass it a 2nd argument to `minify` method or via a hash instead;
* `--skip-aggressive-merging` / `aggressiveMerging` option is removed as aggressive merging is gone too, replaced by smarter override merging.

Please note this list is not final. You are more than welcome to comment these changes in [4.0 release discussion](https://github.com/jakubpawlowicz/clean-css/issues/842) thread.

Expand Down Expand Up @@ -76,7 +77,6 @@ Options:
-O <n> [optimizations] Turn on level <n> optimizations; optionally accepts a list of fine-grained options, defaults to `1`, see examples below
--inline [rules] Enables inlining for listed sources (defaults to `local`)
--inline-timeout [seconds] Per connection timeout when fetching remote stylesheets (defaults to 5 seconds)
--skip-aggressive-merging Disable properties merging based on their order
--skip-rebase Disable URLs rebasing
--source-map Enables building input's source map
--source-map-inline-sources Enables inlining sources inside source maps
Expand Down Expand Up @@ -199,7 +199,6 @@ var minified = new CleanCSS().minify(source).styles;
CleanCSS constructor accepts a hash as a parameter, i.e.,
`new CleanCSS(options)` with the following options available:

* `aggressiveMerging` - set to false to disable aggressive merging of properties.
* `compatibility` - enables compatibility mode, see [below for more examples](#how-to-set-a-compatibility-mode)
* `format` - formats output CSS by using indentation and one rule or property per line.
* `inline` - whether to inline `@import` rules, can be `['all']`, `['local']` (default), `['remote']`, or a blacklisted domain/path e.g. `['!fonts.googleapis.com']`
Expand Down
2 changes: 0 additions & 2 deletions bin/cleancss
Expand Up @@ -20,7 +20,6 @@ commands
.option('-O <n> [optimizations]', 'Turn on level <n> optimizations; optionally accepts a list of fine-grained options, defaults to `1`, see examples below', function (val) { return Math.abs(parseInt(val)); })
.option('--inline [rules]', 'Enables inlining for listed sources (defaults to `local`)')
.option('--inline-timeout [seconds]', 'Per connection timeout when fetching remote stylesheets (defaults to 5 seconds)', parseFloat)
.option('--skip-aggressive-merging', 'Disable properties merging based on their order')
.option('--skip-rebase', 'Disable URLs rebasing')
.option('--source-map', 'Enables building input\'s source map')
.option('--source-map-inline-sources', 'Enables inlining sources inside source maps');
Expand Down Expand Up @@ -125,7 +124,6 @@ if (!fromStdin && commands.args.length === 0) {
// Now coerce commands into CleanCSS configuration...
var debugMode = commands.debug;
var options = {
aggressiveMerging: commands.skipAggressiveMerging ? false : true,
compatibility: commands.compatibility,
format: commands.format,
inline: commands.inline || 'local',
Expand Down
1 change: 0 additions & 1 deletion lib/clean.js
Expand Up @@ -31,7 +31,6 @@ var CleanCSS = module.exports = function CleanCSS(options) {
options = options || {};

this.options = {
aggressiveMerging: undefined === options.aggressiveMerging ? true : !!options.aggressiveMerging,
compatibility: compatibility(options.compatibility),
format: formatFrom(options.format),
inline: inlineOptionsFrom(options.inline),
Expand Down
8 changes: 4 additions & 4 deletions lib/optimizer/level-2/break-up.js
Expand Up @@ -10,7 +10,7 @@ var MULTIPLEX_SEPARATOR = ',';

function _colorFilter(validator) {
return function (value) {
return value[1] == 'invert' || validator.isValidColor(value[1]);
return value[1] == 'invert' || validator.isValidColor(value[1]) || validator.isValidVendorPrefixedValue(value[1]);
};
}

Expand Down Expand Up @@ -46,7 +46,7 @@ function _wrapDefault(name, property, compactable) {

function _widthFilter(validator) {
return function (value) {
return value[1] != 'inherit' && validator.isValidWidth(value[1]) && !validator.isValidStyleKeyword(value[1]) && !validator.isValidColorValue(value[1]);
return value[1] != 'inherit' && validator.isValidWidth(value[1]) && !validator.isValidStyle(value[1]) && !validator.isValidColorValue(value[1]);
};
}

Expand Down Expand Up @@ -85,7 +85,7 @@ function background(property, compactable, validator) {
if (validator.isValidBackgroundAttachment(value[1])) {
attachment.value = [value];
anyValueSet = true;
} else if (validator.isValidBackgroundBox(value[1])) {
} else if (validator.isValidBackgroundClip(value[1]) || validator.isValidBackgroundOrigin(value[1])) {
if (clipSet) {
origin.value = [value];
originSet = true;
Expand Down Expand Up @@ -126,7 +126,7 @@ function background(property, compactable, validator) {
positionSet = true;
}
anyValueSet = true;
} else if ((color.value[0][1] == compactable[color.name].defaultValue || color.value[0][1] == 'none') && validator.isValidColor(value[1])) {
} else if ((color.value[0][1] == compactable[color.name].defaultValue || color.value[0][1] == 'none') && (validator.isValidColor(value[1]) || validator.isValidVendorPrefixedValue(value[1]))) {
color.value = [value];
anyValueSet = true;
} else if (validator.isValidUrl(value[1]) || validator.isValidFunction(value[1])) {
Expand Down
234 changes: 136 additions & 98 deletions lib/optimizer/level-2/can-override.js
@@ -1,142 +1,180 @@
// Functions that decide what value can override what.
// The main purpose is to disallow removing CSS fallbacks.
// A separate implementation is needed for every different kind of CSS property.
// -----
// The generic idea is that properties that have wider browser support are 'more understandable'
// than others and that 'less understandable' values can't override more understandable ones.

// Use when two tokens of the same property can always be merged
function always() {
return true;
}

function alwaysButIntoFunction(property1, property2, validator) {
var value1 = property1.value[0][1];
var value2 = property2.value[0][1];

var validFunction1 = validator.isValidFunction(value1);
var validFunction2 = validator.isValidFunction(value2);
var understandable = require('./compacting/understandable');

if (validFunction1 && validFunction2) {
return validator.areSameFunction(value1, value2);
} else if (!validFunction1 && validFunction2) {
function backgroundPosition(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidKeywordValue('background-position', value2, true)) {
return false;
} else {
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
} else if (validator.isValidKeywordValue('background-position', value2, true)) {
return true;
}

return unit(validator, value1, value2);
}

function backgroundImage(property1, property2, validator) {
// The idea here is that 'more understandable' values override 'less understandable' values, but not vice versa
// Understandability: (none | url | inherit) > (same function) > (same value)
function backgroundSize(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidKeywordValue('background-size', value2, true)) {
return false;
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
} else if (validator.isValidKeywordValue('background-size', value2, true)) {
return true;
}

// (none | url)
var image1 = property1.value[0][1];
var image2 = property2.value[0][1];
return unit(validator, value1, value2);
}

if (image2 == 'none' || image2 == 'inherit' || validator.isValidUrl(image2))
function color(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidColor(value2)) {
return false;
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
if (image1 == 'none' || image1 == 'inherit' || validator.isValidUrl(image1))
} else if (!validator.colorOpacity && (validator.isValidRgbaColor(value1) || validator.isValidHslaColor(value1))) {
return false;
} else if (!validator.colorOpacity && (validator.isValidRgbaColor(value2) || validator.isValidHslaColor(value2))) {
return false;
} else if (validator.isValidColor(value1) && validator.isValidColor(value2)) {
return true;
}

// Functions with the same name can override each other; same values can override each other
return sameFunctionOrValue(property1, property2, validator);
return sameFunctionOrValue(validator, value1, value2);
}

function border(property1, property2, validator) {
return color(property1.components[2], property2.components[2], validator);
function components(overrideCheckers) {
return function (validator, value1, value2, position) {
return overrideCheckers[position](validator, value1, value2);
};
}

// Use for color properties (color, background-color, border-color, etc.)
function color(property1, property2, validator) {
// The idea here is that 'more understandable' values override 'less understandable' values, but not vice versa
// Understandability: (hex | named) > (rgba | hsla) > (same function name) > anything else
// NOTE: at this point rgb and hsl are replaced by hex values by clean-css

var color1 = property1.value[0][1];
var color2 = property2.value[0][1];

if (!validator.colorOpacity && (validator.isValidRgbaColor(color1) || validator.isValidHslaColor(color1)))
function image(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidImage(value2)) {
return false;
if (!validator.colorOpacity && (validator.isValidRgbaColor(color2) || validator.isValidHslaColor(color2)))
return false;

// (hex | named)
if (validator.isValidNamedColor(color2) || validator.isValidHexColor(color2))
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
if (validator.isValidNamedColor(color1) || validator.isValidHexColor(color1))
return false;

// (rgba|hsla)
if (validator.isValidRgbaColor(color2) || validator.isValidHslaColor(color2))
} else if (validator.isValidImage(value2)) {
return true;
if (validator.isValidRgbaColor(color1) || validator.isValidHslaColor(color1))
} else if (validator.isValidImage(value1)) {
return false;
}

// Functions with the same name can override each other; same values can override each other
return sameFunctionOrValue(property1, property2, validator);
return sameFunctionOrValue(validator, value1, value2);
}

function twoOptionalFunctions(property1, property2, validator) {
var value1 = property1.value[0][1];
var value2 = property2.value[0][1];
function keyword(propertyName) {
return function(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidKeywordValue(propertyName, value2)) {
return false;
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
}

return !(validator.isValidFunction(value1) ^ validator.isValidFunction(value2));
return validator.isValidKeywordValue(propertyName, value2, false);
};
}

function sameValue(property1, property2) {
var value1 = property1.value[0][1];
var value2 = property2.value[0][1];
function keywordWithGlobal(propertyName) {
return function(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidKeywordValue(propertyName, value2, true)) {
return false;
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
}

return value1 === value2;
return validator.isValidKeywordValue(propertyName, value2, true);
};
}

function sameFunctionOrValue(property1, property2, validator) {
var value1 = property1.value[0][1];
var value2 = property2.value[0][1];
function sameFunctionOrValue(validator, value1, value2) {
return validator.areSameFunction(value1, value2) ?
true :
value1 === value2;
}

// Functions with the same name can override each other
if (validator.areSameFunction(value1, value2))
function textShadow(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidTextShadow(value2)) {
return false;
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
}

return value1 === value2;
return validator.isValidTextShadow(value2);
}

// Use for properties containing CSS units (margin-top, padding-left, etc.)
function unit(property1, property2, validator) {
// The idea here is that 'more understandable' values override 'less understandable' values, but not vice versa
// Understandability: (unit without functions) > (same functions | standard functions) > anything else
// NOTE: there is no point in having different vendor-specific functions override each other or standard functions,
// or having standard functions override vendor-specific functions, but standard functions can override each other
// NOTE: vendor-specific property values are not taken into consideration here at the moment
var value1 = property1.value[0][1];
var value2 = property2.value[0][1];

if (validator.isValidAndCompatibleUnitWithoutFunction(value1) && !validator.isValidAndCompatibleUnitWithoutFunction(value2))
function unit(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidUnitWithoutFunction(value2)) {
return false;

if (validator.isValidUnitWithoutFunction(value2))
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
} else if (validator.isValidUnitWithoutFunction(value1) && !validator.isValidUnitWithoutFunction(value2)) {
return false;
} else if (validator.isValidUnitWithoutFunction(value2)) {
return true;
if (validator.isValidUnitWithoutFunction(value1))
} else if (validator.isValidUnitWithoutFunction(value1)) {
return false;
} else if (validator.isValidFunctionWithoutVendorPrefix(value1) && validator.isValidFunctionWithoutVendorPrefix(value2)) {
return true;
}

return sameFunctionOrValue(validator, value1, value2);
}

function unitOrKeywordWithGlobal(propertyName) {
var byKeyword = keywordWithGlobal(propertyName);

// Standard non-vendor-prefixed functions can override each other
if (validator.isValidFunctionWithoutVendorPrefix(value2) && validator.isValidFunctionWithoutVendorPrefix(value1)) {
return function(validator, value1, value2) {
return unit(validator, value1, value2) || byKeyword(validator, value1, value2);
};
}

function zIndex(validator, value1, value2) {
if (!understandable(validator, value1, value2, 0, true) && !validator.isValidZIndex(value2)) {
return false;
} else if (validator.isValidVariable(value1) && validator.isValidVariable(value2)) {
return true;
}

// Functions with the same name can override each other; same values can override each other
return sameFunctionOrValue(property1, property2, validator);
return validator.isValidZIndex(value2);
}

module.exports = {
always: always,
alwaysButIntoFunction: alwaysButIntoFunction,
backgroundImage: backgroundImage,
border: border,
color: color,
sameValue: sameValue,
sameFunctionOrValue: sameFunctionOrValue,
twoOptionalFunctions: twoOptionalFunctions,
unit: unit
generic: {
color: color,
components: components,
image: image,
unit: unit
},
property: {
backgroundAttachment: keyword('background-attachment'),
backgroundClip: keywordWithGlobal('background-clip'),
backgroundOrigin: keyword('background-origin'),
backgroundPosition: backgroundPosition,
backgroundRepeat: keyword('background-repeat'),
backgroundSize: backgroundSize,
bottom: unitOrKeywordWithGlobal('bottom'),
borderCollapse: keyword('border-collapse'),
borderStyle: keywordWithGlobal('*-style'),
clear: keywordWithGlobal('clear'),
cursor: keywordWithGlobal('cursor'),
display: keywordWithGlobal('display'),
float: keywordWithGlobal('float'),
fontStyle: keywordWithGlobal('font-style'),
left: unitOrKeywordWithGlobal('left'),
fontWeight: keywordWithGlobal('font-weight'),
listStyleType: keywordWithGlobal('list-style-type'),
listStylePosition: keywordWithGlobal('list-style-position'),
outlineStyle: keywordWithGlobal('*-style'),
overflow: keywordWithGlobal('overflow'),
position: keywordWithGlobal('position'),
right: unitOrKeywordWithGlobal('right'),
textAlign: keywordWithGlobal('text-align'),
textDecoration: keywordWithGlobal('text-decoration'),
textOverflow: keywordWithGlobal('text-overflow'),
textShadow: textShadow,
top: unitOrKeywordWithGlobal('top'),
transform: sameFunctionOrValue,
verticalAlign: unitOrKeywordWithGlobal('vertical-align'),
visibility: keywordWithGlobal('visibility'),
whiteSpace: keywordWithGlobal('white-space'),
zIndex: zIndex
}
};

0 comments on commit 11efd42

Please sign in to comment.