Merge pull request #97 from Konnng/master

Fix regex in resolve-value.js to allow nested CSS functions

lib/resolve-value.js

@@ -1,16 +1,14 @@
+var balanced = require('balanced-match');
+
 var generateScopeList = require('./generate-scope-list');
 var isNodeUnderScope = require('./is-node-under-scope');
 var gatherVariableDependencies = require('./gather-variable-dependencies');
 
 var findNodeAncestorWithSelector = require('./find-node-ancestor-with-selector');
 var cloneSpliceParentOntoNodeWhen = require('./clone-splice-parent-onto-node-when');
 
-
-
-// var() = var( <custom-property-name> [, <any-value> ]? )
-// matches `name[, fallback]`, captures "name" and "fallback"
-// See: http://dev.w3.org/csswg/css-variables/#funcdef-var
-var RE_VAR_FUNC = (/var\(\s*(--[^,\s]+?)(?:\s*,\s*(.+))?\s*\)/);
+// Regexp to capture variable names
+var RE_VAR_FUNC = (/var\(\s*(--[^,\s)]+)/);
 
 function toString(value) {
 	return String(value);
@@ -27,26 +25,53 @@ function toString(value) {
 var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal debugging*/_debugIsInternal) {
 	var debugIndent = _debugIsInternal ? '\t' : '';
 
+	var matchingVarDecl = undefined;
 	var resultantValue = toString(decl.value);
 	var warnings = [];
 
-	var variablesUsedInValueMap = {};
-	// Use `replace` as a loop to go over all occurrences with the `g` flag
-	resultantValue.replace(new RegExp(RE_VAR_FUNC.source, 'g'), function(match, variableName, fallback) {
+	// Match all variables first so we can later on if there are circular dependencies
+	var variablesUsedInValueMap = {}
+	// Create a temporary variable, storing resultantValue variable value
+	var remainingVariableValue = resultantValue;
+	// Use balanced lib to find var() declarations and store variable names
+	while ((matchingVarDecl = balanced('var(', ')', remainingVariableValue))) {
+		// Split at the comma to find variable name and fallback value
+		// There may be other commas in the values so this isn't necessarily just 2 pieces
+		var variableFallbackSplitPieces = matchingVarDecl.body.split(',');
+
+		// Get variable name and fallback, filtering empty items
+		var variableName = variableFallbackSplitPieces[0].trim();
+
+		// add variable found in the object
 		variablesUsedInValueMap[variableName] = true;
-	});
+
+		// Replace variable name (first occurence only) from result, to avoid circular loop
+		remainingVariableValue = (matchingVarDecl.pre || '') + matchingVarDecl.body.replace(variableName, '') + (matchingVarDecl.post || '');
+	}
+	// clear temporary variable
+	remainingVariableValue = undefined;
+
 	var variablesUsedInValue = Object.keys(variablesUsedInValueMap);
 
 	//console.log(debugIndent, (_debugIsInternal ? '' : 'Try resolving'), generateScopeList(decl.parent, true), `ignorePseudoScope=${ignorePseudoScope}`, '------------------------');
 
 	// Resolve any var(...) substitutons
 	var isResultantValueUndefined = false;
-	resultantValue = resultantValue.replace(new RegExp(RE_VAR_FUNC.source, 'g'), function(match, variableName, fallback) {
-		// Loop through the list of declarations for that value and find the one that best matches
-		// By best match, we mean, the variable actually applies. Criteria:
-		//		- is under the same scope
-		//		- The latest defined `!important` if any
-		var matchingVarDeclMapItem;
+
+	// var() = var( <custom-property-name> [, <any-value> ]? )
+	// matches `name[, fallback]`, captures "name" and "fallback"
+	// See: http://dev.w3.org/csswg/css-variables/#funcdef-var
+	while ((matchingVarDecl = balanced('var(', ')', resultantValue))) {
+		var matchingVarDeclMapItem = undefined;
+
+		// Split at the comma to find variable name and fallback value
+		// There may be other commas in the values so this isn't necessarily just 2 pieces
+		var variableFallbackSplitPieces = matchingVarDecl.body.split(',');
+
+		// Get variable name and fallback, filtering empty items
+		var variableName = variableFallbackSplitPieces[0].trim();
+		var fallback = variableFallbackSplitPieces.length > 1 ? variableFallbackSplitPieces.slice(1).join(',').trim() : undefined;
+
 		(map[variableName] || []).forEach(function(varDeclMapItem) {
 			// Make sure the variable declaration came from the right spot
 			// And if the current matching variable is already important, a new one to replace it has to be important
@@ -97,10 +122,9 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal
 			warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]);
 		}
 
-		//console.log(debugIndent, 'replaceValue', replaceValue);
-
-		return replaceValue;
-	});
+		// Replace original declaration with found value
+		resultantValue = (matchingVarDecl.pre || '') + replaceValue + (matchingVarDecl.post || '')
+	}
 
 	return {
 		// The resolved value

package.json

@@ -14,6 +14,7 @@
     "url": "https://github.com/MadLittleMods/postcss-css-variables.git"
   },
   "dependencies": {
+    "balanced-match": "^1.0.0",
     "escape-string-regexp": "^1.0.3",
     "extend": "^3.0.1",
     "postcss": "^6.0.8"

test/fixtures/nested-inside-calc-func-with-fallback-var.css

@@ -0,0 +1,16 @@
+:root {
+	--some-width: 150px;
+	--some-other-width: 50px;
+}
+
+.box-foo {
+	width: calc(58.3333333333% - var(--missing, var(--some-width, 100px))); 
+}
+
+.box-foo {
+	width: calc(58.3333333333% - var(--missing, var(--missing2, 100px))); 
+}
+
+.box-foo {
+	width: calc(58.3333333333% - var(--missing, var(--missing2, var(--some-other-width)))); 
+}

test/fixtures/nested-inside-calc-func-with-fallback-var.expected.css

@@ -0,0 +1,11 @@
+.box-foo {
+	width: calc(58.3333333333% - 150px); 
+}
+
+.box-foo {
+	width: calc(58.3333333333% - 100px); 
+}
+
+.box-foo {
+	width: calc(58.3333333333% - 50px); 
+}

test/fixtures/nested-inside-calc-func-with-fallback.css

@@ -0,0 +1,11 @@
+:root {
+	--some-width: 150px;
+}
+
+.box-foo {
+	width: calc(58.3333333333% - var(--some-width, 100px)); 
+}
+
+.box-foo {
+	width: calc(58.3333333333% - var(--missing, 100px)); 
+}

test/fixtures/nested-inside-calc-func-with-fallback.expected.css

@@ -0,0 +1,7 @@
+.box-foo {
+	width: calc(58.3333333333% - 150px); 
+}
+
+.box-foo {
+	width: calc(58.3333333333% - 100px); 
+}

test/fixtures/nested-inside-calc-func.css

@@ -0,0 +1,23 @@
+:root {
+  --some-width: 150px;
+  --some-other-width: 50px;
+}
+
+.box-foo {
+	width: calc(1000% - var(--some-width)); 
+}
+
+.box-foo {
+	width: calc(1000% - var(--missing-width)); 
+}
+
+.box-foo {
+	width: calc(var(--some-width) - var(--some-other-width)); 
+}
+
+.box-foo {
+  --widthA: 100px;
+  --widthB: calc(var(--widthA) / 2);
+  --widthC: calc(var(--widthB) / 2);
+  width: var(--widthC);
+}

test/fixtures/nested-inside-calc-func.expected.css

@@ -0,0 +1,15 @@
+.box-foo {
+	width: calc(1000% - 150px); 
+}
+
+.box-foo {
+	width: undefined; 
+}
+
+.box-foo {
+	width: calc(150px - 50px); 
+}
+
+.box-foo {
+	width: calc(calc(100px / 2) / 2);
+}

test/fixtures/nested-inside-other-func-with-fallback.css

@@ -0,0 +1,9 @@
+:root {
+	--some-color: red;
+}
+.box-foo {
+	background-color: color(var(--missing, white));
+}
+.box-foo {
+	background-color: color(var(--some-color, white));
+}

test/fixtures/nested-inside-other-func-with-fallback.expected.css

@@ -0,0 +1,6 @@
+.box-foo {
+	background-color: color(white);
+}
+.box-foo {
+	background-color: color(red);
+}

test/fixtures/nested-inside-other-func.css

@@ -1,3 +1,16 @@
+:root {
+	--some-color: red;
+	--some-opacity: 0.3;
+}
+
+.box-foo {
+	background-color: color(var(--some-color));
+}
+
 .box-foo {
-	background-color: color(var(--some-color, white));
+	background-color: rgba(255, 0, 0, var(--some-opacity));
 }
+
+.box-foo {
+	background-color: hsla(120,100%,50%, var(--missing-opacity));
+}

test/fixtures/nested-inside-other-func.expected.css

@@ -1,3 +1,11 @@
 .box-foo {
-	background-color: color(white);
+	background-color: color(red);
 }
+
+.box-foo {
+	background-color: rgba(255, 0, 0, 0.3);
+}
+
+.box-foo {
+	background-color: undefined;
+}

test/test.js

@@ -259,6 +259,10 @@ describe('postcss-css-variables', function() {
 		test('should use fallback variable if provided with missing variables calc', 'missing-variable-should-fallback-calc');
 		test('should use fallback variable if provided with missing variables nested', 'missing-variable-should-fallback-nested');
 		test('should not mangle outer function parentheses', 'nested-inside-other-func');
+		test('should not mangle outer function parentheses - with fallback', 'nested-inside-other-func-with-fallback');
+		test('should not mangle outer function parentheses - calc', 'nested-inside-calc-func');
+		test('should not mangle outer function parentheses - calc with fallback', 'nested-inside-calc-func-with-fallback');
+		test('should not mangle outer function parentheses - calc with fallback var()', 'nested-inside-calc-func-with-fallback-var');
 	});
 
 	test('should accept whitespace in var() declarations', 'whitespace-in-var-declaration' )