From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Mon, 26 Dec 2016 11:42:08 +0000 (+0100)
Subject: Reworks inlining so resources are loaded lazily.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=426127a;p=clean-css.git

Reworks inlining so resources are loaded lazily.

Why:

* when using a hash as an input, the sources it references
  should be processed lazily instead of a whole hash being
  serialized & tokenized eagerly.

This change addresses the need by:

* See #791 - in-memory imports require such lazy processing.
  The other way, we end up with duplicated rules, and have to rely
  on advanced processing to remove them.
---

diff --git a/lib/reader/is-already-loaded.js b/lib/reader/is-already-loaded.js
deleted file mode 100644
index 7a37dc38..00000000
--- a/lib/reader/is-already-loaded.js
+++ /dev/null
@@ -1,11 +0,0 @@
-var restoreImport = require('./restore-import');
-
-var Marker = require('../tokenizer/marker');
-
-function isAlreadyLoaded(uri, sourcesContent) {
-  // the second check is because clean-css accepts an array of input file paths
-  // which are internally transformed into imports
-  return uri in sourcesContent && sourcesContent[uri] != (restoreImport(uri, '') + Marker.SEMICOLON);
-}
-
-module.exports = isAlreadyLoaded;
diff --git a/lib/reader/read-sources.js b/lib/reader/read-sources.js
index d00ecba8..8715cfb4 100644
--- a/lib/reader/read-sources.js
+++ b/lib/reader/read-sources.js
@@ -4,7 +4,6 @@ var path = require('path');
 var applySourceMaps = require('./apply-source-maps');
 var extractImportUrlAndMedia = require('./extract-import-url-and-media');
 var isAllowedResource = require('./is-allowed-resource');
-var isAlreadyLoaded = require('./is-already-loaded');
 var loadOriginalSources = require('./load-original-sources');
 var loadRemoteResource = require('./load-remote-resource');
 var rebase = require('./rebase');
@@ -33,111 +32,106 @@ function readSources(input, context, callback) {
 
 function doReadSources(input, context, callback) {
   if (typeof input == 'string') {
-    return fromString(input, context, {}, callback);
+    return fromString(input, context, callback);
   } else if (Buffer.isBuffer(input)) {
-    return fromString(input.toString(), context, {}, callback);
+    return fromString(input.toString(), context, callback);
   } else if (Array.isArray(input)) {
-    return fromArray(input, context, {}, callback);
+    return fromArray(input, context, callback);
   } else if (typeof input == 'object') {
-    return fromHash(input, context, {}, callback);
+    return fromHash(input, context, callback);
   }
 }
 
-function fromString(input, context, parentInlinerContext, callback) {
-  var inputAsHash = {};
+function fromString(input, context, callback) {
+  context.source = undefined;
+  context.sourcesContent[undefined] = input;
+  context.stats.originalSize += input.length;
 
-  inputAsHash[UNKNOWN_URI] = {
-    styles: input,
-    sourceMap: (typeof context.options.sourceMap !== 'boolean') ? context.options.sourceMap : null
-  };
+  if (context.options.sourceMap && (typeof context.options.sourceMap !== 'boolean')) {
+    trackSourceMap(context.options.sourceMap, undefined, context);
+  }
 
-  return fromHash(inputAsHash, context, parentInlinerContext, callback);
+  return fromStyles(input, context, { processImports: context.options.processImport }, callback);
 }
 
-function fromArray(input, context, parentInlinerContext, callback) {
-  var currentPath = path.resolve('');
-  var inputAsHash = input.reduce(function (accumulator, uri) {
-    var absoluteUri = isAbsoluteResource(uri) || isRemoteResource(uri) ?
-      uri :
-      path.resolve(uri);
-    var relativeToCurrentPath;
-
-    if (isRemoteResource(uri)) {
-      accumulator[uri] = {
-        styles: restoreImport(uri, '') + Marker.SEMICOLON
-      };
-    } else if (!fs.existsSync(absoluteUri) || !fs.statSync(absoluteUri).isFile()) {
-      context.errors.push('Ignoring "' + absoluteUri + '" as resource is missing.');
-    } else {
-      relativeToCurrentPath = path.relative(currentPath, absoluteUri);
-      accumulator[relativeToCurrentPath] = {
-        styles: fs.readFileSync(absoluteUri, 'utf-8')
-      };
-    }
-
+function fromArray(input, context, callback) {
+  var inputAsImports = input.reduce(function (accumulator, uri) {
+    accumulator.push(restoreAsRelativeImport(uri));
     return accumulator;
-  }, {});
+  }, []);
 
-  return fromHash(inputAsHash, context, parentInlinerContext, callback);
+  return fromStyles(inputAsImports.join(''), context, { processImports: true }, callback);
 }
 
-function fromHash(input, context, parentInlinerContext, callback) {
-  var tokens = [];
-  var newTokens = [];
+function fromHash(input, context, callback) {
   var uri;
-  var realUri;
   var source;
-  var parsedMap;
-  var rebasedMap;
-  var rebaseConfig;
+  var inputAsImports = [];
 
   for (uri in input) {
     source = input[uri];
-    realUri = uri == UNKNOWN_URI ? undefined : uri;
+    inputAsImports.push(restoreAsRelativeImport(uri));
+
+    context.sourcesContent[uri] = source.styles;
 
     if (source.sourceMap) {
-      parsedMap = typeof source.sourceMap == 'string' ?
-        JSON.parse(source.sourceMap) :
-        source.sourceMap;
-      rebasedMap = isRemoteResource(uri) ?
-        rebaseRemoteMap(parsedMap, uri) :
-        rebaseLocalMap(parsedMap, uri, context.options.rebaseTo);
-      context.inputSourceMapTracker.track(realUri, rebasedMap);
+      trackSourceMap(source.sourceMap, uri, context);
     }
-
-    context.sourcesContent[realUri] = source.styles;
   }
 
-  for (uri in input) {
-    source = input[uri];
-    realUri = uri == UNKNOWN_URI ? undefined : uri;
-    rebaseConfig = {};
-
-    if (uri == UNKNOWN_URI) {
-      rebaseConfig.fromBase = path.resolve('');
-      rebaseConfig.toBase = context.options.rebaseTo;
-    } else if (isRemoteResource(uri)) {
-      rebaseConfig.fromBase = uri;
-      rebaseConfig.toBase = uri;
-    } else if (isAbsoluteResource(uri)) {
-      rebaseConfig.fromBase = path.dirname(uri);
-      rebaseConfig.toBase = context.options.rebaseTo;
-    } else {
-      rebaseConfig.fromBase = path.dirname(path.resolve(uri));
-      rebaseConfig.toBase = context.options.rebaseTo;
-    }
+  return fromStyles(inputAsImports.join(''), context, { processImports: true }, callback);
+}
 
-    context.source = realUri;
+function trackSourceMap(sourceMap, uri, context) {
+  var parsedMap = typeof sourceMap == 'string' ?
+      JSON.parse(sourceMap) :
+      sourceMap;
+  var rebasedMap = isRemoteResource(uri) ?
+    rebaseRemoteMap(parsedMap, uri) :
+    rebaseLocalMap(parsedMap, uri || UNKNOWN_URI, context.options.rebaseTo);
 
-    newTokens = tokenize(source.styles, context);
-    newTokens = rebase(newTokens, context.options.rebase, context.validator, rebaseConfig);
+  context.inputSourceMapTracker.track(uri, rebasedMap);
+}
 
-    tokens = tokens.concat(newTokens);
+function restoreAsRelativeImport(uri) {
+  var currentPath = path.resolve('');
+  var absoluteUri;
+  var relativeToCurrentPath;
+
+  if (isRemoteResource(uri)) {
+    return restoreImport(uri, '') + Marker.SEMICOLON;
+  } else {
+    absoluteUri = isAbsoluteResource(uri) ?
+      uri :
+      path.resolve(uri);
+    relativeToCurrentPath = path.relative(currentPath, absoluteUri);
 
-    context.stats.originalSize += source.styles.length;
+    return restoreImport(relativeToCurrentPath, '') + Marker.SEMICOLON;
   }
+}
 
-  return context.options.processImport ?
+function fromStyles(styles, context, parentInlinerContext, callback) {
+  var tokens;
+  var rebaseConfig = {};
+
+  if (!context.source) {
+    rebaseConfig.fromBase = path.resolve('');
+    rebaseConfig.toBase = context.options.rebaseTo;
+  } else if (isRemoteResource(context.source)) {
+    rebaseConfig.fromBase = context.source;
+    rebaseConfig.toBase = context.source;
+  } else if (isAbsoluteResource(context.source)) {
+    rebaseConfig.fromBase = path.dirname(context.source);
+    rebaseConfig.toBase = context.options.rebaseTo;
+  } else {
+    rebaseConfig.fromBase = path.dirname(path.resolve(context.source));
+    rebaseConfig.toBase = context.options.rebaseTo;
+  }
+
+  tokens = tokenize(styles, context);
+  tokens = rebase(tokens, context.options.rebase, context.validator, rebaseConfig);
+
+  return parentInlinerContext.processImports ?
     inlineImports(tokens, context, parentInlinerContext, callback) :
     callback(tokens);
 }
@@ -153,6 +147,7 @@ function inlineImports(tokens, externalContext, parentInlinerContext, callback)
     isRemote: parentInlinerContext.isRemote || false,
     localOnly: externalContext.localOnly,
     outputTokens: [],
+    processImports: parentInlinerContext.processImports,
     processImportFrom: externalContext.options.processImportFrom,
     rebaseTo: externalContext.options.rebaseTo,
     sourceTokens: tokens,
@@ -198,7 +193,7 @@ function inlineStylesheet(token, inlinerContext) {
 function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {
   var isAllowed = isAllowedResource(uri, true, inlinerContext.processImportFrom);
   var originalUri = uri;
-  var isLoaded = isAlreadyLoaded(uri, inlinerContext.externalContext.sourcesContent);
+  var isLoaded = uri in inlinerContext.externalContext.sourcesContent;
 
   if (inlinerContext.inlinedStylesheets.indexOf(uri) > -1) {
     inlinerContext.warnings.push('Ignoring remote @import of "' + uri + '" as it has already been imported.');
@@ -227,8 +222,6 @@ function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {
   inlinerContext.inlinedStylesheets.push(uri);
 
   function whenLoaded(error, importedStyles) {
-    var sourceHash = {};
-
     if (error) {
       inlinerContext.errors.push('Broken @import declaration of "' + uri + '" - ' + error);
 
@@ -239,19 +232,20 @@ function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {
       });
     }
 
-    sourceHash[originalUri] = {
-      styles: importedStyles
-    };
+    inlinerContext.processImports = inlinerContext.externalContext.options.processImport;
+    inlinerContext.isRemote = true;
 
+    inlinerContext.externalContext.source = originalUri;
     inlinerContext.externalContext.sourcesContent[uri] = importedStyles;
-    inlinerContext.isRemote = true;
-    fromHash(sourceHash, inlinerContext.externalContext, inlinerContext, function (importedTokens) {
+    inlinerContext.externalContext.stats.originalSize += importedStyles.length;
+
+    return fromStyles(importedStyles, inlinerContext.externalContext, inlinerContext, function (importedTokens) {
       importedTokens = wrapInMedia(importedTokens, mediaQuery, metadata);
 
       inlinerContext.outputTokens = inlinerContext.outputTokens.concat(importedTokens);
       inlinerContext.sourceTokens = inlinerContext.sourceTokens.slice(1);
 
-      doInlineImports(inlinerContext);
+      return doInlineImports(inlinerContext);
     });
   }
 
@@ -269,12 +263,11 @@ function inlineLocalStylesheet(uri, mediaQuery, metadata, inlinerContext) {
   var importedStyles;
   var importedTokens;
   var isAllowed = isAllowedResource(uri, false, inlinerContext.processImportFrom);
-  var sourceHash = {};
-  var isLoaded = isAlreadyLoaded(relativeToCurrentPath, inlinerContext.externalContext.sourcesContent);
+  var isLoaded = relativeToCurrentPath in inlinerContext.externalContext.sourcesContent;
 
   if (inlinerContext.inlinedStylesheets.indexOf(absoluteUri) > -1) {
     inlinerContext.warnings.push('Ignoring local @import of "' + uri + '" as it has already been imported.');
-  } else if (!isLoaded && !fs.existsSync(absoluteUri) || !fs.statSync(absoluteUri).isFile()) {
+  } else if (!isLoaded && (!fs.existsSync(absoluteUri) || !fs.statSync(absoluteUri).isFile())) {
     inlinerContext.errors.push('Ignoring local @import of "' + uri + '" as resource is missing.');
   } else if (!isAllowed && inlinerContext.afterContent) {
     inlinerContext.warnings.push('Ignoring local @import of "' + uri + '" as resource is not allowed and after other content.');
@@ -287,14 +280,15 @@ function inlineLocalStylesheet(uri, mediaQuery, metadata, inlinerContext) {
     importedStyles = isLoaded ?
       inlinerContext.externalContext.sourcesContent[relativeToCurrentPath] :
       fs.readFileSync(absoluteUri, 'utf-8');
+
     inlinerContext.inlinedStylesheets.push(absoluteUri);
+    inlinerContext.processImports = inlinerContext.externalContext.options.processImport;
 
-    sourceHash[relativeToCurrentPath] = {
-      styles: importedStyles
-    };
+    inlinerContext.externalContext.source = relativeToCurrentPath;
     inlinerContext.externalContext.sourcesContent[relativeToCurrentPath] = importedStyles;
+    inlinerContext.externalContext.stats.originalSize += importedStyles.length;
 
-    importedTokens = fromHash(sourceHash, inlinerContext.externalContext, inlinerContext, function (tokens) { return tokens; });
+    importedTokens = fromStyles(importedStyles, inlinerContext.externalContext, inlinerContext, function (tokens) { return tokens; });
     importedTokens = wrapInMedia(importedTokens, mediaQuery, metadata);
 
     inlinerContext.outputTokens = inlinerContext.outputTokens.concat(importedTokens);
diff --git a/test/module-test.js b/test/module-test.js
index 33c0dfcc..6fff4ddf 100644
--- a/test/module-test.js
+++ b/test/module-test.js
@@ -627,18 +627,20 @@ vows.describe('module tests').addBatch({
       }
     },
     'with remote paths': {
-      'topic': new CleanCSS().minify({
-        'http://127.0.0.1/styles.css': {
-          styles: 'div{background-image:url(image.png)}'
-        }
-      }),
+      'topic': function() {
+        return new CleanCSS().minify({
+          'http://127.0.0.1/styles.css': {
+            styles: 'div{background-image:url(image.png)}'
+          }
+        });
+      },
       'gives right output': function (minified) {
         assert.equal(minified.styles, 'div{background-image:url(http://127.0.0.1/image.png)}');
       }
     },
     'with already resolved imports': {
       'topic': function () {
-        new CleanCSS().minify({
+        new CleanCSS({ advanced: false }).minify({
           'main.css': {
             styles: '@import url(test/fixtures/partials/one.css);\n@import url(http://127.0.0.1/test.css);'
           },