[FIX] CSS Variables: Fix variable handling / relative urls

lib/index.js

@@ -191,6 +191,7 @@ Builder.prototype.build = function(options) {
 		// inject the library name as prefix (e.g. "sap.m" as "_sap_m")
 		if (options.library && typeof options.library.name === "string") {
 			const libName = config.libName = options.library.name;
+			config.libPath = libName.replace(/\./g, "/");
 			config.prefix = "_" + libName.replace(/\./g, "_") + "_";
 		}
 
@@ -287,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,27 +28,41 @@ CSSVariablesCollectorPlugin.prototype = {
 		return this.ruleStack.length > 0 && !this.ruleStack[this.ruleStack.length - 1].variable;
 	},
 
-	_isInGlobalOrBaseImport() {
-		return this.config.libName !== "sap.ui.core" && this.importStack.filter((importNode) => {
-			return /\/(?:global|base)\.less$/.test(importNode.importedFilename);
-		}).length > 0;
+	_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 && process && process.env && process.env.LESS_OPENUI5_CSSVARS_REPORT_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";
 			}
 		});
@@ -95,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);
@@ -109,7 +124,9 @@ CSSVariablesCollectorPlugin.prototype = {
 			}
 			this.calcVars[newName] = {
 				css: css,
-				export: !this._isInGlobalOrBaseImport()
+				export: this._isVarInLibrary({
+					varName: newName
+				})
 			};
 			return new less.tree.Call("var", [new less.tree.Anonymous("--" + newName, node.index, node.currentFileInfo, node.mapLines)]);
 		}
@@ -133,6 +150,20 @@ CSSVariablesCollectorPlugin.prototype = {
 	},
 
 	visitRule(node, visitArgs) {
+		// check rule for being a variable declaration
+		const isVarDeclaration = typeof node.name === "string" && node.name.startsWith("@");
+		if (!this._isInMixinOrParen() && isVarDeclaration) {
+			// add the variable declaration to the list of vars
+			const varName = node.name.substr(1);
+			const isVarInLib = this._isVarInLibrary({
+				varName,
+				filename: node.currentFileInfo.filename
+			});
+			this.vars[varName] = {
+				css: this._getCSS(node.value),
+				export: isVarInLib
+			};
+		}
 		// store the rule context for the call variable extraction
 		this.ruleStack.push(node);
 		return node;
@@ -168,29 +199,13 @@ 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)] = {
-					css: this._getCSS(value.value),
-					export: !this._isInGlobalOrBaseImport()
-				};
-			}
-		});
+	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;
+	}
+
+};

test/expected/complex/test-cssvars-variables.css

@@ -3,6 +3,6 @@
   --link-color-hover: #3071a9;
   --other-var: 0 0 0.25rem 0 rgba(0, 0, 0, 0.15), inset 0 -0.0625rem 0 0 #428bca;
   --link-color-active: var(--link-color);
+  --a: 100%;
   --var: var(--a);
-  --a: 9%;
 }