Merge pull request #2246 from less/import-sequence

Attempt to fix import sequencing

lib/less/tree/import.js

@@ -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) {

lib/less/tree/quoted.js

@@ -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) {

lib/less/tree/ruleset.js

@@ -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;
         }, {});
     }

lib/less/visitors/import-sequencer.js

@@ -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;

lib/less/visitors/import-visitor.js

@@ -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 = {
@@ -31,86 +27,122 @@ 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];
 
-            if (evaldImportNode && (!evaldImportNode.css || inlineCSS)) {
-                importNode = evaldImportNode;
-                this.importCount++;
-                var context = new contexts.Eval(this.context, this.context.frames.slice(0));
-
-                if (importNode.options.multiple) {
-                    context.importMultiple = true;
-                }
-
-                // 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 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;
-                            };
-                        }
-                    }
-
-                    var subFinish = function(e) {
-                        importVisitor.importCount--;
-
-                        if (importVisitor.importCount === 0 && importVisitor.isFinished) {
-                            importVisitor._finish(e);
-                        }
-                    };
-
-                    if (root) {
-                        importNode.root = root;
-                        importNode.importedFilename = fullPath;
-
-                        if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
-                            importVisitor.recursionDetector[fullPath] = true;
-                            new ImportVisitor(importVisitor._importer, subFinish, context, importVisitor.onceFileDetectionMap, importVisitor.recursionDetector)
-                                .run(root);
-                            return;
-                        }
-                    }
-
-                    subFinish();
-                });
+            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;
+            }
+
+            // try appending if we haven't determined if it is css or not
+            var tryAppendLessExtension = evaldImportNode.css === undefined;
+
+            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;
+        }
+
+        var importVisitor = this,
+            inlineCSS = importNode.options.inline,
+            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 (root) {
+            importNode.root = root;
+            importNode.importedFilename = fullPath;
+
+            if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
+                importVisitor.recursionDetector[fullPath] = true;
+
+                var oldContext = this.context;
+                this.context = context;
+                try {
+                    this._visitor.visit(root);
+                } catch (e) {
+                    this.error = e;
+                }
+                this.context = oldContext;
+            }
+        }
+
+        importVisitor.importCount--;
+
+        if (importVisitor.isFinished) {
+            this._sequencer.tryRun();
+            if (importVisitor.importCount === 0) {
+                importVisitor._finish(importVisitor.error);
+            }
+        }
+    },
     visitRule: function (ruleNode, visitArgs) {
         visitArgs.visitDeeper = false;
         return ruleNode;

test/less/import-interpolation.less

@@ -4,5 +4,5 @@
 
 @import "import/import-@{in}@{terpolation}.less";
 
-@in: "in";
-@terpolation: "terpolation";
+@import "import/interpolation-vars.less";
+

test/less/import/interpolation-vars.less

@@ -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";