Fixes #806 - skip optimizing variable properties.

Why:

* So far it's impossible to find out real property values so
  breaking up shorthands into components doesn't work.
* In theory one could optimize cases where all component values
  are given and one of them is a variable, but it's an additional
  complexity probably not worth the hassle.

diff --git a/History.md b/History.md
index 90ea48e..61881d7 100644 (file)
--- a/History.md
+++ b/History.md
@@ -25,6 +25,7 @@
 * Fixed issue [#791](https://github.com/jakubpawlowicz/clean-css/issues/791) - resolves imports in-memory if possible.
 * Fixed issue [#796](https://github.com/jakubpawlowicz/clean-css/issues/796) - semantic merging for `@media` blocks.
 * Fixed issue [#801](https://github.com/jakubpawlowicz/clean-css/issues/801) - smarter `@import` inlining.
+* Fixed issue [#806](https://github.com/jakubpawlowicz/clean-css/issues/806) - skip optimizing variable properties.
 * Fixed issue [#817](https://github.com/jakubpawlowicz/clean-css/issues/817) - makes `off` disable rounding.
 * Fixed issue [#818](https://github.com/jakubpawlowicz/clean-css/issues/818) - disables `px` rounding by default.
 * Fixed issue [#829](https://github.com/jakubpawlowicz/clean-css/issues/829) - adds more strict selector merging rules.

diff --git a/lib/optimizer/basic.js b/lib/optimizer/basic.js
index f06bbec..fd98b09 100644 (file)
--- a/lib/optimizer/basic.js
+++ b/lib/optimizer/basic.js
@@ -356,7 +356,7 @@ function optimizeBody(properties, context) {
   var property, name, type, value;
   var valueIsUrl;
   var propertyToken;
-  var _properties = wrapForOptimizing(properties);
+  var _properties = wrapForOptimizing(properties, true);
 
   for (var i = 0, l = _properties.length; i < l; i++) {
     property = _properties[i];

diff --git a/lib/properties/optimizer.js b/lib/properties/optimizer.js
index f7057a5..fd44ca1 100644 (file)
--- a/lib/properties/optimizer.js
+++ b/lib/properties/optimizer.js
@@ -193,7 +193,7 @@ function optimize(selector, properties, mergeAdjacent, withCompacting, context)
   var validator = context.validator;
   var warnings = context.warnings;
 
-  var _properties = wrapForOptimizing(properties);
+  var _properties = wrapForOptimizing(properties, false);
   populateComponents(_properties, validator, warnings);
   _optimize(_properties, mergeAdjacent, context.options.aggressiveMerging, validator);
 

diff --git a/lib/properties/wrap-for-optimizing.js b/lib/properties/wrap-for-optimizing.js
index 22ad546..2b9e1ca 100644 (file)
--- a/lib/properties/wrap-for-optimizing.js
+++ b/lib/properties/wrap-for-optimizing.js
@@ -12,20 +12,28 @@ var Match = {
   IMPORTANT_WORD_PATTERN: new RegExp('important$', 'i'),
   STAR: '*',
   SUFFIX_BANG_PATTERN: /!$/,
-  UNDERSCORE: '_'
+  UNDERSCORE: '_',
+  VARIABLE_REFERENCE_PATTERN: /var\(--.+\)$/
 };
 
-function wrapAll(properties) {
+function wrapAll(properties, includeVariable) {
   var wrapped = [];
   var single;
+  var property;
   var i;
 
   for (i = properties.length - 1; i >= 0; i--) {
-    if (properties[i][0] != Token.PROPERTY) {
+    property = properties[i];
+
+    if (property[0] != Token.PROPERTY) {
+      continue;
+    }
+
+    if (!includeVariable && someVariableReferences(property)) {
       continue;
     }
 
-    single = wrapSingle(properties[i]);
+    single = wrapSingle(property);
     single.all = properties;
     single.position = i;
     wrapped.unshift(single);
@@ -34,6 +42,30 @@ function wrapAll(properties) {
   return wrapped;
 }
 
+function someVariableReferences(property) {
+  var i, l;
+  var value;
+
+  // skipping `property` and property name tokens
+  for (i = 2, l = property.length; i < l; i++) {
+    value = property[i];
+
+    if (value[0] != Token.PROPERTY_VALUE) {
+      continue;
+    }
+
+    if (isVariableReference(value[1])) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+function isVariableReference(value) {
+  return Match.VARIABLE_REFERENCE_PATTERN.test(value);
+}
+
 function isMultiplex(property) {
   var value;
   var i, l;

diff --git a/test/optimizer/advanced-test.js b/test/optimizer/advanced-test.js
index d15e5bb..42c3d2f 100644 (file)
--- a/test/optimizer/advanced-test.js
+++ b/test/optimizer/advanced-test.js
@@ -102,4 +102,20 @@ vows.describe('advanced optimizer')
       ]
     })
   )
+  .addBatch(
+    optimizerContext('variables', {
+      'skip processing properties with variable values - border - 1st value': [
+        '.one{border:var(--color) solid 1px}',
+        '.one{border:var(--color) solid 1px}'
+      ],
+      'skip processing properties with variable values - border - 2nd value': [
+        '.one{border:red var(--style) 1px}',
+        '.one{border:red var(--style) 1px}'
+      ],
+      'skip processing properties with variable values - border - 3rd value': [
+        '.one{border:red solid var(--width)}',
+        '.one{border:red solid var(--width)}'
+      ]
+    })
+  )
   .export(module);

diff --git a/test/properties/wrap-for-optimizing-test.js b/test/properties/wrap-for-optimizing-test.js
index dc3f3fd..56458b6 100644 (file)
--- a/test/properties/wrap-for-optimizing-test.js
+++ b/test/properties/wrap-for-optimizing-test.js
@@ -14,7 +14,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '0'],
             ['property-value', '0']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -64,7 +64,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'color'],
             ['property-value', 'red']
           ]
-        ]);
+        ], true);
       },
       'has two wraps': function (wrapped) {
         assert.lengthOf(wrapped, 2);
@@ -82,7 +82,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'color'],
             ['property-value', 'red']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -101,7 +101,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '/'],
             ['property-value', '2px']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -124,7 +124,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', '--color'],
             ['property-value', 'red']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -136,6 +136,40 @@ vows.describe(wrapForOptimizing)
         assert.isFalse(wrapped[0].block);
       }
     },
+    'variable reference': {
+      'topic': function () {
+        return wrapForOptimizing([
+          [
+            'property',
+            ['property-name', 'color'],
+            ['property-value', 'var(--red)']
+          ]
+        ], true);
+      },
+      'has one wrap': function (wrapped) {
+        assert.lengthOf(wrapped, 1);
+      },
+      'has name': function (wrapped) {
+        assert.deepEqual(wrapped[0].name, 'color');
+      },
+      'has value': function (wrapped) {
+        assert.deepEqual(wrapped[0].value, [['property-value', 'var(--red)']]);
+      }
+    },
+    'variable reference when variables are ignored': {
+      'topic': function () {
+        return wrapForOptimizing([
+          [
+            'property',
+            ['property-name', 'color'],
+            ['property-value', 'var(--red)']
+          ]
+        ], false);
+      },
+      'has one wrap': function (wrapped) {
+        assert.lengthOf(wrapped, 0);
+      }
+    },
     'variable block': {
       'topic': function () {
         return wrapForOptimizing([
@@ -158,7 +192,7 @@ vows.describe(wrapForOptimizing)
               ]
             ]
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -183,7 +217,7 @@ vows.describe(wrapForOptimizing)
               ]
             ]
           ]
-        ]);
+        ], true);
       },
       'is a block': function (wrapped) {
         assert.isTrue(wrapped[0].block);
@@ -196,7 +230,7 @@ vows.describe(wrapForOptimizing)
             'property',
             ['property-name', 'margin']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -216,7 +250,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'margin'],
             ['property-value', '0!important']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -237,7 +271,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '0'],
             ['property-value', '!important']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -258,7 +292,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '0!'],
             ['property-value', 'important']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -280,7 +314,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '!'],
             ['property-value', 'important']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -303,7 +337,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', '_color'],
             ['property-value', 'red']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -323,7 +357,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', '*color'],
             ['property-value', 'red']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -343,7 +377,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'margin'],
             ['property-value', '0\\9']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -363,7 +397,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'margin'],
             ['property-value', '0']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -384,7 +418,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '0'],
             ['property-value', '\\9']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -404,7 +438,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'margin'],
             ['property-value', '0!ie']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -427,7 +461,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'margin'],
             ['property-value', '0 !ie']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -451,7 +485,7 @@ vows.describe(wrapForOptimizing)
             ['property-value', '0'],
             ['property-value', '!ie']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -474,7 +508,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'color'],
             ['property-value', 'red\\9!important']
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);
@@ -497,7 +531,7 @@ vows.describe(wrapForOptimizing)
             ['property-name', 'color', [[1, 2, undefined]]],
             ['property-value', 'red', [[1, 2, undefined]]]
           ]
-        ]);
+        ], true);
       },
       'has one wrap': function (wrapped) {
         assert.lengthOf(wrapped, 1);