From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Sat, 13 Dec 2014 11:53:02 +0000 (+0000)
Subject: Fixes issue #404 - removes all state sharing in API.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=574b64fec47da9536baef174364ee5fc368189fa;p=clean-css.git

Fixes issue #404 - removes all state sharing in API.
---

diff --git a/History.md b/History.md
index 9ce19b32..4f6e085d 100644
--- a/History.md
+++ b/History.md
@@ -24,6 +24,7 @@
 * Fixed issue [#373](https://github.com/GoalSmashers/clean-css/issues/373) - proper background shorthand merging.
 * Fixed issue [#395](https://github.com/GoalSmashers/clean-css/issues/395) - unescaped brackets in data URIs.
 * Fixed issue [#403](https://github.com/GoalSmashers/clean-css/issues/403) - tracking input files in source maps.
+* Fixed issue [#404](https://github.com/GoalSmashers/clean-css/issues/404) - no state sharing in API.
 * Refixed issue [#304](https://github.com/GoalSmashers/clean-css/issues/304) - background position merging.
 
 [2.2.19 / 2014-11-20](https://github.com/jakubpawlowicz/clean-css/compare/v2.2.18...v2.2.19)
diff --git a/README.md b/README.md
index efa2e33d..04bbf07f 100644
--- a/README.md
+++ b/README.md
@@ -37,6 +37,7 @@ npm install clean-css
 * `noRebase` became `rebase` - make sure to reverse the value;
 * no longer possible to use `CleanCSS` as a function as `new CleanCSS` is always required;
 * `minify` method returns a hash instead of a string now, so use `new CleanCSS().minify(source).styles` instead of `new CleanCSS().minify(source)`. This change is due to addition of source-maps.
+* `stats`, `errors`, and `warnings` are now a properties of a hash returned by `minify` method (see above) instead of CleanCSS instance.
 
 ### How to upgrade clean-css from 1.x to 2.x?
 
diff --git a/bin/cleancss b/bin/cleancss
index 6ebd15ec..40ef2d84 100755
--- a/bin/cleancss
+++ b/bin/cleancss
@@ -136,16 +136,16 @@ if (options.sources) {
 function minify(data) {
   new CleanCSS(cleanOptions).minify(data, function(errors, minified) {
     if (cleanOptions.debug) {
-      console.error('Original: %d bytes', this.stats.originalSize);
-      console.error('Minified: %d bytes', this.stats.minifiedSize);
-      console.error('Efficiency: %d%', ~~(this.stats.efficiency * 10000) / 100.0);
-      console.error('Time spent: %dms', this.stats.timeSpent);
+      console.error('Original: %d bytes', minified.stats.originalSize);
+      console.error('Minified: %d bytes', minified.stats.minifiedSize);
+      console.error('Efficiency: %d%', ~~(minified.stats.efficiency * 10000) / 100.0);
+      console.error('Time spent: %dms', minified.stats.timeSpent);
     }
 
-    outputFeedback(this.errors, true);
-    outputFeedback(this.warnings);
+    outputFeedback(minified.errors, true);
+    outputFeedback(minified.warnings);
 
-    if (this.errors.length > 0)
+    if (minified.errors.length > 0)
       process.exit(1);
 
     if (minified.sourceMap) {
diff --git a/lib/clean.js b/lib/clean.js
index 5a739c7a..3f129fce 100644
--- a/lib/clean.js
+++ b/lib/clean.js
@@ -41,75 +41,79 @@ var CleanCSS = module.exports = function CleanCSS(options) {
     sourceMap: options.sourceMap,
     target: options.target
   };
+};
 
-  this.stats = {};
-  this.context = {
+CleanCSS.prototype.minify = function(data, callback) {
+  var context = {
+    stats: {},
     errors: [],
     warnings: [],
-    debug: options.debug,
+    options: this.options,
+    debug: this.options.debug,
     sourceTracker: new SourceTracker()
   };
-  this.errors = this.context.errors;
-  this.warnings = this.context.warnings;
-};
-
-CleanCSS.prototype.minify = function(data, callback) {
-  var options = this.options;
-  var self = this;
 
   if (Buffer.isBuffer(data))
     data = data.toString();
 
-  if (options.processImport || data.indexOf('@shallow') > 0) {
+  if (context.options.processImport || data.indexOf('@shallow') > 0) {
     // inline all imports
     var runner = callback ?
       process.nextTick :
       function (callback) { return callback(); };
 
     return runner(function () {
-      return new ImportInliner(self.context, options.inliner, options.rebase).process(data, {
+      return new ImportInliner(context, context.options.inliner, context.options.rebase).process(data, {
         localOnly: !callback,
-        root: options.root || process.cwd(),
-        relativeTo: options.relativeTo,
-        whenDone: runMinifier(callback, self)
+        root: context.options.root || process.cwd(),
+        relativeTo: context.options.relativeTo,
+        whenDone: runMinifier(callback, context)
       });
     });
   } else {
-    return runMinifier(callback, self)(data);
+    return runMinifier(callback, context)(data);
   }
 };
 
-function runMinifier(callback, self) {
+function runMinifier(callback, context) {
   function whenSourceMapReady (data) {
-    data = self.options.debug ?
-      minifyWithDebug(self, data) :
-      minify.call(self, data);
+    data = context.options.debug ?
+      minifyWithDebug(context, data) :
+      minify(context, data);
+    data = withMetadata(context, data);
 
     return callback ?
-      callback.call(self, self.context.errors.length > 0 ? self.context.errors : null, data) :
+      callback.call(null, context.errors.length > 0 ? context.errors : null, data) :
       data;
   }
 
   return function (data) {
-    if (self.options.sourceMap) {
-      self.inputSourceMapTracker = new InputSourceMapTracker(self.options, self.context);
-      return self.inputSourceMapTracker.track(data, function () { return whenSourceMapReady(data); });
+    if (context.options.sourceMap) {
+      context.inputSourceMapTracker = new InputSourceMapTracker(context.options, context);
+      return context.inputSourceMapTracker.track(data, function () { return whenSourceMapReady(data); });
     } else {
       return whenSourceMapReady(data);
     }
   };
 }
 
-function minifyWithDebug(self, data) {
+function withMetadata(context, data) {
+  data.stats = context.stats;
+  data.errors = context.errors;
+  data.warnings = context.warnings;
+  return data;
+}
+
+function minifyWithDebug(context, data) {
   var startedAt = process.hrtime();
-  self.stats.originalSize = self.context.sourceTracker.removeAll(data).length;
+  context.stats.originalSize = context.sourceTracker.removeAll(data).length;
 
-  data = minify.call(self, data);
+  data = minify(context, data);
 
   var elapsed = process.hrtime(startedAt);
-  self.stats.timeSpent = ~~(elapsed[0] * 1e3 + elapsed[1] / 1e6);
-  self.stats.efficiency = 1 - data.styles.length / self.stats.originalSize;
-  self.stats.minifiedSize = data.styles.length;
+  context.stats.timeSpent = ~~(elapsed[0] * 1e3 + elapsed[1] / 1e6);
+  context.stats.efficiency = 1 - data.styles.length / context.stats.originalSize;
+  context.stats.minifiedSize = data.styles.length;
 
   return data;
 }
@@ -124,10 +128,9 @@ function benchmark(runner) {
   };
 }
 
-function minify(data) {
-  var options = this.options;
-  var context = this.context;
-  var sourceMapTracker = this.inputSourceMapTracker;
+function minify(context, data) {
+  var options = context.options;
+  var sourceMapTracker = context.inputSourceMapTracker;
 
   var commentsProcessor = new CommentsProcessor(context, options.keepSpecialComments, options.keepBreaks, options.sourceMap);
   var expressionsProcessor = new ExpressionsProcessor(options.sourceMap);
diff --git a/test/module-test.js b/test/module-test.js
index be02e4d7..64dc0a74 100644
--- a/test/module-test.js
+++ b/test/module-test.js
@@ -32,8 +32,8 @@ vows.describe('module tests').addBatch({
     topic: function() {
       new CleanCSS().minify('a{color:#f00}', this.callback);
     },
-    'should correctly set context': function() {
-      assert.equal(true, this instanceof CleanCSS);
+    'should not set context': function() {
+      assert.equal(false, this instanceof CleanCSS);
     },
     'should yield no error': function(errors, minified) {
       /* jshint unused: false */
@@ -53,174 +53,125 @@ vows.describe('module tests').addBatch({
     }
   },
   'no debug': {
-    topic: function() {
-      var minifier = new CleanCSS();
-      minifier.minify('a{ color: #f00 }');
-      return minifier;
-    },
-    'should not populate stats hash': function(minifier) {
-      assert.deepEqual({}, minifier.stats);
+    'topic': new CleanCSS().minify('a{ color: #f00 }'),
+    'should not populate stats hash': function (error, minified) {
+      assert.deepEqual({}, minified.stats);
     }
   },
   'debug': {
-    topic: function() {
-      var minifier = new CleanCSS({ debug: true });
-      minifier.minify('a{ color: #f00 }');
-      return minifier;
-    },
-    'should give time taken': function(minifier) {
-      assert.isNumber(minifier.stats.timeSpent);
+    'topic': new CleanCSS({ debug: true }).minify('a{ color: #f00 }'),
+    'should give time taken': function (error, minified) {
+      assert.isNumber(minified.stats.timeSpent);
     },
-    'should give original size': function(minifier) {
-      assert.equal(minifier.stats.originalSize, 16);
+    'should give original size': function (error, minified) {
+      assert.equal(minified.stats.originalSize, 16);
     },
-    'should give minified size': function(minifier) {
-      assert.equal(minifier.stats.minifiedSize, 12);
+    'should give minified size': function (error, minified) {
+      assert.equal(minified.stats.minifiedSize, 12);
     },
-    'should give efficiency': function(minifier) {
-      assert.equal(minifier.stats.efficiency, 0.25);
+    'should give efficiency': function (error, minified) {
+      assert.equal(minified.stats.efficiency, 0.25);
     }
   },
   'no warnings': {
-    topic: function() {
-      var minifier = new CleanCSS();
-      minifier.minify('a{color:red}');
-      return minifier;
-    },
-    'if no reasons given': function(minifier) {
-      assert.deepEqual(minifier.warnings, []);
+    'topic': new CleanCSS().minify('a{ color: #f00 }'),
+    'if no reasons given': function (error, minified) {
+      assert.deepEqual(minified.warnings, []);
     }
   },
   'warnings': {
-    topic: function() {
-      var minifier = new CleanCSS({
-          root: 'test/data',
-          target: 'custom-warnings.css'
-        });
-      minifier.minify('a{color:red}');
-      return minifier;
-    },
-    'if both root and output used reasons given': function(minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.match(minifier.warnings[0], /Both 'root' and output file given/);
+    'topic': new CleanCSS({ root: 'test/data', target: 'custom-warnings.css' }).minify('a{color:red}'),
+    'if both root and output used reasons given': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.match(minified.warnings[0], /Both 'root' and output file given/);
     }
   },
   'warnings on extra closing brace': {
-    topic: function() {
-      var minifier = new CleanCSS();
-      var minified = minifier.minify('a{display:block}}');
-      this.callback(null, minified, minifier);
-    },
-    'should minify correctly': function(error, minified) {
+    'topic': new CleanCSS().minify('a{display:block}}'),
+    'should minify correctly': function (error, minified) {
       assert.equal(minified.styles, 'a{display:block}');
     },
-    'should raise no errors': function(error, minified, minifier) {
-      assert.equal(minifier.errors.length, 0);
+    'should raise no errors': function (error, minified) {
+      assert.equal(minified.errors.length, 0);
     },
-    'should raise one warning': function(error, minified, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.equal(minifier.warnings[0], 'Unexpected \'}\' in \'a{display:block}}\'. Ignoring.');
+    'should raise one warning': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.equal(minified.warnings[0], 'Unexpected \'}\' in \'a{display:block}}\'. Ignoring.');
     }
   },
   'warnings on unexpected body': {
-    topic: function() {
-      var minifier = new CleanCSS();
-      var minified = minifier.minify('a{display:block}color:#535353}p{color:red}');
-      this.callback(null, minified, minifier);
-    },
-    'should minify correctly': function(error, minified) {
+    'topic': new CleanCSS().minify('a{display:block}color:#535353}p{color:red}'),
+    'should minify correctly': function (error, minified) {
       assert.equal(minified.styles, 'a{display:block}p{color:red}');
     },
-    'should raise no errors': function(error, minified, minifier) {
-      assert.equal(minifier.errors.length, 0);
+    'should raise no errors': function (error, minified) {
+      assert.equal(minified.errors.length, 0);
     },
-    'should raise one warning': function(error, minified, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.equal(minifier.warnings[0], 'Unexpected content: \'color:#535353}\'. Ignoring.');
+    'should raise one warning': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.equal(minified.warnings[0], 'Unexpected content: \'color:#535353}\'. Ignoring.');
     }
   },
   'warnings on invalid properties': {
-    topic: function() {
-      var minifier = new CleanCSS();
-      var minified = minifier.minify('a{color:}');
-      this.callback(null, minified, minifier);
-    },
-    'should minify correctly': function(error, minified) {
+    'topic': new CleanCSS().minify('a{color:}'),
+    'should minify correctly': function (error, minified) {
       assert.equal(minified.styles, '');
     },
-    'should raise no errors': function(error, minified, minifier) {
-      assert.equal(minifier.errors.length, 0);
+    'should raise no errors': function (error, minified) {
+      assert.equal(minified.errors.length, 0);
     },
-    'should raise one warning': function(error, minified, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.equal(minifier.warnings[0], 'Empty property \'color\' inside \'a\' selector. Ignoring.');
+    'should raise one warning': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.equal(minified.warnings[0], 'Empty property \'color\' inside \'a\' selector. Ignoring.');
     }
   },
   'warnings on broken urls': {
-    topic: function () {
-      var minifier = new CleanCSS();
-      var minified = minifier.minify('a{background:url(image/}');
-      this.callback(null, minified, minifier);
-    },
-    'should output correct content': function(error, minified) {
+    'topic': new CleanCSS().minify('a{background:url(image/}'),
+    'should output correct content': function (error, minified) {
       assert.equal(minified.styles, 'a{background:url(image/}');
     },
-    'should raise no errors': function(error, minified, minifier) {
-      assert.equal(minifier.errors.length, 0);
+    'should raise no errors': function (error, minified) {
+      assert.equal(minified.errors.length, 0);
     },
-    'should raise one warning': function(error, minified, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.equal(minifier.warnings[0], 'Broken URL declaration: \'url(image/\'.');
+    'should raise one warning': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.equal(minified.warnings[0], 'Broken URL declaration: \'url(image/\'.');
     }
   },
   'warnings on broken imports': {
-    topic: function () {
-      var minifier = new CleanCSS();
-      var minified = minifier.minify('@impor');
-      this.callback(null, minified, minifier);
-    },
-    'should output correct content': function(error, minified) {
+    'topic': new CleanCSS().minify('@impor'),
+    'should output correct content': function (error, minified) {
       assert.equal(minified.styles, '');
     },
-    'should raise no errors': function(error, minified, minifier) {
-      assert.equal(minifier.errors.length, 0);
+    'should raise no errors': function (error, minified) {
+      assert.equal(minified.errors.length, 0);
     },
-    'should raise one warning': function(error, minified, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.equal(minifier.warnings[0], 'Broken declaration: \'@impor\'.');
+    'should raise one warning': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.equal(minified.warnings[0], 'Broken declaration: \'@impor\'.');
     }
   },
   'warnings on broken comments': {
-    topic: function () {
-      var minifier = new CleanCSS();
-      var minified = minifier.minify('a{}/* ');
-      this.callback(null, minified, minifier);
-    },
-    'should output correct content': function(error, minified) {
+    'topic': new CleanCSS().minify('a{}/* '),
+    'should output correct content': function (error, minified) {
       assert.equal(minified.styles, '');
     },
-    'should raise no errors': function(error, minified, minifier) {
-      assert.equal(minifier.errors.length, 0);
+    'should raise no errors': function (error, minified) {
+      assert.equal(minified.errors.length, 0);
     },
-    'should raise one warning': function(error, minified, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.equal(minifier.warnings[0], 'Broken comment: \'/* \'.');
+    'should raise one warning': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.equal(minified.warnings[0], 'Broken comment: \'/* \'.');
     }
   },
   'no errors': {
-    topic: function() {
-      var minifier = new CleanCSS();
-      minifier.minify('a{color:red}');
-      return minifier;
-    },
-    'if no reasons given': function(minifier) {
-      assert.deepEqual(minifier.errors, []);
+    'topic': new CleanCSS().minify('a{color:red}'),
+    'if no reasons given': function (error, minified) {
+      assert.deepEqual(minified.errors, []);
     }
   },
   'errors': {
-    topic: function() {
-      return new CleanCSS();
-    },
+    'topic': new CleanCSS(),
     'if both root and output used reasons given': function(minifier) {
       assert.doesNotThrow(function() {
         minifier.minify('@import url(/some/fake/file);', function(errors) {
@@ -230,6 +181,16 @@ vows.describe('module tests').addBatch({
       });
     }
   },
+  'errors when re-running minification': {
+    'topic': new CleanCSS(),
+    'if both root and output used reasons given': function (minifier) {
+      minifier.minify('@import url(/some/fake/file);');
+      minifier.minify('@import url(/some/fake/file);', function(errors) {
+        assert.equal(errors.length, 1);
+        assert.equal(errors[0], 'Broken @import declaration of "/some/fake/file"');
+      });
+    }
+  },
   'buffer passed in': {
     'topic': function() {
       return new CleanCSS().minify(new Buffer('@import url(test/data/partials/one.css);'));
diff --git a/test/protocol-imports-test.js b/test/protocol-imports-test.js
index 4e951c5c..bddb0602 100644
--- a/test/protocol-imports-test.js
+++ b/test/protocol-imports-test.js
@@ -408,18 +408,16 @@ vows.describe('protocol imports').addBatch({
         .get('/remote.css')
         .reply(200, 'div{padding:0}');
 
-      var minifier = new CleanCSS();
-      var minified = minifier.minify(source);
-      this.callback(null, minifier, minified);
+      return new CleanCSS().minify(source);
     },
-    'should not raise errors': function(error, minifier) {
-      assert.isEmpty(minifier.errors);
+    'should not raise errors': function (error, minified) {
+      assert.isEmpty(minified.errors);
     },
-    'should raise warnings': function(error, minifier) {
-      assert.equal(minifier.warnings.length, 1);
-      assert.match(minifier.warnings[0], /no callback given/);
+    'should raise warnings': function (error, minified) {
+      assert.equal(minified.warnings.length, 1);
+      assert.match(minified.warnings[0], /no callback given/);
     },
-    'should process @import': function(error, minifier, minified) {
+    'should process @import': function (error, minified) {
       assert.equal(minified.styles, '@import url(http://127.0.0.1/remote.css);.one{color:red}');
     },
     teardown: function() {