[FIX] Variables: resolve url values in variables

Author: petermuessig

lib/index.js

@@ -288,10 +288,13 @@ Builder.prototype.build = function(options) {
 					// generate the skeleton-css and the less-variables
 					const CSSVariablesCollectorPlugin = require("./plugin/css-variables-collector");
 					const oCSSVariablesCollector = new CSSVariablesCollectorPlugin(config);
+					const CSSVariablesUrlResolverPlugin = require("./plugin/css-variables-urlresolver");
+					const oCSSVariablesUrlResolverPlugin = new CSSVariablesUrlResolverPlugin(config);
 					result.cssSkeleton = tree.toCSS(Object.assign({}, options.compiler, {
-						plugins: [oCSSVariablesCollector]
+						plugins: [oCSSVariablesCollector, oCSSVariablesUrlResolverPlugin]
 					}));
-					result.cssVariablesSource = oCSSVariablesCollector.toLessVariables();
+					const varsOverride = oCSSVariablesUrlResolverPlugin.getVariables();
+					result.cssVariablesSource = oCSSVariablesCollector.toLessVariables(varsOverride);
 					if (oRTL) {
 						const oCSSVariablesCollectorRTL = new CSSVariablesCollectorPlugin(config);
 						result.cssSkeletonRtl = tree.toCSS(Object.assign({}, options.compiler, {

lib/plugin/css-variables-collector.js

@@ -11,12 +11,13 @@ const CSSVariablesCollectorPlugin = module.exports = function(config) {
 	this.ruleStack = [];
 	this.mixinStack = [];
 	this.parenStack = [];
-	this.importStack = [];
 };
 
 CSSVariablesCollectorPlugin.prototype = {
 
+	// needed to keep the less variable references intact to use this info for the CSS variables references
 	isPreEvalVisitor: true,
+
 	isReplacing: true,
 
 	_isInMixinOrParen() {
@@ -27,28 +28,41 @@ CSSVariablesCollectorPlugin.prototype = {
 		return this.ruleStack.length > 0 && !this.ruleStack[this.ruleStack.length - 1].variable;
 	},
 
-	_isVarInLibrary() {
-		// the last less file defines the location of the css variable and this must
-		// be in the path of the current library, otherwise it is an external variable
-		const regexp = new RegExp(`^/resources/${this.config.libPath}/themes/`);
-		return this.importStack.length > 0 ? regexp.test(this.importStack[this.importStack.length - 1].importedFilename) : true;
+	_isVarInLibrary({varName, filename}) {
+		// for libraries we check that the file is within the libraries theme path
+		// in all other cases with no filename (indicates calculated variables)
+		// or in case of variables in standalone less files we just include them!
+		const include = !filename ||
+			(this.config.libPath ? filename.startsWith(`/resources/${this.config.libPath}/themes/`) : true);
+		return include;
 	},
 
 	_isRelevant() {
 		return !this._isInMixinOrParen() && this._isVarInRule();
 	},
 
-	toLessVariables() {
+	toLessVariables(varsOverride) {
+		const vars = {};
+		Object.keys(this.vars).forEach((key) => {
+			const override = this.vars[key].updateAfterEval && varsOverride[key] !== undefined;
+			if (override) {
+				console.log(`Override variable "${key}" from "${this.vars[key].css}" to "${varsOverride[key].css}"`);
+			}
+			vars[key] = {
+				css: override ? varsOverride[key].css : this.vars[key].css,
+				export: this.vars[key].export
+			};
+		});
 		let lessVariables = "";
-		Object.keys(this.vars).forEach((value, index) => {
-			lessVariables += "@" + value + ": " + this.vars[value].css + ";\n";
+		Object.keys(vars).forEach((value, index) => {
+			lessVariables += "@" + value + ": " + vars[value].css + ";\n";
 		});
 		Object.keys(this.calcVars).forEach((value, index) => {
 			lessVariables += "@" + value + ": " + this.calcVars[value].css + ";\n";
 		});
 		lessVariables += "\n:root {\n";
-		Object.keys(this.vars).forEach((value, index) => {
-			if (this.vars[value].export) {
+		Object.keys(vars).forEach((value, index) => {
+			if (vars[value].export) {
 				lessVariables += "--" + value + ": @" + value + ";\n";
 			}
 		});
@@ -96,7 +110,7 @@ CSSVariablesCollectorPlugin.prototype = {
 	},
 
 	visitCall(node, visitArgs) {
-		// if variables are used inside rules, generate a new dynamic variable for it!
+		// if variables are used inside rules, generate a new calculated variable for it!
 		const isRelevantFunction = typeof less.tree.functions[node.name] === "function" && ["rgba"].indexOf(node.name) === -1;
 		if (this._isRelevant() && isRelevantFunction) {
 			const css = this._getCSS(node);
@@ -110,7 +124,9 @@ CSSVariablesCollectorPlugin.prototype = {
 			}
 			this.calcVars[newName] = {
 				css: css,
-				export: this._isVarInLibrary()
+				export: this._isVarInLibrary({
+					varName: newName
+				})
 			};
 			return new less.tree.Call("var", [new less.tree.Anonymous("--" + newName, node.index, node.currentFileInfo, node.mapLines)]);
 		}
@@ -169,30 +185,32 @@ CSSVariablesCollectorPlugin.prototype = {
 		return node;
 	},
 
-	visitImport(node, visitArgs) {
-		// store the import context
-		this.importStack.push(node);
-		return node;
-	},
-
-	visitImportOut(node) {
-		// remove import context
-		this.importStack.pop();
-		return node;
-	},
-
 	visitRuleset(node, visitArgs) {
 		node.rules.forEach((value) => {
 			const isVarDeclaration = value instanceof less.tree.Rule && typeof value.name === "string" && value.name.startsWith("@");
 			if (!this._isInMixinOrParen() && isVarDeclaration) {
 				// add the variable declaration to the list of vars
-				this.vars[value.name.substr(1)] = {
+				const varName = value.name.substr(1);
+				this.vars[varName] = {
 					css: this._getCSS(value.value),
-					export: this._isVarInLibrary()
+					export: this._isVarInLibrary({
+						varName,
+						filename: value.currentFileInfo.filename
+					})
 				};
 			}
 		});
 		return node;
+	},
+
+	visitUrl(node, visitArgs) {
+		// we mark the less variables which should be updated after eval
+		// => strangewise less variables with "none" values are also urls
+		//    after the less variables have been evaluated
+		if (this.ruleStack.length > 0 && this.ruleStack[0].variable) {
+			this.vars[this.ruleStack[0].name.substr(1)].updateAfterEval = true;
+		}
+		return node;
 	}
 
 };

lib/plugin/css-variables-urlresolver.js

@@ -0,0 +1,48 @@
+"use strict";
+
+const less = require("../thirdparty/less");
+
+const CSSVariablesUrlResolverPlugin = module.exports = function(config) {
+	this.config = config;
+	// eslint-disable-next-line new-cap
+	this.native = new less.tree.visitor(this);
+	this.vars = {};
+	this.ruleStack = [];
+};
+
+CSSVariablesUrlResolverPlugin.prototype = {
+
+	isPreVisitor: true,
+
+	run(root) {
+		return this.native.visit(root);
+	},
+
+	getVariables() {
+		return this.vars;
+	},
+
+	visitRule(node, visitArgs) {
+		// store the rule context for the call variable extraction
+		this.ruleStack.push(node);
+		return node;
+	},
+
+	visitRuleOut(node) {
+		// remove rule context
+		this.ruleStack.pop();
+		return node;
+	},
+
+	visitUrl(node, visitArgs) {
+		// for top-level variables we need the resolved urls
+		if (this.ruleStack.length > 0 && this.ruleStack[0].variable) {
+			const varName = this.ruleStack[0].name.substr(1);
+			this.vars[varName] = {
+				css: node.toCSS()
+			};
+		}
+		return node;
+	}
+
+};