Skip to content

Commit 6538c52

Browse files
lukeapagelukeapage
committed
Merge pull request #2246 from less/import-sequence
Attempt to fix import sequencing
2 parents 0b3accc + 793383e commit 6538c52

File tree

7 files changed

+181
-71
lines changed

7 files changed

+181
-71
lines changed

‎lib/less/tree/import.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,17 @@ Import.prototype.getPath = function () {
7373
}
7474
return null;
7575
};
76+
Import.prototype.isVariableImport = function () {
77+
var path = this.path;
78+
if (path instanceof URL) {
79+
path = path.value;
80+
}
81+
if (path instanceof Quoted) {
82+
return path.containsVariables();
83+
}
84+
85+
return true;
86+
};
7687
Import.prototype.evalForImport = function (context) {
7788
var path = this.path;
7889
if (path instanceof URL) {

‎lib/less/tree/quoted.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ Quoted.prototype.genCSS = function (context, output) {
2020
output.add(this.quote);
2121
}
2222
};
23+
Quoted.prototype.containsVariables = function() {
24+
return this.value.match(/(`([^`]+)`)|@\{([\w-]+)\}/);
25+
};
2326
Quoted.prototype.eval = function (context) {
2427
var that = this, value = this.value;
2528
var javascriptReplacement = function (_, exp) {

‎lib/less/tree/ruleset.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,16 @@ Ruleset.prototype.variables = function () {
218218
if (r instanceof Rule && r.variable === true) {
219219
hash[r.name] = r;
220220
}
221+
// when evaluating variables in an import statement, imports have not been eval'd
222+
// so we need to go inside import statements.
223+
if (r.type === "Import" && r.root) {
224+
var vars = r.root.variables();
225+
for(var name in vars) {
226+
if (vars.hasOwnProperty(name)) {
227+
hash[name] = vars[name];
228+
}
229+
}
230+
}
221231
return hash;
222232
}, {});
223233
}

‎lib/less/visitors/import-sequencer.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
function ImportSequencer(onSequencerEmpty) {
2+
this.imports = [];
3+
this.variableImports = [];
4+
this._onSequencerEmpty = onSequencerEmpty;
5+
}
6+
7+
ImportSequencer.prototype.addImport = function(callback) {
8+
var importSequencer = this,
9+
importItem = {
10+
callback: callback,
11+
args: null,
12+
isReady: false
13+
};
14+
this.imports.push(importItem);
15+
return function() {
16+
importItem.args = Array.prototype.slice.call(arguments, 0);
17+
importItem.isReady = true;
18+
importSequencer.tryRun();
19+
};
20+
};
21+
22+
ImportSequencer.prototype.addVariableImport = function(callback) {
23+
this.variableImports.push(callback);
24+
};
25+
26+
ImportSequencer.prototype.tryRun = function() {
27+
while(true) {
28+
while(this.imports.length > 0) {
29+
var importItem = this.imports[0];
30+
if (!importItem.isReady) {
31+
return;
32+
}
33+
this.imports = this.imports.slice(1);
34+
importItem.callback.apply(null, importItem.args);
35+
}
36+
if (this.variableImports.length === 0) {
37+
break;
38+
}
39+
var variableImport = this.variableImports[0];
40+
this.variableImports = this.variableImports.slice(1);
41+
variableImport();
42+
}
43+
if (this._onSequencerEmpty) {
44+
this._onSequencerEmpty();
45+
}
46+
};
47+
48+
module.exports = ImportSequencer;

‎lib/less/visitors/import-visitor.js‎

Lines changed: 101 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,17 @@
11
var contexts = require("../contexts"),
2-
Visitor = require("./visitor");
2+
Visitor = require("./visitor"),
3+
ImportSequencer = require("./import-sequencer");
4+
5+
var ImportVisitor = function(importer, finish) {
36

4-
var ImportVisitor = function(importer, finish, evalEnv, onceFileDetectionMap, recursionDetector) {
57
this._visitor = new Visitor(this);
68
this._importer = importer;
79
this._finish = finish;
8-
this.context = evalEnv || new contexts.Eval();
10+
this.context = new contexts.Eval();
911
this.importCount = 0;
10-
this.onceFileDetectionMap = onceFileDetectionMap || {};
12+
this.onceFileDetectionMap = {};
1113
this.recursionDetector = {};
12-
if (recursionDetector) {
13-
for(var fullFilename in recursionDetector) {
14-
if (recursionDetector.hasOwnProperty(fullFilename)) {
15-
this.recursionDetector[fullFilename] = true;
16-
}
17-
}
18-
}
14+
this._sequencer = new ImportSequencer();
1915
};
2016

2117
ImportVisitor.prototype = {
@@ -31,85 +27,121 @@ ImportVisitor.prototype = {
3127
}
3228

3329
this.isFinished = true;
34-
30+
this._sequencer.tryRun();
3531
if (this.importCount === 0) {
36-
this._finish(error);
32+
this._finish(error || this.error);
3733
}
3834
},
3935
visitImport: function (importNode, visitArgs) {
40-
var importVisitor = this,
41-
evaldImportNode,
42-
inlineCSS = importNode.options.inline;
36+
var inlineCSS = importNode.options.inline;
4337

4438
if (!importNode.css || inlineCSS) {
4539

46-
try {
47-
evaldImportNode = importNode.evalForImport(this.context);
48-
} catch(e){
49-
if (!e.filename) { e.index = importNode.index; e.filename = importNode.currentFileInfo.filename; }
50-
// attempt to eval properly and treat as css
51-
importNode.css = true;
52-
// if that fails, this error will be thrown
53-
importNode.error = e;
40+
var context = new contexts.Eval(this.context, this.context.frames.slice(0));
41+
var importParent = context.frames[0];
42+
43+
this.importCount++;
44+
if (importNode.isVariableImport()) {
45+
this._sequencer.addVariableImport(this.processImportNode.bind(this, importNode, context, importParent));
46+
} else {
47+
importNode = this.processImportNode(importNode, context, importParent);
48+
}
49+
}
50+
visitArgs.visitDeeper = false;
51+
return importNode;
52+
},
53+
processImportNode: function(importNode, context, importParent) {
54+
var evaldImportNode,
55+
inlineCSS = importNode.options.inline;
56+
57+
try {
58+
evaldImportNode = importNode.evalForImport(context);
59+
} catch(e){
60+
if (!e.filename) { e.index = importNode.index; e.filename = importNode.currentFileInfo.filename; }
61+
// attempt to eval properly and treat as css
62+
importNode.css = true;
63+
// if that fails, this error will be thrown
64+
importNode.error = e;
65+
}
66+
67+
if (evaldImportNode && (!evaldImportNode.css || inlineCSS)) {
68+
69+
if (evaldImportNode.options.multiple) {
70+
context.importMultiple = true;
5471
}
5572

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

61-
if (importNode.options.multiple) {
62-
context.importMultiple = true;
76+
var onImported = this.onImported.bind(this, evaldImportNode, context),
77+
sequencedOnImported = this._sequencer.addImport(onImported);
78+
79+
this._importer.push(evaldImportNode.getPath(), tryAppendLessExtension, evaldImportNode.currentFileInfo, evaldImportNode.options, sequencedOnImported);
80+
81+
for(var i = 0; i < importParent.rules.length; i++) {
82+
if (importParent.rules[i] === importNode) {
83+
importParent.rules[i] = evaldImportNode;
84+
break;
6385
}
86+
}
87+
importNode = evaldImportNode;
88+
} else {
89+
this.importCount--;
90+
}
91+
return importNode;
92+
},
93+
onImported: function (importNode, context, e, root, importedAtRoot, fullPath) {
94+
if (e) {
95+
if (!e.filename) {
96+
e.index = importNode.index; e.filename = importNode.currentFileInfo.filename;
97+
}
98+
this.error = e;
99+
}
64100

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

72-
var duplicateImport = importedAtRoot || fullPath in importVisitor.recursionDetector;
73-
if (!context.importMultiple) {
74-
if (duplicateImport) {
75-
importNode.skip = true;
76-
} else {
77-
importNode.skip = function() {
78-
if (fullPath in importVisitor.onceFileDetectionMap) {
79-
return true;
80-
}
81-
importVisitor.onceFileDetectionMap[fullPath] = true;
82-
return false;
83-
};
84-
}
105+
if (!context.importMultiple) {
106+
if (duplicateImport) {
107+
importNode.skip = true;
108+
} else {
109+
importNode.skip = function() {
110+
if (fullPath in importVisitor.onceFileDetectionMap) {
111+
return true;
85112
}
113+
importVisitor.onceFileDetectionMap[fullPath] = true;
114+
return false;
115+
};
116+
}
117+
}
86118

87-
var subFinish = function(e) {
88-
importVisitor.importCount--;
119+
if (root) {
120+
importNode.root = root;
121+
importNode.importedFilename = fullPath;
89122

90-
if (importVisitor.importCount === 0 && importVisitor.isFinished) {
91-
importVisitor._finish(e);
92-
}
93-
};
123+
if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
124+
importVisitor.recursionDetector[fullPath] = true;
94125

95-
if (root) {
96-
importNode.root = root;
97-
importNode.importedFilename = fullPath;
126+
var oldContext = this.context;
127+
this.context = context;
128+
try {
129+
this._visitor.visit(root);
130+
} catch (e) {
131+
this.error = e;
132+
}
133+
this.context = oldContext;
134+
}
135+
}
98136

99-
if (!inlineCSS && (context.importMultiple || !duplicateImport)) {
100-
importVisitor.recursionDetector[fullPath] = true;
101-
new ImportVisitor(importVisitor._importer, subFinish, context, importVisitor.onceFileDetectionMap, importVisitor.recursionDetector)
102-
.run(root);
103-
return;
104-
}
105-
}
137+
importVisitor.importCount--;
106138

107-
subFinish();
108-
});
139+
if (importVisitor.isFinished) {
140+
this._sequencer.tryRun();
141+
if (importVisitor.importCount === 0) {
142+
importVisitor._finish(importVisitor.error);
109143
}
110144
}
111-
visitArgs.visitDeeper = false;
112-
return importNode;
113145
},
114146
visitRule: function (ruleNode, visitArgs) {
115147
visitArgs.visitDeeper = false;

‎test/less/import-interpolation.less‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44

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

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

‎test/less/import/interpolation-vars.less‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
@in: "in";
2+
@terpolation: "terpolation";
3+
4+
// should be ignored because its already imported
5+
// and it uses a variable from the parent scope
6+
@import "import-@{my_theme}-e.less";

0 commit comments

Comments
 (0)