From adcc69c77660380b201b08eb523f6fa709326109 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Tue, 16 Apr 2019 18:40:24 -0300 Subject: [PATCH 01/16] fixed regex in resolve-value.js This fix will make work, when using nested CSS functions in the same declaration. See https://github.com/MadLittleMods/postcss-css-variables/issues/96 --- lib/resolve-value.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 6f27956..535e6fb 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -10,7 +10,8 @@ var cloneSpliceParentOntoNodeWhen = require('./clone-splice-parent-onto-node-whe // var() = var( [, ]? ) // 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*\)/); +//var RE_VAR_FUNC = (/var\(\s*(--[^,\s]+?)(?:\s*,\s*(.+))?\s*\)/); +var RE_VAR_FUNC = (/var\(\s*(--[^,\s]+?)(?:\s*,\s*([^)]+))?\s*\)/); function toString(value) { return String(value); From 398a88e9c53f453c1db963f32d3c993ffbe9deda Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Tue, 16 Apr 2019 18:45:36 -0300 Subject: [PATCH 02/16] package.json version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3c2b79c..364cd09 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "postcss-css-variables", - "version": "0.12.0", + "version": "0.12.1", "description": "PostCSS plugin to transform CSS Custom Properties(CSS variables) syntax into a static representation", "keywords": [ "postcss", From 6ce15de65beac3c07108c8927c4162317d196631 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Wed, 24 Apr 2019 00:55:36 -0300 Subject: [PATCH 03/16] undo version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 364cd09..3c2b79c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "postcss-css-variables", - "version": "0.12.1", + "version": "0.12.0", "description": "PostCSS plugin to transform CSS Custom Properties(CSS variables) syntax into a static representation", "keywords": [ "postcss", From dff2e3a58b706dc7d1de845ac5efa7c623105aba Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Wed, 24 Apr 2019 00:57:26 -0300 Subject: [PATCH 04/16] Fixed a problem with var() declarations in nested functions --- lib/resolve-value.js | 120 +++++++++--------- test/fixtures/nested-inside-other-func.css | 12 ++ .../nested-inside-other-func.expected.css | 8 ++ 3 files changed, 81 insertions(+), 59 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 535e6fb..38cbf8d 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -10,8 +10,7 @@ var cloneSpliceParentOntoNodeWhen = require('./clone-splice-parent-onto-node-whe // var() = var( [, ]? ) // 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*\)/); -var RE_VAR_FUNC = (/var\(\s*(--[^,\s]+?)(?:\s*,\s*([^)]+))?\s*\)/); +var RE_VAR_FUNC = (/var\(\s*(--[^,\s]+?)(?:\s*,\s*([^()]+|.+))?\s*\)/); function toString(value) { return String(value); @@ -42,66 +41,69 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal // 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; - (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 - var isRoot = varDeclMapItem.parent.type === 'root' || varDeclMapItem.parent.selectors[0] === ':root'; - var underScope = isNodeUnderScope(decl.parent, varDeclMapItem.parent); - var underScsopeIgnorePseudo = isNodeUnderScope(decl.parent, varDeclMapItem.parent, ignorePseudoScope); - - //console.log(debugIndent, 'isNodeUnderScope', underScope, underScsopeIgnorePseudo, generateScopeList(varDeclMapItem.parent, true), varDeclMapItem.decl.value); - - if( - underScsopeIgnorePseudo && - // And if the currently matched declaration is `!important`, it will take another `!important` to override it - (!(matchingVarDeclMapItem || {}).isImportant || varDeclMapItem.isImportant) - ) { - matchingVarDeclMapItem = varDeclMapItem; + while (resultantValue.match(new RegExp(RE_VAR_FUNC.source, 'g'))) { + 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; + (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 + var isRoot = varDeclMapItem.parent.type === 'root' || varDeclMapItem.parent.selectors[0] === ':root'; + + var underScope = isNodeUnderScope(decl.parent, varDeclMapItem.parent); + var underScsopeIgnorePseudo = isNodeUnderScope(decl.parent, varDeclMapItem.parent, ignorePseudoScope); + + //console.log(debugIndent, 'isNodeUnderScope', underScope, underScsopeIgnorePseudo, generateScopeList(varDeclMapItem.parent, true), varDeclMapItem.decl.value); + + if( + underScsopeIgnorePseudo && + // And if the currently matched declaration is `!important`, it will take another `!important` to override it + (!(matchingVarDeclMapItem || {}).isImportant || varDeclMapItem.isImportant) + ) { + matchingVarDeclMapItem = varDeclMapItem; + } + }); + + // Default to the calculatedInPlaceValue which might be a previous fallback, then try this declarations fallback + var replaceValue = (matchingVarDeclMapItem || {}).calculatedInPlaceValue || (function() { + // Resolve `var` values in fallback + var fallbackValue = fallback; + if(fallback) { + var fallbackDecl = decl.clone({ parent: decl.parent, value: fallback }); + fallbackValue = resolveValue(fallbackDecl, map, false, /*internal*/true).value; + } + + return fallbackValue; + })(); + // Otherwise if the dependency health is good(no circular or self references), dive deeper and resolve + if(matchingVarDeclMapItem !== undefined && !gatherVariableDependencies(variablesUsedInValue, map).hasCircularOrSelfReference) { + // Splice the declaration parent onto the matching entry + + var varDeclScopeList = generateScopeList(decl.parent.parent, true); + var innerMostAtRuleSelector = varDeclScopeList[0].slice(-1)[0]; + var nodeToSpliceParentOnto = findNodeAncestorWithSelector(innerMostAtRuleSelector, matchingVarDeclMapItem.decl.parent); + // See: `test/fixtures/cascade-with-calc-expression-on-nested-rules` + var matchingMimicDecl = cloneSpliceParentOntoNodeWhen(matchingVarDeclMapItem.decl, decl.parent.parent, function(ancestor) { + return ancestor === nodeToSpliceParentOnto; + }); + + replaceValue = resolveValue(matchingMimicDecl, map, false, /*internal*/true).value; } - }); - - // Default to the calculatedInPlaceValue which might be a previous fallback, then try this declarations fallback - var replaceValue = (matchingVarDeclMapItem || {}).calculatedInPlaceValue || (function() { - // Resolve `var` values in fallback - var fallbackValue = fallback; - if(fallback) { - var fallbackDecl = decl.clone({ parent: decl.parent, value: fallback }); - fallbackValue = resolveValue(fallbackDecl, map, false, /*internal*/true).value; + + isResultantValueUndefined = replaceValue === undefined; + if(isResultantValueUndefined) { + warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]); } - - return fallbackValue; - })(); - // Otherwise if the dependency health is good(no circular or self references), dive deeper and resolve - if(matchingVarDeclMapItem !== undefined && !gatherVariableDependencies(variablesUsedInValue, map).hasCircularOrSelfReference) { - // Splice the declaration parent onto the matching entry - - var varDeclScopeList = generateScopeList(decl.parent.parent, true); - var innerMostAtRuleSelector = varDeclScopeList[0].slice(-1)[0]; - var nodeToSpliceParentOnto = findNodeAncestorWithSelector(innerMostAtRuleSelector, matchingVarDeclMapItem.decl.parent); - // See: `test/fixtures/cascade-with-calc-expression-on-nested-rules` - var matchingMimicDecl = cloneSpliceParentOntoNodeWhen(matchingVarDeclMapItem.decl, decl.parent.parent, function(ancestor) { - return ancestor === nodeToSpliceParentOnto; - }); - - replaceValue = resolveValue(matchingMimicDecl, map, false, /*internal*/true).value; - } - - isResultantValueUndefined = replaceValue === undefined; - if(isResultantValueUndefined) { - warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]); - } - - //console.log(debugIndent, 'replaceValue', replaceValue); - - return replaceValue; - }); + + //console.log(debugIndent, 'replaceValue', replaceValue); + + return replaceValue; + }); + } return { // The resolved value diff --git a/test/fixtures/nested-inside-other-func.css b/test/fixtures/nested-inside-other-func.css index cbaa013..b546b44 100644 --- a/test/fixtures/nested-inside-other-func.css +++ b/test/fixtures/nested-inside-other-func.css @@ -1,3 +1,15 @@ +:root { + --some-width: 150px; +} + .box-foo { background-color: color(var(--some-color, white)); } + +.box-foo { + width: calc(58.3333333333% - var(--some-width, 100px)); +} + +.box-foo { + width: calc(58.3333333333% - var(--missing, 100px)); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func.expected.css b/test/fixtures/nested-inside-other-func.expected.css index b9b5187..2c2b2d4 100644 --- a/test/fixtures/nested-inside-other-func.expected.css +++ b/test/fixtures/nested-inside-other-func.expected.css @@ -1,3 +1,11 @@ .box-foo { background-color: color(white); } + +.box-foo { + width: calc(58.3333333333% - 150px); +} + +.box-foo { + width: calc(58.3333333333% - 100px); +} \ No newline at end of file From bd81a6fd625c5caed439ce399303fb5c4d2cef5d Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Wed, 24 Apr 2019 02:36:09 -0300 Subject: [PATCH 05/16] small code improvement to map css variables correctly (for nested variables with CSS functions) and added few more tests. --- lib/resolve-value.js | 10 +++++++--- test/fixtures/nested-inside-other-func.css | 13 +++++++++++++ test/fixtures/nested-inside-other-func.expected.css | 12 ++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 38cbf8d..5c65497 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -32,9 +32,13 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal 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) { - variablesUsedInValueMap[variableName] = true; - }); + var resultantValueCopy = resultantValue; + while (resultantValueCopy.match(new RegExp(RE_VAR_FUNC.source, 'g'))) { + resultantValueCopy = resultantValueCopy.replace(new RegExp(RE_VAR_FUNC.source, 'g'), function(match, variableName, fallback) { + variablesUsedInValueMap[variableName] = true; + }); + } + var variablesUsedInValue = Object.keys(variablesUsedInValueMap); //console.log(debugIndent, (_debugIsInternal ? '' : 'Try resolving'), generateScopeList(decl.parent, true), `ignorePseudoScope=${ignorePseudoScope}`, '------------------------'); diff --git a/test/fixtures/nested-inside-other-func.css b/test/fixtures/nested-inside-other-func.css index b546b44..2febe16 100644 --- a/test/fixtures/nested-inside-other-func.css +++ b/test/fixtures/nested-inside-other-func.css @@ -1,5 +1,6 @@ :root { --some-width: 150px; + --some-other-width: 50px; } .box-foo { @@ -12,4 +13,16 @@ .box-foo { width: calc(58.3333333333% - var(--missing, 100px)); +} + +.box-foo { + width: calc(58.3333333333% - var(--missing, var(--some-width, 100px))); +} + +.box-foo { + width: calc(58.3333333333% - var(--missing)); +} + +.box-foo { + width: calc(var(--some-width) - var(--some-other-width)); } \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func.expected.css b/test/fixtures/nested-inside-other-func.expected.css index 2c2b2d4..3ab863c 100644 --- a/test/fixtures/nested-inside-other-func.expected.css +++ b/test/fixtures/nested-inside-other-func.expected.css @@ -8,4 +8,16 @@ .box-foo { width: calc(58.3333333333% - 100px); +} + +.box-foo { + width: calc(58.3333333333% - 150px); +} + +.box-foo { + width: undefined; +} + +.box-foo { + width: calc(150px - 50px); } \ No newline at end of file From bf638e10f722fafeb0175bb81550fd0f684cbdfa Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Wed, 24 Apr 2019 02:44:47 -0300 Subject: [PATCH 06/16] .editorconfig added, to fix indentation problems (tab vs spaces) --- .editorconfig | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..a2dbfaa --- /dev/null +++ b/.editorconfig @@ -0,0 +1,8 @@ +[*] +end_of_line = lf +insert_final_newline = true + +[*.{js,css}] +charset = utf-8 +indent_style = tab +indent_size = 4 \ No newline at end of file From cd5c5dcf447c803f8b68185cd3da0158aa622cd8 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Thu, 25 Apr 2019 21:47:29 -0300 Subject: [PATCH 07/16] Code improviments to match var() declarations in nested CSS functions, using match-recursive libr .editorconfig removed --- .editorconfig | 8 --- lib/resolve-value.js | 64 ++++++++++--------- package.json | 1 + test/fixtures/nested-inside-other-func.css | 24 ++++++- .../nested-inside-other-func.expected.css | 19 +++++- 5 files changed, 76 insertions(+), 40 deletions(-) delete mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig deleted file mode 100644 index a2dbfaa..0000000 --- a/.editorconfig +++ /dev/null @@ -1,8 +0,0 @@ -[*] -end_of_line = lf -insert_final_newline = true - -[*.{js,css}] -charset = utf-8 -indent_style = tab -indent_size = 4 \ No newline at end of file diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 5c65497..730bb37 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -1,3 +1,5 @@ +var matchRecursive = require('match-recursive'); + var generateScopeList = require('./generate-scope-list'); var isNodeUnderScope = require('./is-node-under-scope'); var gatherVariableDependencies = require('./gather-variable-dependencies'); @@ -5,12 +7,8 @@ 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( [, ]? ) -// 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*\)/); +// regex to capture variable names +var RE_VAR_FUNC = (/var\(\s*(--[^,\s)]+)/); function toString(value) { return String(value); @@ -32,37 +30,42 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var variablesUsedInValueMap = {}; // Use `replace` as a loop to go over all occurrences with the `g` flag - var resultantValueCopy = resultantValue; - while (resultantValueCopy.match(new RegExp(RE_VAR_FUNC.source, 'g'))) { - resultantValueCopy = resultantValueCopy.replace(new RegExp(RE_VAR_FUNC.source, 'g'), function(match, variableName, fallback) { - variablesUsedInValueMap[variableName] = true; - }); - } - + resultantValue.replace(new RegExp(RE_VAR_FUNC.source, 'g'), function(match, variableName, fallback) { + variablesUsedInValueMap[variableName] = true; + }); var variablesUsedInValue = Object.keys(variablesUsedInValueMap); //console.log(debugIndent, (_debugIsInternal ? '' : 'Try resolving'), generateScopeList(decl.parent, true), `ignorePseudoScope=${ignorePseudoScope}`, '------------------------'); // Resolve any var(...) substitutons var isResultantValueUndefined = false; + + // var() = var( [, ]? ) + // matches `name[, fallback]`, captures "name" and "fallback" + // See: http://dev.w3.org/csswg/css-variables/#funcdef-var + var matches = null; + // iterate over possible recursive matches + while ((matches = matchRecursive(resultantValue, 'var(...)')).length) { + // var originalDeclaration = matches.shift(); // get what was matched + + // iterate over found var() occurences (some declaration may have multiple var() nested) + matches.forEach(function (match) { + match = match.split(','); + match = [ match.shift(), match.join(',') ].filter(item => item) // clear empty items + + var [ variableName, fallback ] = match.map(item => item.trim()) - while (resultantValue.match(new RegExp(RE_VAR_FUNC.source, 'g'))) { - 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; (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 var isRoot = varDeclMapItem.parent.type === 'root' || varDeclMapItem.parent.selectors[0] === ':root'; - + var underScope = isNodeUnderScope(decl.parent, varDeclMapItem.parent); var underScsopeIgnorePseudo = isNodeUnderScope(decl.parent, varDeclMapItem.parent, ignorePseudoScope); - + //console.log(debugIndent, 'isNodeUnderScope', underScope, underScsopeIgnorePseudo, generateScopeList(varDeclMapItem.parent, true), varDeclMapItem.decl.value); - + if( underScsopeIgnorePseudo && // And if the currently matched declaration is `!important`, it will take another `!important` to override it @@ -71,7 +74,7 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal matchingVarDeclMapItem = varDeclMapItem; } }); - + // Default to the calculatedInPlaceValue which might be a previous fallback, then try this declarations fallback var replaceValue = (matchingVarDeclMapItem || {}).calculatedInPlaceValue || (function() { // Resolve `var` values in fallback @@ -80,13 +83,13 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var fallbackDecl = decl.clone({ parent: decl.parent, value: fallback }); fallbackValue = resolveValue(fallbackDecl, map, false, /*internal*/true).value; } - + return fallbackValue; })(); // Otherwise if the dependency health is good(no circular or self references), dive deeper and resolve if(matchingVarDeclMapItem !== undefined && !gatherVariableDependencies(variablesUsedInValue, map).hasCircularOrSelfReference) { // Splice the declaration parent onto the matching entry - + var varDeclScopeList = generateScopeList(decl.parent.parent, true); var innerMostAtRuleSelector = varDeclScopeList[0].slice(-1)[0]; var nodeToSpliceParentOnto = findNodeAncestorWithSelector(innerMostAtRuleSelector, matchingVarDeclMapItem.decl.parent); @@ -94,18 +97,19 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var matchingMimicDecl = cloneSpliceParentOntoNodeWhen(matchingVarDeclMapItem.decl, decl.parent.parent, function(ancestor) { return ancestor === nodeToSpliceParentOnto; }); - + replaceValue = resolveValue(matchingMimicDecl, map, false, /*internal*/true).value; } - + isResultantValueUndefined = replaceValue === undefined; if(isResultantValueUndefined) { warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]); } - + + // replace original declaration + resultantValue = resultantValue.replace(`var(${match.join(',')})`, replaceValue) + //console.log(debugIndent, 'replaceValue', replaceValue); - - return replaceValue; }); } diff --git a/package.json b/package.json index 3c2b79c..6c2f507 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "dependencies": { "escape-string-regexp": "^1.0.3", "extend": "^3.0.1", + "match-recursive": "^0.1.1", "postcss": "^6.0.8" }, "devDependencies": { diff --git a/test/fixtures/nested-inside-other-func.css b/test/fixtures/nested-inside-other-func.css index 2febe16..4a69636 100644 --- a/test/fixtures/nested-inside-other-func.css +++ b/test/fixtures/nested-inside-other-func.css @@ -1,10 +1,17 @@ :root { + --some-color: red; --some-width: 150px; --some-other-width: 50px; + --some-margin: 20px; + --some-padding: 20px; } .box-foo { - background-color: color(var(--some-color, white)); + background-color: color(var(--some-color)); +} + +.box-foo { + background-color: color(var(--missing, white)); } .box-foo { @@ -25,4 +32,19 @@ .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); +} + +.box-foo { + margin: calc(var(--some-margin) - 2px) calc(1rem - var(--missing)); +} + +.box-foo { + padding: calc(var(--some-padding) - 2px) calc(100px - var(--some-padding)) calc(100px - calc(var(--missing, 20px) - 10px)); } \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func.expected.css b/test/fixtures/nested-inside-other-func.expected.css index 3ab863c..b3732b6 100644 --- a/test/fixtures/nested-inside-other-func.expected.css +++ b/test/fixtures/nested-inside-other-func.expected.css @@ -1,3 +1,7 @@ +.box-foo { + background-color: color(red); +} + .box-foo { background-color: color(white); } @@ -20,4 +24,17 @@ .box-foo { width: calc(150px - 50px); -} \ No newline at end of file +} + +.box-foo { + width: calc(calc(100px / 2) / 2); +} + +.box-foo { + margin: undefined +} + +.box-foo { + padding: calc(20px - 2px) calc(100px - 20px) calc(100px - calc(20px - 10px)); +} + From 9088a860eaab1bb208aa3e927c94a1de74ce2129 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Fri, 26 Apr 2019 14:18:16 -0300 Subject: [PATCH 08/16] Update package.json package.json preinstall instruction added, to avoid npm not installing required package dependencies. --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 6c2f507..abebad1 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "type": "git", "url": "https://github.com/MadLittleMods/postcss-css-variables.git" }, + "preinstall": "npm install escape-string-regexp extend match-recursive postcss", "dependencies": { "escape-string-regexp": "^1.0.3", "extend": "^3.0.1", From f3b8ea5c43edf39687701a12f4b17141bc101251 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Mon, 29 Apr 2019 19:47:30 -0300 Subject: [PATCH 09/16] Code improvements: changed regex match library to balanced-match and more tests added for css nested functions. --- lib/resolve-value.js | 121 +++++++++--------- package.json | 2 +- test/fixtures/nested-inside-other-func.css | 8 +- .../nested-inside-other-func.expected.css | 4 + 4 files changed, 68 insertions(+), 67 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 730bb37..fe1f7e4 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -1,4 +1,4 @@ -var matchRecursive = require('match-recursive'); +var balanced = require('balanced-match'); var generateScopeList = require('./generate-scope-list'); var isNodeUnderScope = require('./is-node-under-scope'); @@ -43,74 +43,67 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal // var() = var( [, ]? ) // matches `name[, fallback]`, captures "name" and "fallback" // See: http://dev.w3.org/csswg/css-variables/#funcdef-var - var matches = null; - // iterate over possible recursive matches - while ((matches = matchRecursive(resultantValue, 'var(...)')).length) { - // var originalDeclaration = matches.shift(); // get what was matched - - // iterate over found var() occurences (some declaration may have multiple var() nested) - matches.forEach(function (match) { - match = match.split(','); - match = [ match.shift(), match.join(',') ].filter(item => item) // clear empty items - - var [ variableName, fallback ] = match.map(item => item.trim()) - - var matchingVarDeclMapItem; - (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 - var isRoot = varDeclMapItem.parent.type === 'root' || varDeclMapItem.parent.selectors[0] === ':root'; - - var underScope = isNodeUnderScope(decl.parent, varDeclMapItem.parent); - var underScsopeIgnorePseudo = isNodeUnderScope(decl.parent, varDeclMapItem.parent, ignorePseudoScope); - - //console.log(debugIndent, 'isNodeUnderScope', underScope, underScsopeIgnorePseudo, generateScopeList(varDeclMapItem.parent, true), varDeclMapItem.decl.value); - - if( - underScsopeIgnorePseudo && - // And if the currently matched declaration is `!important`, it will take another `!important` to override it - (!(matchingVarDeclMapItem || {}).isImportant || varDeclMapItem.isImportant) - ) { - matchingVarDeclMapItem = varDeclMapItem; - } - }); - - // Default to the calculatedInPlaceValue which might be a previous fallback, then try this declarations fallback - var replaceValue = (matchingVarDeclMapItem || {}).calculatedInPlaceValue || (function() { - // Resolve `var` values in fallback - var fallbackValue = fallback; - if(fallback) { - var fallbackDecl = decl.clone({ parent: decl.parent, value: fallback }); - fallbackValue = resolveValue(fallbackDecl, map, false, /*internal*/true).value; - } - - return fallbackValue; - })(); - // Otherwise if the dependency health is good(no circular or self references), dive deeper and resolve - if(matchingVarDeclMapItem !== undefined && !gatherVariableDependencies(variablesUsedInValue, map).hasCircularOrSelfReference) { - // Splice the declaration parent onto the matching entry - - var varDeclScopeList = generateScopeList(decl.parent.parent, true); - var innerMostAtRuleSelector = varDeclScopeList[0].slice(-1)[0]; - var nodeToSpliceParentOnto = findNodeAncestorWithSelector(innerMostAtRuleSelector, matchingVarDeclMapItem.decl.parent); - // See: `test/fixtures/cascade-with-calc-expression-on-nested-rules` - var matchingMimicDecl = cloneSpliceParentOntoNodeWhen(matchingVarDeclMapItem.decl, decl.parent.parent, function(ancestor) { - return ancestor === nodeToSpliceParentOnto; - }); - - replaceValue = resolveValue(matchingMimicDecl, map, false, /*internal*/true).value; + var match; + while (match = balanced('var(', ')', resultantValue)) { + var matchingVarDeclMapItem = undefined; + + // split comma to find variable name and fallback value + match.body = match.body.split(','); + // get variable name and fallback, filtering empty items + var [ variableName, fallback ] = [ match.body.shift(), match.body.join(',') ].map(item => item.trim()).filter(item => !!item); + + (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 + var isRoot = varDeclMapItem.parent.type === 'root' || varDeclMapItem.parent.selectors[0] === ':root'; + + var underScope = isNodeUnderScope(decl.parent, varDeclMapItem.parent); + var underScsopeIgnorePseudo = isNodeUnderScope(decl.parent, varDeclMapItem.parent, ignorePseudoScope); + + //console.log(debugIndent, 'isNodeUnderScope', underScope, underScsopeIgnorePseudo, generateScopeList(varDeclMapItem.parent, true), varDeclMapItem.decl.value); + + if( + underScsopeIgnorePseudo && + // And if the currently matched declaration is `!important`, it will take another `!important` to override it + (!(matchingVarDeclMapItem || {}).isImportant || varDeclMapItem.isImportant) + ) { + matchingVarDeclMapItem = varDeclMapItem; } + }); - isResultantValueUndefined = replaceValue === undefined; - if(isResultantValueUndefined) { - warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]); + // Default to the calculatedInPlaceValue which might be a previous fallback, then try this declarations fallback + var replaceValue = (matchingVarDeclMapItem || {}).calculatedInPlaceValue || (function() { + // Resolve `var` values in fallback + var fallbackValue = fallback; + if(fallback) { + var fallbackDecl = decl.clone({ parent: decl.parent, value: fallback }); + fallbackValue = resolveValue(fallbackDecl, map, false, /*internal*/true).value; } - // replace original declaration - resultantValue = resultantValue.replace(`var(${match.join(',')})`, replaceValue) + return fallbackValue; + })(); + // Otherwise if the dependency health is good(no circular or self references), dive deeper and resolve + if(matchingVarDeclMapItem !== undefined && !gatherVariableDependencies(variablesUsedInValue, map).hasCircularOrSelfReference) { + // Splice the declaration parent onto the matching entry + + var varDeclScopeList = generateScopeList(decl.parent.parent, true); + var innerMostAtRuleSelector = varDeclScopeList[0].slice(-1)[0]; + var nodeToSpliceParentOnto = findNodeAncestorWithSelector(innerMostAtRuleSelector, matchingVarDeclMapItem.decl.parent); + // See: `test/fixtures/cascade-with-calc-expression-on-nested-rules` + var matchingMimicDecl = cloneSpliceParentOntoNodeWhen(matchingVarDeclMapItem.decl, decl.parent.parent, function(ancestor) { + return ancestor === nodeToSpliceParentOnto; + }); - //console.log(debugIndent, 'replaceValue', replaceValue); - }); + replaceValue = resolveValue(matchingMimicDecl, map, false, /*internal*/true).value; + } + + isResultantValueUndefined = replaceValue === undefined; + if(isResultantValueUndefined) { + warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]); + } + + // replace original declaration + resultantValue = `${match.pre || ''}${replaceValue}${match.post || ''}`; } return { diff --git a/package.json b/package.json index 6c2f507..e709f4c 100644 --- a/package.json +++ b/package.json @@ -14,9 +14,9 @@ "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", - "match-recursive": "^0.1.1", "postcss": "^6.0.8" }, "devDependencies": { diff --git a/test/fixtures/nested-inside-other-func.css b/test/fixtures/nested-inside-other-func.css index 4a69636..cd69027 100644 --- a/test/fixtures/nested-inside-other-func.css +++ b/test/fixtures/nested-inside-other-func.css @@ -2,7 +2,7 @@ --some-color: red; --some-width: 150px; --some-other-width: 50px; - --some-margin: 20px; + --some-margin: 25px; --some-padding: 20px; } @@ -22,12 +22,16 @@ width: calc(58.3333333333% - var(--missing, 100px)); } +.box-foo { + width: calc(80vw - var(--missing, var(--some-other-width))); +} + .box-foo { width: calc(58.3333333333% - var(--missing, var(--some-width, 100px))); } .box-foo { - width: calc(58.3333333333% - var(--missing)); + width: calc(100vw - var(--missing)); } .box-foo { diff --git a/test/fixtures/nested-inside-other-func.expected.css b/test/fixtures/nested-inside-other-func.expected.css index b3732b6..637094a 100644 --- a/test/fixtures/nested-inside-other-func.expected.css +++ b/test/fixtures/nested-inside-other-func.expected.css @@ -14,6 +14,10 @@ width: calc(58.3333333333% - 100px); } +.box-foo { + width: calc(80vw - 50px); +} + .box-foo { width: calc(58.3333333333% - 150px); } From ba6e1782da7ca9d7fa807b7b4f5a252b4deeeab2 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Thu, 9 May 2019 10:01:21 -0300 Subject: [PATCH 10/16] Revert "Update package.json" This reverts commit 9088a860eaab1bb208aa3e927c94a1de74ce2129. --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 486822d..e709f4c 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,6 @@ "type": "git", "url": "https://github.com/MadLittleMods/postcss-css-variables.git" }, - "preinstall": "npm install escape-string-regexp extend match-recursive postcss", "dependencies": { "balanced-match": "^1.0.0", "escape-string-regexp": "^1.0.3", From 94eb0aa261eba6ef5709f2d7db9949ce533910be Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Fri, 10 May 2019 11:22:17 -0300 Subject: [PATCH 11/16] Code refactor, to make lib more robust and compatible with older node versions --- lib/resolve-value.js | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index fe1f7e4..f7a72ed 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -14,6 +14,10 @@ function toString(value) { return String(value); } +function filterDistinct(value, index, self) { + return self.indexOf(value) === index; +} + // Pass in a value string to parse/resolve and a map of available values // and we can figure out the final value // @@ -25,15 +29,19 @@ function toString(value) { var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal debugging*/_debugIsInternal) { var debugIndent = _debugIsInternal ? '\t' : ''; + var matchingVarDecl = undefined; + var RE_VAR_FUNC_G = new RegExp(RE_VAR_FUNC.source, 'g'); 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) { - variablesUsedInValueMap[variableName] = true; - }); - var variablesUsedInValue = Object.keys(variablesUsedInValueMap); + // match all variables first, to gather all available variables in decl.value + var variablesUsedInValue = []; + while ((matchingVarDecl = RE_VAR_FUNC_G.exec(resultantValue))) { + variablesUsedInValue.push(matchingVarDecl[1]); + } + // remove duplicates from array + variablesUsedInValue = variablesUsedInValue.filter(filterDistinct); //console.log(debugIndent, (_debugIsInternal ? '' : 'Try resolving'), generateScopeList(decl.parent, true), `ignorePseudoScope=${ignorePseudoScope}`, '------------------------'); @@ -43,14 +51,16 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal // var() = var( [, ]? ) // matches `name[, fallback]`, captures "name" and "fallback" // See: http://dev.w3.org/csswg/css-variables/#funcdef-var - var match; - while (match = balanced('var(', ')', resultantValue)) { + while ((matchingVarDecl = balanced('var(', ')', resultantValue))) { var matchingVarDeclMapItem = undefined; - // split comma to find variable name and fallback value - match.body = match.body.split(','); + // Split at the comma to find variable name and fallback value + // There may be other commas in the values so this isn't necessary just 2 pieces + var variableFallbackSplitPieces = matchingVarDecl.body.split(','); + // get variable name and fallback, filtering empty items - var [ variableName, fallback ] = [ match.body.shift(), match.body.join(',') ].map(item => item.trim()).filter(item => !!item); + 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 @@ -103,7 +113,7 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal } // replace original declaration - resultantValue = `${match.pre || ''}${replaceValue}${match.post || ''}`; + resultantValue = (matchingVarDecl.pre || '') + replaceValue + (matchingVarDecl.post || '') } return { From 45ffc6c96f58e33428cae7aeca4095df34e57086 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Mon, 13 May 2019 22:41:15 -0300 Subject: [PATCH 12/16] more use-case tests added --- ...ted-inside-calc-func-with-fallback-var.css | 16 +++++++ ...e-calc-func-with-fallback-var.expected.css | 11 +++++ .../nested-inside-calc-func-with-fallback.css | 11 +++++ ...nside-calc-func-with-fallback.expected.css | 7 +++ test/fixtures/nested-inside-calc-func.css | 23 ++++++++++ .../nested-inside-calc-func.expected.css | 15 +++++++ ...nested-inside-other-func-with-fallback.css | 9 ++++ ...side-other-func-with-fallback.expected.css | 6 +++ test/fixtures/nested-inside-other-func.css | 44 ++----------------- .../nested-inside-other-func.expected.css | 39 ++-------------- test/test.js | 4 ++ 11 files changed, 108 insertions(+), 77 deletions(-) create mode 100644 test/fixtures/nested-inside-calc-func-with-fallback-var.css create mode 100644 test/fixtures/nested-inside-calc-func-with-fallback-var.expected.css create mode 100644 test/fixtures/nested-inside-calc-func-with-fallback.css create mode 100644 test/fixtures/nested-inside-calc-func-with-fallback.expected.css create mode 100644 test/fixtures/nested-inside-calc-func.css create mode 100644 test/fixtures/nested-inside-calc-func.expected.css create mode 100644 test/fixtures/nested-inside-other-func-with-fallback.css create mode 100644 test/fixtures/nested-inside-other-func-with-fallback.expected.css diff --git a/test/fixtures/nested-inside-calc-func-with-fallback-var.css b/test/fixtures/nested-inside-calc-func-with-fallback-var.css new file mode 100644 index 0000000..8d7fb73 --- /dev/null +++ b/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)))); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-calc-func-with-fallback-var.expected.css b/test/fixtures/nested-inside-calc-func-with-fallback-var.expected.css new file mode 100644 index 0000000..7771eca --- /dev/null +++ b/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); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-calc-func-with-fallback.css b/test/fixtures/nested-inside-calc-func-with-fallback.css new file mode 100644 index 0000000..9cf1b73 --- /dev/null +++ b/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)); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-calc-func-with-fallback.expected.css b/test/fixtures/nested-inside-calc-func-with-fallback.expected.css new file mode 100644 index 0000000..af0874c --- /dev/null +++ b/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); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-calc-func.css b/test/fixtures/nested-inside-calc-func.css new file mode 100644 index 0000000..f7d270d --- /dev/null +++ b/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); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-calc-func.expected.css b/test/fixtures/nested-inside-calc-func.expected.css new file mode 100644 index 0000000..8aa3d33 --- /dev/null +++ b/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); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func-with-fallback.css b/test/fixtures/nested-inside-other-func-with-fallback.css new file mode 100644 index 0000000..826b96e --- /dev/null +++ b/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)); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func-with-fallback.expected.css b/test/fixtures/nested-inside-other-func-with-fallback.expected.css new file mode 100644 index 0000000..0c8f61a --- /dev/null +++ b/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); +} \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func.css b/test/fixtures/nested-inside-other-func.css index cd69027..90ebcdf 100644 --- a/test/fixtures/nested-inside-other-func.css +++ b/test/fixtures/nested-inside-other-func.css @@ -1,9 +1,6 @@ :root { --some-color: red; - --some-width: 150px; - --some-other-width: 50px; - --some-margin: 25px; - --some-padding: 20px; + --some-opacity: 0.3; } .box-foo { @@ -11,44 +8,9 @@ } .box-foo { - background-color: color(var(--missing, white)); + background-color: rgba(255, 0, 0, var(--some-opacity)); } .box-foo { - width: calc(58.3333333333% - var(--some-width, 100px)); -} - -.box-foo { - width: calc(58.3333333333% - var(--missing, 100px)); -} - -.box-foo { - width: calc(80vw - var(--missing, var(--some-other-width))); -} - -.box-foo { - width: calc(58.3333333333% - var(--missing, var(--some-width, 100px))); -} - -.box-foo { - width: calc(100vw - var(--missing)); -} - -.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); -} - -.box-foo { - margin: calc(var(--some-margin) - 2px) calc(1rem - var(--missing)); -} - -.box-foo { - padding: calc(var(--some-padding) - 2px) calc(100px - var(--some-padding)) calc(100px - calc(var(--missing, 20px) - 10px)); + background-color: hsla(120,100%,50%, var(--missing-opacity)); } \ No newline at end of file diff --git a/test/fixtures/nested-inside-other-func.expected.css b/test/fixtures/nested-inside-other-func.expected.css index 637094a..e3bd8d3 100644 --- a/test/fixtures/nested-inside-other-func.expected.css +++ b/test/fixtures/nested-inside-other-func.expected.css @@ -3,42 +3,9 @@ } .box-foo { - background-color: color(white); + background-color: rgba(255, 0, 0, 0.3); } .box-foo { - width: calc(58.3333333333% - 150px); -} - -.box-foo { - width: calc(58.3333333333% - 100px); -} - -.box-foo { - width: calc(80vw - 50px); -} - -.box-foo { - width: calc(58.3333333333% - 150px); -} - -.box-foo { - width: undefined; -} - -.box-foo { - width: calc(150px - 50px); -} - -.box-foo { - width: calc(calc(100px / 2) / 2); -} - -.box-foo { - margin: undefined -} - -.box-foo { - padding: calc(20px - 2px) calc(100px - 20px) calc(100px - calc(20px - 10px)); -} - + background-color: undefined; +} \ No newline at end of file diff --git a/test/test.js b/test/test.js index f0879f7..899435e 100644 --- a/test/test.js +++ b/test/test.js @@ -255,6 +255,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' ) From 5a75debb3601862f7e6a02af94a30a45091db1f1 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Sat, 1 Jun 2019 20:45:09 -0300 Subject: [PATCH 13/16] Comment capitalization, to match the rest of the comments --- lib/resolve-value.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index f7a72ed..e6e7bdd 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -7,7 +7,7 @@ var gatherVariableDependencies = require('./gather-variable-dependencies'); var findNodeAncestorWithSelector = require('./find-node-ancestor-with-selector'); var cloneSpliceParentOntoNodeWhen = require('./clone-splice-parent-onto-node-when'); -// regex to capture variable names +// Regexp to capture variable names var RE_VAR_FUNC = (/var\(\s*(--[^,\s)]+)/); function toString(value) { @@ -35,12 +35,12 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var warnings = []; - // match all variables first, to gather all available variables in decl.value + // Match all variables first so we can later on if there are circular dependencies var variablesUsedInValue = []; while ((matchingVarDecl = RE_VAR_FUNC_G.exec(resultantValue))) { variablesUsedInValue.push(matchingVarDecl[1]); } - // remove duplicates from array + // Remove duplicates from array variablesUsedInValue = variablesUsedInValue.filter(filterDistinct); //console.log(debugIndent, (_debugIsInternal ? '' : 'Try resolving'), generateScopeList(decl.parent, true), `ignorePseudoScope=${ignorePseudoScope}`, '------------------------'); @@ -55,10 +55,10 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal 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 necessary just 2 pieces + // 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 + // Get variable name and fallback, filtering empty items var variableName = variableFallbackSplitPieces[0].trim(); var fallback = variableFallbackSplitPieces.length > 1 ? variableFallbackSplitPieces.slice(1).join(',').trim() : undefined; @@ -112,7 +112,7 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal warnings.push(['variable ' + variableName + ' is undefined and used without a fallback', { node: decl }]); } - // replace original declaration + // Replace original declaration with found value resultantValue = (matchingVarDecl.pre || '') + replaceValue + (matchingVarDecl.post || '') } From b3b3fb0f3469296e57574ec8af3f56640eb5b85b Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Sat, 1 Jun 2019 21:18:51 -0300 Subject: [PATCH 14/16] Changed match strategy for variables names, to make better use of balanced npm library --- lib/resolve-value.js | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index e6e7bdd..4795108 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -34,12 +34,34 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var resultantValue = toString(decl.value); var warnings = []; - // Match all variables first so we can later on if there are circular dependencies var variablesUsedInValue = []; - while ((matchingVarDecl = RE_VAR_FUNC_G.exec(resultantValue))) { - variablesUsedInValue.push(matchingVarDecl[1]); + // Create a temporary variable, storing resultantValue variable value + var resultantValueTemp = resultantValue; + // Use balanced lib to find var() declarations and store variable names + while ((matchingVarDecl = balanced('var(', ')', resultantValueTemp))) { + // 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(); + + // Push found variable to variables used array + variablesUsedInValue.push(variableName); + + // Replace variable name (first occurence only) from result, to avoid circular loop + resultantValueTemp = (matchingVarDecl.pre || '') + matchingVarDecl.body.replace(variableName, '') + (matchingVarDecl.post || ''); } + // clear temporary variable + resultantValueTemp = undefined; + + // Old strategy to find used variable names in declaration value, using RegExp + // TODO: remove unused block + // while ((matchingVarDecl = RE_VAR_FUNC_G.exec(resultantValue))) { + // variablesUsedInValue.push(matchingVarDecl[1]); + // } + // Remove duplicates from array variablesUsedInValue = variablesUsedInValue.filter(filterDistinct); From ca24d6064e3a77f4750cc9155204c73564073836 Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Sat, 22 Jun 2019 11:31:38 -0300 Subject: [PATCH 15/16] Small changes and code cleanup --- lib/resolve-value.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 4795108..0f5fafd 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -30,16 +30,15 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var debugIndent = _debugIsInternal ? '\t' : ''; var matchingVarDecl = undefined; - var RE_VAR_FUNC_G = new RegExp(RE_VAR_FUNC.source, 'g'); var resultantValue = toString(decl.value); var warnings = []; // Match all variables first so we can later on if there are circular dependencies - var variablesUsedInValue = []; + var variablesUsedInValueMap = {} // Create a temporary variable, storing resultantValue variable value - var resultantValueTemp = resultantValue; + var remainingVariableValue = resultantValue; // Use balanced lib to find var() declarations and store variable names - while ((matchingVarDecl = balanced('var(', ')', resultantValueTemp))) { + 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(','); @@ -47,23 +46,16 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal // Get variable name and fallback, filtering empty items var variableName = variableFallbackSplitPieces[0].trim(); - // Push found variable to variables used array - variablesUsedInValue.push(variableName); + // add variable found in the object + variablesUsedInValueMap[variableName] = true; // Replace variable name (first occurence only) from result, to avoid circular loop - resultantValueTemp = (matchingVarDecl.pre || '') + matchingVarDecl.body.replace(variableName, '') + (matchingVarDecl.post || ''); + remainingVariableValue = (matchingVarDecl.pre || '') + matchingVarDecl.body.replace(variableName, '') + (matchingVarDecl.post || ''); } // clear temporary variable - resultantValueTemp = undefined; + remainingVariableValue = undefined; - // Old strategy to find used variable names in declaration value, using RegExp - // TODO: remove unused block - // while ((matchingVarDecl = RE_VAR_FUNC_G.exec(resultantValue))) { - // variablesUsedInValue.push(matchingVarDecl[1]); - // } - - // Remove duplicates from array - variablesUsedInValue = variablesUsedInValue.filter(filterDistinct); + var variablesUsedInValue = Object.keys(variablesUsedInValueMap); //console.log(debugIndent, (_debugIsInternal ? '' : 'Try resolving'), generateScopeList(decl.parent, true), `ignorePseudoScope=${ignorePseudoScope}`, '------------------------'); From eb8adbe13190fae476830e909ada6718bca1dc9d Mon Sep 17 00:00:00 2001 From: Julio Vedovatto Date: Sun, 24 Nov 2019 23:43:54 -0300 Subject: [PATCH 16/16] Unused method removed --- lib/resolve-value.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/resolve-value.js b/lib/resolve-value.js index 0f5fafd..1954758 100644 --- a/lib/resolve-value.js +++ b/lib/resolve-value.js @@ -14,10 +14,6 @@ function toString(value) { return String(value); } -function filterDistinct(value, index, self) { - return self.indexOf(value) === index; -} - // Pass in a value string to parse/resolve and a map of available values // and we can figure out the final value // @@ -32,7 +28,7 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal var matchingVarDecl = undefined; var resultantValue = toString(decl.value); var warnings = []; - + // Match all variables first so we can later on if there are circular dependencies var variablesUsedInValueMap = {} // Create a temporary variable, storing resultantValue variable value @@ -61,7 +57,7 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal // Resolve any var(...) substitutons var isResultantValueUndefined = false; - + // var() = var( [, ]? ) // matches `name[, fallback]`, captures "name" and "fallback" // See: http://dev.w3.org/csswg/css-variables/#funcdef-var @@ -75,7 +71,7 @@ var resolveValue = function(decl, map, /*optional*/ignorePseudoScope, /*internal // 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