Skip to content

Commit

Permalink
Fixes #791 - resolves imports in-memory if possible.
Browse files Browse the repository at this point in the history
Why:

* When a hash is given as a source, some imports can be inlined
  in-memory taking styles from the hash.
  • Loading branch information
jakubpawlowicz committed Dec 26, 2016
1 parent 31f8563 commit 330682e
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 9 deletions.
1 change: 1 addition & 0 deletions History.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* Fixed issue [#686](https:/jakubpawlowicz/clean-css/issues/686) - adds rounding precision for all units.
* Fixed issue [#756](https:/jakubpawlowicz/clean-css/issues/756) - adds disabling font-weight optimizations.
* Fixed issue [#758](https:/jakubpawlowicz/clean-css/issues/758) - ignores rules with empty selector.
* Fixed issue [#791](https:/jakubpawlowicz/clean-css/issues/791) - resolves imports in-memory if possible.
* Fixed issue [#817](https:/jakubpawlowicz/clean-css/issues/817) - makes `off` disable rounding.
* Fixed issue [#818](https:/jakubpawlowicz/clean-css/issues/818) - disables `px` rounding by default.
* Fixed issue [#834](https:/jakubpawlowicz/clean-css/issues/834) - adds extra line break in nested blocks.
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@ new CleanCSS().minify({
});
```

Important note - any `@import` rules already present in the hash will be automatically resolved in memory.

### How to set a compatibility mode?

Compatibility settings are controlled by `--compatibility` switch (CLI) and `compatibility` option (library mode).
Expand Down
11 changes: 11 additions & 0 deletions lib/reader/is-already-loaded.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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;
38 changes: 29 additions & 9 deletions lib/reader/read-sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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');
Expand All @@ -13,6 +14,7 @@ var restoreImport = require('./restore-import');

var tokenize = require('../tokenizer/tokenize');
var Token = require('../tokenizer/token');
var Marker = require('../tokenizer/marker');
var isAbsoluteResource = require('../utils/is-absolute-resource');
var isImport = require('../utils/is-import');
var isRemoteResource = require('../utils/is-remote-resource');
Expand Down Expand Up @@ -62,7 +64,7 @@ function fromArray(input, context, parentInlinerContext, callback) {

if (isRemoteResource(uri)) {
accumulator[uri] = {
styles: restoreImport(uri, '') + ';'
styles: restoreImport(uri, '') + Marker.SEMICOLON
};
} else if (!fs.existsSync(absoluteUri) || !fs.statSync(absoluteUri).isFile()) {
context.errors.push('Ignoring "' + absoluteUri + '" as resource is missing.');
Expand All @@ -83,13 +85,15 @@ function fromHash(input, context, parentInlinerContext, callback) {
var tokens = [];
var newTokens = [];
var uri;
var realUri;
var source;
var parsedMap;
var rebasedMap;
var rebaseConfig;

for (uri in input) {
source = input[uri];
realUri = uri == UNKNOWN_URI ? undefined : uri;

if (source.sourceMap) {
parsedMap = typeof source.sourceMap == 'string' ?
Expand All @@ -98,12 +102,15 @@ function fromHash(input, context, parentInlinerContext, callback) {
rebasedMap = isRemoteResource(uri) ?
rebaseRemoteMap(parsedMap, uri) :
rebaseLocalMap(parsedMap, uri, context.options.rebaseTo);
context.inputSourceMapTracker.track(uri == UNKNOWN_URI ? undefined : uri, rebasedMap);
context.inputSourceMapTracker.track(realUri, rebasedMap);
}

context.source = uri !== UNKNOWN_URI ? uri : undefined;
context.sourcesContent[context.source] = source.styles;
context.sourcesContent[realUri] = source.styles;
}

for (uri in input) {
source = input[uri];
realUri = uri == UNKNOWN_URI ? undefined : uri;
rebaseConfig = {};

if (uri == UNKNOWN_URI) {
Expand All @@ -120,6 +127,8 @@ function fromHash(input, context, parentInlinerContext, callback) {
rebaseConfig.toBase = context.options.rebaseTo;
}

context.source = realUri;

newTokens = tokenize(source.styles, context);
newTokens = rebase(newTokens, context.options.rebase, context.validator, rebaseConfig);

Expand Down Expand Up @@ -189,6 +198,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);

if (inlinerContext.inlinedStylesheets.indexOf(uri) > -1) {
inlinerContext.warnings.push('Ignoring remote @import of "' + uri + '" as it has already been imported.');
Expand All @@ -198,7 +208,7 @@ function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {
inlinerContext.warnings.push('Ignoring remote @import of "' + uri + '" as no callback given and after other content.');
inlinerContext.sourceTokens = inlinerContext.sourceTokens.slice(1);
return doInlineImports(inlinerContext);
} else if (inlinerContext.localOnly) {
} else if (inlinerContext.localOnly && !isLoaded) {
inlinerContext.warnings.push('Skipping remote @import of "' + uri + '" as no callback given.');
inlinerContext.outputTokens = inlinerContext.outputTokens.concat(inlinerContext.sourceTokens.slice(0, 1));
inlinerContext.sourceTokens = inlinerContext.sourceTokens.slice(1);
Expand All @@ -216,7 +226,7 @@ function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {

inlinerContext.inlinedStylesheets.push(uri);

loadRemoteResource(uri, inlinerContext.inlinerOptions, function (error, importedStyles) {
function whenLoaded(error, importedStyles) {
var sourceHash = {};

if (error) {
Expand All @@ -233,6 +243,7 @@ function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {
styles: importedStyles
};

inlinerContext.externalContext.sourcesContent[uri] = importedStyles;
inlinerContext.isRemote = true;
fromHash(sourceHash, inlinerContext.externalContext, inlinerContext, function (importedTokens) {
importedTokens = wrapInMedia(importedTokens, mediaQuery, metadata);
Expand All @@ -242,7 +253,11 @@ function inlineRemoteStylesheet(uri, mediaQuery, metadata, inlinerContext) {

doInlineImports(inlinerContext);
});
});
}

return isLoaded ?
whenLoaded(null, inlinerContext.externalContext.sourcesContent[uri]) :
loadRemoteResource(uri, inlinerContext.inlinerOptions, whenLoaded);
}

function inlineLocalStylesheet(uri, mediaQuery, metadata, inlinerContext) {
Expand All @@ -255,10 +270,11 @@ function inlineLocalStylesheet(uri, mediaQuery, metadata, inlinerContext) {
var importedTokens;
var isAllowed = isAllowedResource(uri, false, inlinerContext.processImportFrom);
var sourceHash = {};
var isLoaded = isAlreadyLoaded(relativeToCurrentPath, 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 (!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.');
Expand All @@ -268,12 +284,16 @@ function inlineLocalStylesheet(uri, mediaQuery, metadata, inlinerContext) {
inlinerContext.warnings.push('Skipping local @import of "' + uri + '" as resource is not allowed.');
inlinerContext.outputTokens = inlinerContext.outputTokens.concat(inlinerContext.sourceTokens.slice(0, 1));
} else {
importedStyles = fs.readFileSync(absoluteUri, 'utf-8');
importedStyles = isLoaded ?
inlinerContext.externalContext.sourcesContent[relativeToCurrentPath] :
fs.readFileSync(absoluteUri, 'utf-8');
inlinerContext.inlinedStylesheets.push(absoluteUri);

sourceHash[relativeToCurrentPath] = {
styles: importedStyles
};
inlinerContext.externalContext.sourcesContent[relativeToCurrentPath] = importedStyles;

importedTokens = fromHash(sourceHash, inlinerContext.externalContext, inlinerContext, function (tokens) { return tokens; });
importedTokens = wrapInMedia(importedTokens, mediaQuery, metadata);

Expand Down
18 changes: 18 additions & 0 deletions test/module-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,24 @@ vows.describe('module tests').addBatch({
'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({
'main.css': {
styles: '@import url(test/fixtures/partials/one.css);\n@import url(http://127.0.0.1/test.css);'
},
'test/fixtures/partials/one.css': {
styles: '.one { background-color:#f00; }'
},
'http://127.0.0.1/test.css': {
styles: '.test { color: #000 }'
}
}, this.callback);
},
'gives right output without reading resources': function (minified) {
assert.equal(minified.styles, '.one{background-color:red}.test{color:#000}');
}
}
}
}).export(module);

0 comments on commit 330682e

Please sign in to comment.