Skip to content

Commit

Permalink
See #290 - reworks border overriding.
Browse files Browse the repository at this point in the history
Why:

* Border is a complex case where both `border` and all its
  components: `border-color`, `border-style`, and `border-width`
  are components. Also `border-(bottom|left|right|top)` are
  shorthands too.
* this commit restores aggressive merging shorthand overriding right
  inside shorthand compacting code.
  • Loading branch information
jakubpawlowicz committed Jan 18, 2017
1 parent e1d88d3 commit 04b4958
Show file tree
Hide file tree
Showing 23 changed files with 1,014 additions and 165 deletions.
284 changes: 240 additions & 44 deletions lib/optimizer/level-2/compactable.js

Large diffs are not rendered by default.

40 changes: 40 additions & 0 deletions lib/optimizer/level-2/compacting/find-component-in.js
@@ -0,0 +1,40 @@
var compactable = require('../compactable');

function findComponentIn(shorthand, longhand) {
var comparator = nameComparator(longhand);

return findInDirectComponents(shorthand, comparator) || findInSubComponents(shorthand, comparator);
}

function nameComparator(to) {
return function (property) {
return to.name === property.name;
};
}

function findInDirectComponents(shorthand, comparator) {
return shorthand.components.filter(comparator)[0];
}

function findInSubComponents(shorthand, comparator) {
var shorthandComponent;
var longhandMatch;
var i, l;

if (!compactable[shorthand.name].shorthandComponents) {
return;
}

for (i = 0, l = shorthand.components.length; i < l; i++) {
shorthandComponent = shorthand.components[i];
longhandMatch = findInDirectComponents(shorthandComponent, comparator);

if (longhandMatch) {
return longhandMatch;
}
}

return;
}

module.exports = findComponentIn;
22 changes: 22 additions & 0 deletions lib/optimizer/level-2/compacting/is-component-of.js
@@ -0,0 +1,22 @@
var compactable = require('../compactable');

function isComponentOf(property1, property2, shallow) {
return isDirectComponentOf(property1, property2) ||
!shallow && !!compactable[property1.name].shorthandComponents && isSubComponentOf(property1, property2);
}

function isDirectComponentOf(property1, property2) {
var descriptor = compactable[property1.name];

return 'components' in descriptor && descriptor.components.indexOf(property2.name) > -1;
}

function isSubComponentOf(property1, property2) {
return property1
.components
.some(function (component) {
return isDirectComponentOf(component, property2);
});
}

module.exports = isComponentOf;
43 changes: 6 additions & 37 deletions lib/optimizer/level-2/compacting/optimize.js
Expand Up @@ -13,28 +13,6 @@ var OptimizationLevel = require('../../../options/optimization-level').Optimizat

var serializeProperty = require('../../../writer/one-time').property;

var shorthands = {
'border-color': ['border'],
'border-style': ['border'],
'border-width': ['border'],
'border-bottom': ['border'],
'border-bottom-color': ['border-bottom', 'border-color', 'border'],
'border-bottom-style': ['border-bottom', 'border-style', 'border'],
'border-bottom-width': ['border-bottom', 'border-width', 'border'],
'border-left': ['border'],
'border-left-color': ['border-left', 'border-color', 'border'],
'border-left-style': ['border-left', 'border-style', 'border'],
'border-left-width': ['border-left', 'border-width', 'border'],
'border-right': ['border'],
'border-right-color': ['border-right', 'border-color', 'border'],
'border-right-style': ['border-right', 'border-style', 'border'],
'border-right-width': ['border-right', 'border-width', 'border'],
'border-top': ['border'],
'border-top-color': ['border-top', 'border-color', 'border'],
'border-top-style': ['border-top', 'border-style', 'border'],
'border-top-width': ['border-top', 'border-width', 'border'],
};

function _optimize(properties, mergeAdjacent, aggressiveMerging, validator) {
var overrideMapping = {};
var lastName = null;
Expand Down Expand Up @@ -123,26 +101,14 @@ function _optimize(properties, mergeAdjacent, aggressiveMerging, validator) {
} else {
overrideMapping[_name] = overrideMapping[_name] || [];
overrideMapping[_name].push(position);

// TODO: to be removed with
// certain shorthand (see values of `shorthands`) should trigger removal of
// longhand properties (see keys of `shorthands`)
var _shorthands = shorthands[_name];
if (_shorthands) {
for (j = _shorthands.length - 1; j >= 0; j--) {
var shorthand = _shorthands[j];
overrideMapping[shorthand] = overrideMapping[shorthand] || [];
overrideMapping[shorthand].push(position);
}
}
}

lastName = _name;
lastProperty = property;
}
}

function compactorOptimize(selector, properties, mergeAdjacent, withCompacting, context) {
function compactorOptimize(selector, properties, mergeAdjacent, withCompacting, overrideOptions, context) {
var validator = context.validator;
var warnings = context.warnings;

Expand All @@ -153,12 +119,15 @@ function compactorOptimize(selector, properties, mergeAdjacent, withCompacting,
for (var i = 0, l = _properties.length; i < l; i++) {
var _property = _properties[i];
if (_property.block) {
compactorOptimize(selector, _property.value[0][1], mergeAdjacent, withCompacting, context);
compactorOptimize(selector, _property.value[0][1], mergeAdjacent, withCompacting, overrideOptions, context);
}
}

if (overrideOptions.enabled && context.options.level[OptimizationLevel.Two].compactShorthands) {
compactOverrides(_properties, context.options.compatibility, overrideOptions.merging, validator);
}

if (withCompacting && context.options.level[OptimizationLevel.Two].compactShorthands) {
compactOverrides(_properties, context.options.compatibility, validator);
compactShorthands(_properties, validator);
}

Expand Down
70 changes: 48 additions & 22 deletions lib/optimizer/level-2/compacting/override-compactor.js
@@ -1,10 +1,14 @@
var hasInherit = require('./has-inherit');
var everyCombination = require('./every-combination');
var findComponentIn = require('./find-component-in');
var isComponentOf = require('./is-component-of');
var overridesNonComponentShorthand = require('./overrides-non-component-shorthand');
var sameVendorPrefixesIn = require('./vendor-prefixes').same;

var canOverride = require('../can-override');
var compactable = require('../compactable');
var deepClone = require('../clone').deep;
var deepClone = require('../clone').deep;
var restoreWithComponents = require('../restore-with-components');
var shallowClone = require('../clone').shallow;

Expand All @@ -15,13 +19,6 @@ var Marker = require('../../../tokenizer/marker');

var serializeProperty = require('../../../writer/one-time').property;

// Used when searching for a component that matches property
function nameMatchFilter(to) {
return function (property) {
return to.name === property.name;
};
}

function wouldBreakCompatibility(property, validator) {
for (var i = 0; i < property.components.length; i++) {
var component = property.components[i];
Expand All @@ -38,10 +35,6 @@ function wouldBreakCompatibility(property, validator) {
return false;
}

function isComponentOf(shorthand, longhand) {
return compactable[shorthand.name].components.indexOf(longhand.name) > -1;
}

function overrideIntoMultiplex(property, by) {
by.unused = true;

Expand Down Expand Up @@ -167,10 +160,10 @@ function wouldResultInLongerValue(left, right) {
var lengthBefore = lengthOf(multiplexClone) + 1 + lengthOf(simpleClone);

if (left.multiplex) {
component = multiplexClone.components.filter(nameMatchFilter(simpleClone))[0];
component = findComponentIn(multiplexClone, simpleClone);
overrideIntoMultiplex(component, simpleClone);
} else {
component = simpleClone.components.filter(nameMatchFilter(multiplexClone))[0];
component = findComponentIn(simpleClone, multiplexClone);
turnIntoMultiplex(simpleClone, multiplexSize(multiplexClone));
overrideByMultiplex(component, multiplexClone);
}
Expand Down Expand Up @@ -222,8 +215,11 @@ function intoLayers(values) {
return layers;
}

function compactOverrides(properties, compatibility, validator) {
function compactOverrides(properties, compatibility, merging, validator) {
var mayOverride, right, left, component;
var overriddenComponents;
var overriddenComponent;
var overridingComponent;
var i, j, k;

propertyLoop:
Expand All @@ -238,6 +234,7 @@ function compactOverrides(properties, compatibility, validator) {

mayOverride = compactable[right.name].canOverride || canOverride.sameValue;

traverseLoop:
for (j = i - 1; j >= 0; j--) {
left = properties[j];

Expand All @@ -259,7 +256,7 @@ function compactOverrides(properties, compatibility, validator) {
if (noneOverrideHack(left, right))
continue;

if (!left.shorthand && right.shorthand && isComponentOf(right, left)) {
if (right.shorthand && isComponentOf(right, left)) {
// maybe `left` can be overridden by `right` which is a shorthand?
if (!right.important && left.important)
continue;
Expand All @@ -270,12 +267,41 @@ function compactOverrides(properties, compatibility, validator) {
if (!anyValue(validator.isValidFunction, left) && overridingFunction(right, validator))
continue;

component = right.components.filter(nameMatchFilter(left))[0];
component = findComponentIn(right, left);
mayOverride = (compactable[left.name] && compactable[left.name].canOverride) || canOverride.sameValue;
if (everyCombination(mayOverride, left, component, validator)) {
left.unused = true;
}
} else if (left.shorthand && !right.shorthand && isComponentOf(left, right)) {
} else if (right.shorthand && overridesNonComponentShorthand(right, left)) {
// `right` is a shorthand while `left` can be overriden by it, think `border` and `border-top`
if (!right.important && left.important) {
continue;
}

if (!sameVendorPrefixesIn([left], right.components)) {
continue;
}

if (!anyValue(validator.isValidFunction, left) && overridingFunction(right, validator)) {
continue;
}

overriddenComponents = left.shorthand ?
left.components:
[left];

for (k = overriddenComponents.length - 1; k >= 0; k--) {
overriddenComponent = overriddenComponents[k];
overridingComponent = findComponentIn(right, overriddenComponent);
mayOverride = (overriddenComponent.name in compactable && compactable[overriddenComponent.name].canOverride) || canOverride.sameValue;

if (!everyCombination(mayOverride, left, overridingComponent, validator)) {
continue traverseLoop;
}
}

left.unused = true;
} else if (merging && left.shorthand && !right.shorthand && isComponentOf(left, right, true)) {
// maybe `right` can be pulled into `left` which is a shorthand?
if (right.important && !left.important)
continue;
Expand All @@ -292,7 +318,7 @@ function compactOverrides(properties, compatibility, validator) {
if (overridingFunction(left, validator))
continue;

component = left.components.filter(nameMatchFilter(right))[0];
component = findComponentIn(left, right);
if (everyCombination(mayOverride, component, right, validator)) {
var disabledBackgroundMerging =
!compatibility.properties.backgroundClipMerging && component.name.indexOf('background-clip') > -1 ||
Expand All @@ -318,7 +344,7 @@ function compactOverrides(properties, compatibility, validator) {
override(component, right);
left.dirty = true;
}
} else if (left.shorthand && right.shorthand && left.name == right.name) {
} else if (merging && left.shorthand && right.shorthand && left.name == right.name) {
// merge if all components can be merged

if (!left.multiplex && right.multiplex)
Expand Down Expand Up @@ -347,13 +373,13 @@ function compactOverrides(properties, compatibility, validator) {

overrideShorthand(left, right);
left.dirty = true;
} else if (left.shorthand && right.shorthand && isComponentOf(left, right)) {
} else if (merging && left.shorthand && right.shorthand && isComponentOf(left, right)) {
// border is a shorthand but any of its components is a shorthand too

if (!left.important && right.important)
continue;

component = left.components.filter(nameMatchFilter(right))[0];
component = findComponentIn(left, right);
mayOverride = compactable[right.name].canOverride || canOverride.sameValue;
if (!everyCombination(mayOverride, component, right, validator))
continue;
Expand All @@ -367,7 +393,7 @@ function compactOverrides(properties, compatibility, validator) {
if (rightRestored.length > 1)
continue;

component = left.components.filter(nameMatchFilter(right))[0];
component = findComponentIn(left, right);
override(component, right);
right.dirty = true;
} else if (left.name == right.name) {
Expand Down
@@ -0,0 +1,9 @@
var compactable = require('../compactable');

function overridesNonComponentShorthand(property1, property2) {
return property1.name in compactable &&
'overridesShorthands' in compactable[property1.name] &&
compactable[property1.name].overridesShorthands.indexOf(property2.name) > -1;
}

module.exports = overridesNonComponentShorthand;
10 changes: 10 additions & 0 deletions lib/optimizer/level-2/compacting/populate-components.js
Expand Up @@ -2,6 +2,9 @@ var compactable = require('../compactable');
var InvalidPropertyError = require('../invalid-property-error');

function populateComponents(properties, validator, warnings) {
var component;
var j, m;

for (var i = properties.length - 1; i >= 0; i--) {
var property = properties[i];
var descriptor = compactable[property.name];
Expand All @@ -12,6 +15,13 @@ function populateComponents(properties, validator, warnings) {

try {
property.components = descriptor.breakUp(property, compactable, validator);

if (descriptor.shorthandComponents) {
for (j = 0, m = property.components.length; j < m; j++) {
component = property.components[j];
component.components = compactable[component.name].breakUp(component, compactable, validator);
}
}
} catch (e) {
if (e instanceof InvalidPropertyError) {
property.components = []; // this will set property.unused to true below
Expand Down
21 changes: 15 additions & 6 deletions lib/optimizer/level-2/compacting/shorthand-compactor.js
Expand Up @@ -113,12 +113,18 @@ function invalidateOrCompact(properties, position, candidates, validator) {

function compactShortands(properties, validator) {
var candidates = {};
var descriptor;
var componentOf;
var property;
var i, l;
var j, m;

if (properties.length < 3)
return;

for (var i = 0, l = properties.length; i < l; i++) {
var property = properties[i];
for (i = 0, l = properties.length; i < l; i++) {
property = properties[i];

if (property.unused)
continue;

Expand All @@ -128,16 +134,19 @@ function compactShortands(properties, validator) {
if (property.block)
continue;

var descriptor = compactable[property.name];
descriptor = compactable[property.name];
if (!descriptor || !descriptor.componentOf)
continue;

if (property.shorthand) {
invalidateOrCompact(properties, i, candidates, validator);
} else {
var componentOf = descriptor.componentOf;
candidates[componentOf] = candidates[componentOf] || {};
candidates[componentOf][property.name] = property;
for (j = 0, m = descriptor.componentOf.length; j < m; j++) {
componentOf = descriptor.componentOf[j];

candidates[componentOf] = candidates[componentOf] || {};
candidates[componentOf][property.name] = property;
}
}
}

Expand Down

0 comments on commit 04b4958

Please sign in to comment.