[FIX] CSS Variables: Fix variable handling / relative urls (#172)

- Improve detection of library variables
- Resolve url values in variables
- Handle variables in the proper order
- Enhance overall CSS Variables tests

Co-authored-by: Matthias Osswald <mat.osswald@sap.com>

lib/index.js

@@ -191,7 +191,10 @@ 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, "_") + "_";
+		} else {
+			config.prefix = ""; // no library, no prefix
 		}
 
 		// Keep filename for later usage (see ImportCollectorPlugin) as less modifies the parserOptions.filename
@@ -287,10 +290,12 @@ 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 oVariableCollector = new VariableCollectorPlugin(options.compiler);
 					result.cssSkeleton = tree.toCSS(Object.assign({}, options.compiler, {
-						plugins: [oCSSVariablesCollector]
+						plugins: [oCSSVariablesCollector, oVariableCollector]
 					}));
-					result.cssVariablesSource = oCSSVariablesCollector.toLessVariables();
+					const varsOverride = oVariableCollector.getAllVariables();
+					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,44 @@ 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({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 regex = new RegExp(`(^|/)${this.config.libPath}/themes/`);
+		const include = !filename ||
+			(this.config.libPath ? regex.test(filename) : 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]}"`);
+			}
+			*/
+			vars[key] = {
+				css: override ? varsOverride[key] : 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";
 			}
 		});
@@ -89,15 +107,17 @@ CSSVariablesCollectorPlugin.prototype = {
 
 	visitOperation(node, visitArgs) {
 		if (this._isRelevant()) {
+			// console.log("visitOperation", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node));
 			return new less.tree.Call("calc", [new less.tree.Expression([node.operands[0], new less.tree.Anonymous(node.op), node.operands[1]])]);
 		}
 		return node;
 	},
 
 	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) {
+			// console.log("visitCall", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node));
 			const css = this._getCSS(node);
 			let newName = this.config.prefix + "function_" + node.name + Object.keys(this.vars).length;
 			// check for duplicate value in vars already
@@ -109,7 +129,7 @@ CSSVariablesCollectorPlugin.prototype = {
 			}
 			this.calcVars[newName] = {
 				css: css,
-				export: !this._isInGlobalOrBaseImport()
+				export: this._isVarInLibrary()
 			};
 			return new less.tree.Call("var", [new less.tree.Anonymous("--" + newName, node.index, node.currentFileInfo, node.mapLines)]);
 		}
@@ -119,6 +139,7 @@ CSSVariablesCollectorPlugin.prototype = {
 	visitNegative(node, visitArgs) {
 		// convert negative into calc function
 		if (this._isRelevant()) {
+			// console.log("visitNegative", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node));
 			return new less.tree.Call("calc", [new less.tree.Expression([new less.tree.Anonymous("-1"), new less.tree.Anonymous("*"), node.value])]);
 		}
 		return node;
@@ -133,6 +154,19 @@ 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({
+				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 +202,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;
 	}
 

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

@@ -9,6 +9,34 @@ a:hover {
   color: #fff;
   background: var(--link-color);
 }
+.background {
+  background-image: var(--backgroundImage);
+}
 .lazy-eval {
   width: var(--var);
 }
+.nested .calc-vars {
+  height: var(--c);
+  width: var(--d);
+  color: var(--function_lighten14);
+  top: calc(-1 * var(--top));
+  line-height: calc(var(--lineHeight) / 2);
+  background: #428bca;
+  border-color: #92bce0;
+  background: #5697d0;
+}
+.nested .calc-vars-duplicate {
+  color: var(--function_lighten14);
+  padding-left: calc(20px + 0.5rem);
+  border-color: #ffffff;
+  background: #ffffff;
+}
+.nested .calc-vars-duplicate somePrefix.somePadding {
+  padding: 1rem;
+  box-sizing: border-box;
+}
+@media (max-width: 599px) {
+  .nested .calc-vars-duplicate somePrefix.somePadding {
+    padding: 0;
+  }
+}

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

@@ -3,6 +3,15 @@
   --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);
+  --backgroundVariant: 1;
+  --backgroundImage: url(img.png);
+  --a: 100%;
   --var: var(--a);
-  --a: 9%;
+  --b: #245682;
+  --padLeft: 20px;
+  --top: 5rem;
+  --lineHeight: 2rem;
+  --c: calc(20%);
+  --d: -100%;
+  --function_lighten14: #3071a9;
 }

test/expected/complex/test-cssvars-variables.source.less

@@ -0,0 +1,33 @@
+@link-color: #428bca;
+@link-color-hover: darken(@link-color, 10%);
+@other-var: 0 0 0.25rem 0 rgba(0, 0, 0, 0.15), inset 0 -0.0625rem 0 0 @link-color;
+@link-color-active: @link-color;
+@backgroundVariant: 1;
+@backgroundImage: url(img.png);
+@a: 100%;
+@var: @a;
+@b: darken(@link-color-active, 20%);
+@padLeft: 20px;
+@top: 5rem;
+@lineHeight: 2rem;
+@c: calc(100% - 80px);
+@d: -@a;
+@function_lighten14: lighten(@b, 10%);
+
+:root {
+--link-color: @link-color;
+--link-color-hover: @link-color-hover;
+--other-var: @other-var;
+--link-color-active: @link-color-active;
+--backgroundVariant: @backgroundVariant;
+--backgroundImage: @backgroundImage;
+--a: @a;
+--var: @var;
+--b: @b;
+--padLeft: @padLeft;
+--top: @top;
+--lineHeight: @lineHeight;
+--c: @c;
+--d: @d;
+--function_lighten14: @function_lighten14;
+}

test/expected/complex/test.css

@@ -9,6 +9,34 @@ a:hover {
   color: #fff;
   background: #428bca;
 }
+.background {
+  background-image: url(img.png);
+}
 .lazy-eval {
   width: 9%;
 }
+.nested .calc-vars {
+  height: calc(20%);
+  width: -100%;
+  color: #3071a9;
+  top: -5rem;
+  line-height: 1rem;
+  background: #428bca;
+  border-color: #92bce0;
+  background: #5697d0;
+}
+.nested .calc-vars-duplicate {
+  color: #3071a9;
+  padding-left: calc(20px + 0.5rem);
+  border-color: #ffffff;
+  background: #ffffff;
+}
+.nested .calc-vars-duplicate somePrefix.somePadding {
+  padding: 1rem;
+  box-sizing: border-box;
+}
+@media (max-width: 599px) {
+  .nested .calc-vars-duplicate somePrefix.somePadding {
+    padding: 0;
+  }
+}

test/expected/libraries/lib1/my/ui/lib/themes/base/library-RTL.css

@@ -6,4 +6,4 @@
 }
 
 /* Inline theming parameters */
-#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%7D')}
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%2C%22url1%22%3A%22url(%27111%27)%22%7D')}

test/expected/libraries/lib1/my/ui/lib/themes/base/library.css

@@ -6,4 +6,4 @@
 }
 
 /* Inline theming parameters */
-#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%7D')}
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%2C%22url1%22%3A%22url(%27111%27)%22%7D')}

test/expected/libraries/lib1/my/ui/lib/themes/foo/css_variables.css

@@ -0,0 +1,10 @@
+:root {
+  --color1: #ffffff;
+  --url1: url('../base/111');
+}
+.fooContrast {
+  --color1: #000000;
+}
+
+/* Inline theming parameters */
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%2C%22url1%22%3A%22url(%27..%2Fbase%2F111%27)%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}

test/expected/libraries/lib1/my/ui/lib/themes/foo/css_variables.source.less

@@ -0,0 +1,7 @@
+@color1: #ffffff;
+@url1: url('../base/111');
+
+:root {
+--color1: @color1;
+--url1: @url1;
+}

test/expected/libraries/lib1/my/ui/lib/themes/foo/library-RTL.css

@@ -10,4 +10,4 @@
 }
 
 /* Inline theming parameters */
-#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%2C%22url1%22%3A%22url(%27..%2Fbase%2F111%27)%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}

test/expected/libraries/lib1/my/ui/lib/themes/foo/library.css

@@ -10,4 +10,4 @@
 }
 
 /* Inline theming parameters */
-#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%2C%22url1%22%3A%22url(%27..%2Fbase%2F111%27)%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}

test/expected/libraries/lib1/my/ui/lib/themes/foo/library_skeleton-RTL.css

@@ -0,0 +1,10 @@
+.myUiLibRule1 {
+  color: var(--color1);
+}
+.myUiLibRule2 {
+  padding: 1px 4px 3px 2px;
+}
+.fooContrast.myUiLibRule1,
+.fooContrast .myUiLibRule1 {
+  color: var(--color1);
+}

test/expected/libraries/lib1/my/ui/lib/themes/foo/library_skeleton.css

@@ -0,0 +1,10 @@
+.myUiLibRule1 {
+  color: var(--color1);
+}
+.myUiLibRule2 {
+  padding: 1px 2px 3px 4px;
+}
+.fooContrast.myUiLibRule1,
+.fooContrast .myUiLibRule1 {
+  color: var(--color1);
+}

test/expected/libraries/lib2/my/ui/lib/themes/bar/library-RTL.css

@@ -10,4 +10,4 @@
 }
 
 /* Inline theming parameters */
-#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%2C%22url1%22%3A%22url(%27..%2Fbase%2F111%27)%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}

test/expected/libraries/lib2/my/ui/lib/themes/bar/library.css

@@ -10,4 +10,4 @@
 }
 
 /* Inline theming parameters */
-#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}
+#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%2C%22url1%22%3A%22url(%27..%2Fbase%2F111%27)%22%7D%2C%22scopes%22%3A%7B%22barContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}

test/expected/simple/test-cssvars-variables.source.less

@@ -0,0 +1,5 @@
+@myColor: #ff0000;
+
+:root {
+--myColor: @myColor;
+}