From 47ecfb9fb4bee2586e27a74fc4c52f3537ff4fb5 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Fri, 10 Apr 2015 09:31:18 +0100 Subject: [PATCH] Re-adds `background` merging based on length. Adds merging `background` shorthand with longhands based on a result length so it's done only if the result is shorter. Also adds deep cloning in addition to shallow one. --- lib/properties/clone.js | 26 +++ lib/properties/override-compactor.js | 125 +++++++++++--- lib/properties/restore-shorthands.js | 5 + lib/properties/restore.js | 2 +- lib/properties/shallow-clone.js | 9 - lib/stringifier/one-time.js | 7 + test/fixtures/issue-466-min.css | 2 +- test/integration-test.js | 5 - test/properties/override-compacting-test.js | 172 ++++++++++++++------ test/properties/restore-shorthands-test.js | 34 ++++ 10 files changed, 306 insertions(+), 81 deletions(-) create mode 100644 lib/properties/clone.js delete mode 100644 lib/properties/shallow-clone.js diff --git a/lib/properties/clone.js b/lib/properties/clone.js new file mode 100644 index 00000000..1f4e73ce --- /dev/null +++ b/lib/properties/clone.js @@ -0,0 +1,26 @@ +var wrapSingle = require('./wrap-for-optimizing').single; + +function deep(property) { + var cloned = shallow(property); + for (var i = property.components.length - 1; i >= 0; i--) { + var component = shallow(property.components[i]); + component.value = property.components[i].value; + cloned.components.unshift(component); + } + + cloned.dirty = true; + cloned.value = property.value; + + return cloned; +} + +function shallow(property) { + var cloned = wrapSingle([[property.name, property.important, property.hack]]); + cloned.unused = false; + return cloned; +} + +module.exports = { + deep: deep, + shallow: shallow +}; diff --git a/lib/properties/override-compactor.js b/lib/properties/override-compactor.js index 1e8143e6..23442a57 100644 --- a/lib/properties/override-compactor.js +++ b/lib/properties/override-compactor.js @@ -1,6 +1,12 @@ var canOverride = require('./can-override'); var compactable = require('./compactable'); -var shallowClone = require('./shallow-clone'); +var deepClone = require('./clone').deep; +var shallowClone = require('./clone').shallow; +var restoreShorthands = require('./restore-shorthands'); + +var stringifyProperty = require('../stringifier/one-time').property; + +var MULTIPLEX_SEPARATOR = ','; // Used when searching for a component that matches property function nameMatchFilter(to) { @@ -29,22 +35,45 @@ function isComponentOf(shorthand, longhand) { return compactable[shorthand.name].components.indexOf(longhand.name) > -1; } -function overrideSimple(property, by) { +function overrideIntoMultiplex(property, by) { by.unused = true; - property.value = by.value; + + for (var i = 0, l = property.value.length; i < l; i++) { + property.value[i] = by.value; + } } -function overrideMultiplex(property, by) { +function overrideByMultiplex(property, by) { + // FIXME: we store component multiplex values and normal multiplex property values' differently + // e.g [[['no-repeat']], [['no-repeat']]] + // vs + // [['no-repeat'], [','], ['no-repeat']] + // We should rather use the latter as it's the standard way + by.unused = true; + property.value = []; - for (var i = 0; i < property.value.length; i++) { - property.value[i] = by.value; + for (var i = 0, propertyIndex = 0, l = by.value.length; i < l; i++) { + if (by.value[i] == MULTIPLEX_SEPARATOR) { + propertyIndex++; + continue; + } + + property.value[propertyIndex] = property.value[propertyIndex] || []; + property.value[propertyIndex].push(by.value[i]); } } +function overrideSimple(property, by) { + by.unused = true; + property.value = by.value; +} + function override(property, by) { - if (property.multiplex) - overrideMultiplex(property, by); + if (by.multiplex) + overrideByMultiplex(property, by); + else if (property.multiplex) + overrideIntoMultiplex(property, by); else overrideSimple(property, by); } @@ -53,7 +82,7 @@ function overrideShorthand(property, by) { by.unused = true; for (var i = 0, l = property.components.length; i < l; i++) { - override(property.components[i], by.components[i]); + override(property.components[i], by.components[i], property.multiplex); } } @@ -66,6 +95,68 @@ function hasInherits(property) { return false; } +function turnIntoMultiplex(property, size) { + property.multiplex = true; + + for (var i = 0, l = property.components.length; i < l; i++) { + var component = property.components[i]; + var value = component.value; + component.value = []; + + for (var j = 0; j < size; j++) { + component.value.push(value); + } + } +} + +function multiplexSize(component) { + var size = 0; + + for (var i = 0, l = component.value.length; i < l; i++) { + if (component.value[i] == MULTIPLEX_SEPARATOR) + size++; + } + + return size + 1; +} + +function lengthOf(property) { + var fakeAsArray = [[property.name]].concat(property.value); + return stringifyProperty([fakeAsArray], 0).length; +} + +function wouldResultInLongerValue(left, right) { + if (!left.multiplex && !right.multiplex || left.multiplex && right.multiplex) + return false; + + var multiplex = left.multiplex ? left : right; + var simple = left.multiplex ? right : left; + var component; + + var multiplexClone = deepClone(multiplex); + restoreShorthands([multiplexClone]); + + var simpleClone = deepClone(simple); + restoreShorthands([simpleClone]); + + var lengthBefore = lengthOf(multiplexClone) + 1 + lengthOf(simpleClone); + + if (left.multiplex) { + component = multiplexClone.components.filter(nameMatchFilter(simpleClone))[0]; + overrideIntoMultiplex(component, simpleClone); + } else { + component = simpleClone.components.filter(nameMatchFilter(multiplexClone))[0]; + turnIntoMultiplex(simpleClone, multiplexSize(multiplexClone)); + overrideByMultiplex(component, multiplexClone); + } + + restoreShorthands([simpleClone]); + + var lengthAfter = lengthOf(simpleClone); + + return lengthBefore < lengthAfter; +} + function compactOverrides(properties, compatibility) { var mayOverride, right, left, component; var i, j, k; @@ -83,12 +174,6 @@ function compactOverrides(properties, compatibility) { if (!left.shorthand && right.shorthand && isComponentOf(right, left)) { // maybe `left` can be overridden by `right` which is a shorthand? - // TODO: this is actually more complex, as in some cases it's better to incorporate the value, e.g. - // background:url(...); background-repeat:no-repeat,no-repeat; - // background:url(...) no-repeat,no-repeat; - if (!right.multiplex && left.multiplex) - continue; - if (!right.important && left.important) continue; @@ -99,10 +184,6 @@ function compactOverrides(properties, compatibility) { } } else if (left.shorthand && !right.shorthand && isComponentOf(left, right)) { // maybe `right` can be pulled into `left` which is a shorthand? - // TODO - see above - if (right.multiplex && !left.multiplex) - continue; - if (right.important && !left.important) continue; @@ -120,6 +201,12 @@ function compactOverrides(properties, compatibility) { if (component.value[0][0] != right.value[0][0] && (hasInherits(left) || hasInherits(right))) continue; + if (wouldResultInLongerValue(left, right)) + continue; + + if (!left.multiplex && right.multiplex) + turnIntoMultiplex(left, multiplexSize(right)); + override(component, right); left.dirty = true; } diff --git a/lib/properties/restore-shorthands.js b/lib/properties/restore-shorthands.js index 579038be..eff2f862 100644 --- a/lib/properties/restore-shorthands.js +++ b/lib/properties/restore-shorthands.js @@ -7,6 +7,11 @@ function restoreShorthands(properties) { if (descriptor && descriptor.shorthand && property.dirty && !property.unused) { var restored = descriptor.restore(property, compactable); + property.value = restored; + + if (!('all' in property)) + continue; + var current = property.all[property.position]; current.splice(1, current.length - 1); diff --git a/lib/properties/restore.js b/lib/properties/restore.js index 52e2e337..61af307d 100644 --- a/lib/properties/restore.js +++ b/lib/properties/restore.js @@ -1,4 +1,4 @@ -var shallowClone = require('./shallow-clone'); +var shallowClone = require('./clone').shallow; function background(property, compactable) { var components = property.components; diff --git a/lib/properties/shallow-clone.js b/lib/properties/shallow-clone.js deleted file mode 100644 index b810e90f..00000000 --- a/lib/properties/shallow-clone.js +++ /dev/null @@ -1,9 +0,0 @@ -var wrapSingle = require('./wrap-for-optimizing').single; - -function shallowClone(property) { - var cloned = wrapSingle([[property.name, property.important, property.hack]]); - cloned.unused = false; - return cloned; -} - -module.exports = shallowClone; diff --git a/lib/stringifier/one-time.js b/lib/stringifier/one-time.js index 2e8d8daa..f70551dc 100644 --- a/lib/stringifier/one-time.js +++ b/lib/stringifier/one-time.js @@ -17,6 +17,12 @@ function body(tokens) { return fakeContext.output.join(''); } +function property(tokens, position) { + var fakeContext = context(); + helpers.property(tokens, position, true, fakeContext); + return fakeContext.output.join(''); +} + function selectors(tokens) { var fakeContext = context(); helpers.selectors(tokens, fakeContext); @@ -31,6 +37,7 @@ function value(tokens, position) { module.exports = { body: body, + property: property, selectors: selectors, value: value }; diff --git a/test/fixtures/issue-466-min.css b/test/fixtures/issue-466-min.css index c4bc93e2..d0ad7ba9 100644 --- a/test/fixtures/issue-466-min.css +++ b/test/fixtures/issue-466-min.css @@ -1 +1 @@ -div{background:url(image.png);background:linear-gradient(rgba(0,0,0,.1),rgba(0,0,0,.5)),url(image.png)} +div{background:url(image.png);background-image:linear-gradient(rgba(0,0,0,.1),rgba(0,0,0,.5)),url(image.png)} diff --git a/test/integration-test.js b/test/integration-test.js index 11bf567e..a0077a13 100644 --- a/test/integration-test.js +++ b/test/integration-test.js @@ -2083,11 +2083,6 @@ title']{display:block}", '.one{background:50% no-repeat}.one{background-image:url(/img.png)}', '.one{background:url(/img.png)50% no-repeat}' ], - // TODO: restore multiplex merging - // 'merging color with backgrounds': [ - // 'p{background:red;background-image:url(1.png),url(2.png)}', - // 'p{background:url(1.png),url(2.png)red}' - // ], 'unknown @ rule': '@unknown "test";h1{color:red}', 'property without a value': [ 'a{color:}', diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js index a3e90926..69edfb57 100644 --- a/test/properties/override-compacting-test.js +++ b/test/properties/override-compacting-test.js @@ -8,7 +8,7 @@ var SourceTracker = require('../../lib/utils/source-tracker'); var Compatibility = require('../../lib/utils/compatibility'); var addOptimizationMetadata = require('../../lib/selectors/optimization-metadata'); -function _optimize(source, compatibility) { +function _optimize(source, compatibility, aggressiveMerging) { var tokens = new Tokenizer({ options: {}, sourceTracker: new SourceTracker(), @@ -16,8 +16,13 @@ function _optimize(source, compatibility) { }).toTokens(source); compatibility = new Compatibility(compatibility).toOptions(); + var options = { + aggressiveMerging: undefined === aggressiveMerging ? true : aggressiveMerging, + compatibility: compatibility, + shorthandCompacting: true + }; addOptimizationMetadata(tokens); - optimize(tokens[0][1], tokens[0][2], false, { compatibility: compatibility, shorthandCompacting: true }); + optimize(tokens[0][1], tokens[0][2], false, options); return tokens[0][2]; } @@ -41,23 +46,6 @@ vows.describe(optimize) ]); } }, - 'longhand then shorthand - multiplex then simple': { - 'topic': 'p{background-repeat:no-repeat,no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__}', - 'into': function (topic) { - assert.deepEqual(_optimize(topic), [ - [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']], - [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']] - ]); - } - }, - 'longhand then shorthand - simple then multiplex': { - 'topic': 'p{background-repeat:no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}', - 'into': function (topic) { - assert.deepEqual(_optimize(topic), [ - [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], [','], ['__ESCAPED_URL_CLEAN_CSS1__']] - ]); - } - }, 'shorthand then longhand': { 'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__ repeat;background-repeat:no-repeat}', 'into': function (topic) { @@ -83,23 +71,6 @@ vows.describe(optimize) ]); } }, - 'shorthand then longhand - multiple values': { - 'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__;background-repeat:no-repeat}', - 'into': function (topic) { - assert.deepEqual(_optimize(topic), [ - [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['no-repeat'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['no-repeat']] - ]); - } - }, - 'shorthand then longhand - single value then multi value': { - 'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background-repeat:no-repeat,no-repeat}', - 'into': function (topic) { - assert.deepEqual(_optimize(topic), [ - [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']], - [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']] - ]); - } - }, 'shorthand then longhand - disabled background size merging': { 'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background-size:50%}', 'into': function (topic) { @@ -192,16 +163,125 @@ vows.describe(optimize) [['background', true , false], ['__ESCAPED_URL_CLEAN_CSS1__'], ['red']] ]); } + }, + 'with aggressive off': { + 'topic': 'a{background:white;color:red;background:red}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic, null, false), [ + [['background', false , false], ['red']], + [['color', false , false], ['red']] + ]); + } + } + }) + .addBatch({ + 'shorthand then longhand multiplex': { + 'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['top'], ['left'], ['no-repeat'], [','], ['top'], ['left'], ['no-repeat']] + ]); + } + }, + 'shorthand multiplex then longhand': { + 'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__;background-repeat:no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['no-repeat'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['no-repeat']] + ]); + } + }, + 'longhand then shorthand multiplex': { + 'topic': 'p{background-repeat:no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], [','], ['__ESCAPED_URL_CLEAN_CSS1__']] + ]); + } + }, + 'longhand multiplex then shorthand': { + 'topic': 'p{background-repeat:no-repeat,no-repeat;background:__ESCAPED_URL_CLEAN_CSS0__}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']] + ]); + } + }, + 'multiplex longhand into multiplex shorthand123': { + 'topic': 'p{background:no-repeat,no-repeat;background-position:top left,bottom left}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['top'], ['left'], ['no-repeat'], [','], ['bottom'], ['left'], ['no-repeat']] + ]); + } + }, + 'not too long into multiplex #1': { + 'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['top'], ['left'], ['no-repeat'], [','], ['top'], ['left'], ['no-repeat']] + ]); + } + }, + 'not too long into multiplex #2': { + 'topic': 'p{background:repeat content-box;background-repeat:no-repeat,no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['no-repeat'], ['content-box'], [','], ['no-repeat'], ['content-box']] + ]); + } + }, + 'not too long into multiplex - twice': { + 'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat;background-image:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['top'], ['left'], ['no-repeat'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['top'], ['left'], ['no-repeat']] + ]); + } + }, + 'not too long into multiplex - over a property': { + 'topic': 'p{background:top left;background-repeat:no-repeat,no-repeat;background-image:__ESCAPED_URL_CLEAN_CSS0__}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['top'], ['left']], + [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']] + ]); + } + }, + 'too long into multiplex #1': { + 'topic': 'p{background:__ESCAPED_URL_CLEAN_CSS0__;background-repeat:no-repeat,no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__']], + [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']] + ]); + } + }, + 'too long into multiplex #2': { + 'topic': 'p{background:content-box padding-box;background-repeat:no-repeat,no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['content-box'], ['padding-box']], + [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']] + ]); + } + }, + 'too long into multiplex #3': { + 'topic': 'p{background:top left / 20px 20px;background-repeat:no-repeat,no-repeat}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['top'], ['left'], ['/'], ['20px'], ['20px']], + [['background-repeat', false , false], ['no-repeat'], [','], ['no-repeat']] + ]); + } + }, + 'background color into background': { + 'topic': 'p{background:red;background-repeat:__ESCAPED_URL_CLEAN_CSS0__,__ESCAPED_URL_CLEAN_CSS1__}', + 'into': function (topic) { + assert.deepEqual(_optimize(topic), [ + [['background', false , false], ['__ESCAPED_URL_CLEAN_CSS0__'], ['red'], [','], ['__ESCAPED_URL_CLEAN_CSS1__'], ['red']], + ]); + } } - - // 'aggressive off - overriddable': { - // 'topic': 'a{background:white;color:red;background:red}', - // 'into': function (topic) { - // assert.deepEqual(optimize(topic, false, false), [ - // [['color', false , false], ['red']], - // [['background', false , false], ['red']] - // ]); - // } - // } }) .export(module); diff --git a/test/properties/restore-shorthands-test.js b/test/properties/restore-shorthands-test.js index 07670cd2..7a852540 100644 --- a/test/properties/restore-shorthands-test.js +++ b/test/properties/restore-shorthands-test.js @@ -3,6 +3,7 @@ var assert = require('assert'); var wrapForOptimizing = require('../../lib/properties/wrap-for-optimizing').all; var populateComponents = require('../../lib/properties/populate-components'); +var shallowClone = require('../../lib/properties/clone').shallow; var restoreShorthands = require('../../lib/properties/restore-shorthands'); @@ -36,6 +37,39 @@ vows.describe(restoreShorthands) 'is same as source': function (properties) { assert.deepEqual(properties, ['/*comment */', [['background', false, false], ['url(image.png)']]]); } + }, + 'values': { + 'topic': function () { + var properties = [[['background', false, false], ['url(image.png)']]]; + var _properties = wrapForOptimizing(properties); + populateComponents(_properties); + + _properties[0].value = []; + _properties[0].dirty = true; + + restoreShorthands(_properties); + return _properties; + }, + 'updates value': function (_properties) { + assert.deepEqual(_properties[0].value, [['url(image.png)']]); + } + }, + 'in cloned without reference to `all`': { + 'topic': function () { + var properties = [[['background', false, false], ['url(image.png)']]]; + var _properties = wrapForOptimizing(properties); + populateComponents(_properties); + + var cloned = shallowClone(_properties[0]); + cloned.components = _properties[0].components; + cloned.dirty = true; + + restoreShorthands([cloned]); + return cloned; + }, + 'does not fail': function (cloned) { + assert.deepEqual(cloned.value, [['url(image.png)']]); + } } }) .export(module); -- 2.34.1