Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix import sequencing #2246

Merged
merged 14 commits into from
Oct 26, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/less/tree/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,17 @@ Import.prototype.getPath = function () {
}
return null;
};
Import.prototype.isVariableImport = function () {
var path = this.path;
if (path instanceof URL) {
path = path.value;
}
if (path instanceof Quoted) {
return path.containsVariables();
}

return true;
};
Import.prototype.evalForImport = function (context) {
var path = this.path;
if (path instanceof URL) {
Expand Down
3 changes: 3 additions & 0 deletions lib/less/tree/quoted.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ Quoted.prototype.genCSS = function (context, output) {
output.add(this.quote);
}
};
Quoted.prototype.containsVariables = function() {
return this.value.match(/(`([^`]+)`)|@\{([\w-]+)\}/);
};
Quoted.prototype.eval = function (context) {
var that = this, value = this.value;
var javascriptReplacement = function (_, exp) {
Expand Down
10 changes: 10 additions & 0 deletions lib/less/tree/ruleset.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,16 @@ Ruleset.prototype.variables = function () {
if (r instanceof Rule && r.variable === true) {
hash[r.name] = r;
}
// when evaluating variables in an import statement, imports have not been eval'd
// so we need to go inside import statements.
if (r.type === "Import" && r.root) {
var vars = r.root.variables();
for(var name in vars) {
if (vars.hasOwnProperty(name)) {
hash[name] = vars[name];
}
}
}
return hash;
}, {});
}
Expand Down
48 changes: 48 additions & 0 deletions lib/less/visitors/import-sequencer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
function ImportSequencer(onSequencerEmpty) {
this.imports = [];
this.variableImports = [];
this._onSequencerEmpty = onSequencerEmpty;
}

ImportSequencer.prototype.addImport = function(callback) {
var importSequencer = this,
importItem = {
callback: callback,
args: null,
isReady: false
};
this.imports.push(importItem);
return function() {
importItem.args = Array.prototype.slice.call(arguments, 0);
importItem.isReady = true;
importSequencer.tryRun();
};
};

ImportSequencer.prototype.addVariableImport = function(callback) {
this.variableImports.push(callback);
};

ImportSequencer.prototype.tryRun = function() {
while(true) {
while(this.imports.length > 0) {
var importItem = this.imports[0];
if (!importItem.isReady) {
return;
}
this.imports = this.imports.slice(1);
importItem.callback.apply(null, importItem.args);
}
if (this.variableImports.length === 0) {
break;
}
var variableImport = this.variableImports[0];
this.variableImports = this.variableImports.slice(1);
variableImport();
}
if (this._onSequencerEmpty) {
this._onSequencerEmpty();
}
};

module.exports = ImportSequencer;
170 changes: 101 additions & 69 deletions lib/less/visitors/import-visitor.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
var contexts = require("../contexts"),
Visitor = require("./visitor");
Visitor = require("./visitor"),
ImportSequencer = require("./import-sequencer");

var ImportVisitor = function(importer, finish) {

var ImportVisitor = function(importer, finish, evalEnv, onceFileDetectionMap, recursionDetector) {
this._visitor = new Visitor(this);
this._importer = importer;
this._finish = finish;
this.context = evalEnv || new contexts.Eval();
this.context = new contexts.Eval();
this.importCount = 0;
this.onceFileDetectionMap = onceFileDetectionMap || {};
this.onceFileDetectionMap = {};
this.recursionDetector = {};
if (recursionDetector) {
for(var fullFilename in recursionDetector) {
if (recursionDetector.hasOwnProperty(fullFilename)) {
this.recursionDetector[fullFilename] = true;
}
}
}
this._sequencer = new ImportSequencer();
};

ImportVisitor.prototype = {
Expand All @@ -31,85 +27,121 @@ ImportVisitor.prototype = {
}

this.isFinished = true;

this._sequencer.tryRun();
if (this.importCount === 0) {
this._finish(error);
this._finish(error || this.error);
}
},
visitImport: function (importNode, visitArgs) {
var importVisitor = this,
evaldImportNode,
inlineCSS = importNode.options.inline;
var inlineCSS = importNode.options.inline;

if (!importNode.css || inlineCSS) {

try {
evaldImportNode = importNode.evalForImport(this.context);
} catch(e){
if (!e.filename) { e.index = importNode.index; e.filename = importNode.currentFileInfo.filename; }
// attempt to eval properly and treat as css
importNode.css = true;
// if that fails, this error will be thrown
importNode.error = e;
var context = new contexts.Eval(this.context, this.context.frames.slice(0));
var importParent = context.frames[0];

this.importCount++;
if (importNode.isVariableImport()) {
this._sequencer.addVariableImport(this.processImportNode.bind(this, importNode, context, importParent));
} else {
importNode = this.processImportNode(importNode, context, importParent);
}
}
visitArgs.visitDeeper = false;
return importNode;
},
processImportNode: function(importNode, context, importParent) {
var evaldImportNode,
inlineCSS = importNode.options.inline;

try {
evaldImportNode = importNode.evalForImport(context);
} catch(e){
if (!e.filename) { e.index = importNode.index; e.filename = importNode.currentFileInfo.filename; }
// attempt to eval properly and treat as css
importNode.css = true;
// if that fails, this error will be thrown
importNode.error = e;
}

if (evaldImportNode && (!evaldImportNode.css || inlineCSS)) {

if (evaldImportNode.options.multiple) {
context.importMultiple = true;
}

if (evaldImportNode && (!evaldImportNode.css || inlineCSS)) {
importNode = evaldImportNode;
this.importCount++;
var context = new contexts.Eval(this.context, this.context.frames.slice(0));
// try appending if we haven't determined if it is css or not
var tryAppendLessExtension = evaldImportNode.css === undefined;

if (importNode.options.multiple) {
context.importMultiple = true;
var onImported = this.onImported.bind(this, evaldImportNode, context),
sequencedOnImported = this._sequencer.addImport(onImported);

this._importer.push(evaldImportNode.getPath(), tryAppendLessExtension, evaldImportNode.currentFileInfo, evaldImportNode.options, sequencedOnImported);

for(var i = 0; i < importParent.rules.length; i++) {
if (importParent.rules[i] === importNode) {
importParent.rules[i] = evaldImportNode;
break;
}
}
importNode = evaldImportNode;
} else {
this.importCount--;
}
return importNode;
},
onImported: function (importNode, context, e, root, importedAtRoot, fullPath) {
if (e) {
if (!e.filename) {
e.index = importNode.index; e.filename = importNode.currentFileInfo.filename;
}
this.error = e;
}

// try appending if we haven't determined if it is css or not
var tryAppendLessExtension = importNode.css === undefined;
this._importer.push(importNode.getPath(), tryAppendLessExtension, importNode.currentFileInfo, importNode.options, function (e, root, importedAtRoot, fullPath) {
if (e && !e.filename) {
e.index = importNode.index; e.filename = importNode.currentFileInfo.filename;
}
var importVisitor = this,
inlineCSS = importNode.options.inline,
duplicateImport = importedAtRoot || fullPath in importVisitor.recursionDetector;

var duplicateImport = importedAtRoot || fullPath in importVisitor.recursionDetector;
if (!context.importMultiple) {
if (duplicateImport) {
importNode.skip = true;
} else {
importNode.skip = function() {
if (fullPath in importVisitor.onceFileDetectionMap) {
return true;
}
importVisitor.onceFileDetectionMap[fullPath] = true;
return false;
};
}
if (!context.importMultiple) {
if (duplicateImport) {
importNode.skip = true;
} else {
importNode.skip = function() {
if (fullPath in importVisitor.onceFileDetectionMap) {
return true;
}
importVisitor.onceFileDetectionMap[fullPath] = true;
return false;
};
}
}

var subFinish = function(e) {
importVisitor.importCount--;
if (root) {
importNode.root = root;
importNode.importedFilename = fullPath;

if (importVisitor.importCount === 0 && importVisitor.isFinished) {
importVisitor._finish(e);
}
};
if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
importVisitor.recursionDetector[fullPath] = true;

if (root) {
importNode.root = root;
importNode.importedFilename = fullPath;
var oldContext = this.context;
this.context = context;
try {
this._visitor.visit(root);
} catch (e) {
this.error = e;
}
this.context = oldContext;
}
}

if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
importVisitor.recursionDetector[fullPath] = true;
new ImportVisitor(importVisitor._importer, subFinish, context, importVisitor.onceFileDetectionMap, importVisitor.recursionDetector)
.run(root);
return;
}
}
importVisitor.importCount--;

subFinish();
});
if (importVisitor.isFinished) {
this._sequencer.tryRun();
if (importVisitor.importCount === 0) {
importVisitor._finish(importVisitor.error);
}
}
visitArgs.visitDeeper = false;
return importNode;
},
visitRule: function (ruleNode, visitArgs) {
visitArgs.visitDeeper = false;
Expand Down
4 changes: 2 additions & 2 deletions test/less/import-interpolation.less
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@

@import "import/import-@{in}@{terpolation}.less";

@in: "in";
@terpolation: "terpolation";
@import "import/interpolation-vars.less";

6 changes: 6 additions & 0 deletions test/less/import/interpolation-vars.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@in: "in";
@terpolation: "terpolation";

// should be ignored because its already imported
// and it uses a variable from the parent scope
@import "import-@{my_theme}-e.less";