From 77c79744c8609e48647cc1749fcc7e16f39c69f6 Mon Sep 17 00:00:00 2001 From: Adam Detrick Date: Mon, 11 Jun 2018 16:57:00 -0400 Subject: [PATCH 1/5] works add option to clean from output variables injected via js add option to clean from output variables injected via js --- README.md | 6 ++++ index.js | 20 ++++++++++- .../js-defined-preserve-cleanInjected.css | 5 +++ ...efined-preserve-cleanInjected.expected.css | 8 +++++ test/fixtures/js-defined-preserve.css | 5 +++ .../fixtures/js-defined-preserve.expected.css | 18 ++++++++++ test/test.js | 33 ++++++++++++++----- 7 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/js-defined-preserve-cleanInjected.css create mode 100644 test/fixtures/js-defined-preserve-cleanInjected.expected.css create mode 100644 test/fixtures/js-defined-preserve.css create mode 100644 test/fixtures/js-defined-preserve.expected.css diff --git a/README.md b/README.md index f74a8c2..a1d6b52 100644 --- a/README.md +++ b/README.md @@ -368,6 +368,12 @@ Can be a simple key-value pair or an object with a `value` property and an optio The object keys are automatically prefixed with `--` (according to CSS custom property syntax) if you do not provide it. +### `cleanInjectedVariables` (default: `false`) + +Removes custom property declarations inserted via the `variables` option from final output. + +A typical use case is [CSS Modules](https://github.com/css-modules/css-modules), where you would want to avoid +repeating custom property definitions in every module passed through this plugin. ```js var postcss = require('postcss'); diff --git a/index.js b/index.js index 382018d..b7b43b7 100644 --- a/index.js +++ b/index.js @@ -62,7 +62,10 @@ var defaults = { // Define variables via JS // Simple key-value pair // or an object with a `value` property and an optional `isImportant` bool property - variables: {} + variables: {}, + // Remove variables injected via JS with the `variables` option above + // before serializing to CSS + cleanInjectedVariables: false }; module.exports = postcss.plugin('postcss-css-variables', function(options) { @@ -82,6 +85,10 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { // We use this because we don't want to modify the AST when we still need to reference these later on var nodesToRemoveAtEnd = []; + // List of declarations injected from `opts.variables` to remove at the end + // if user passes `opts.cleanInjectedVariables` + var injectedDeclsToRemoveAtEnd = []; + // Map of variable names to a list of declarations var map = {}; @@ -106,6 +113,12 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { }); variableRootRule.append(varDecl); + // mark JS-injected variables for removal if `cleanInjectedVariables` + // option is passed. + if (opts.cleanInjectedVariables) { + injectedDeclsToRemoveAtEnd.push(varDecl); + } + // Add the entry to the map prevVariableMap[variableName] = (prevVariableMap[variableName] || []).concat({ decl: varDecl, @@ -249,6 +262,11 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { // We clean up at the end because we don't want to modify the AST when we still need to reference these later on nodesToRemoveAtEnd.forEach(cleanUpNode); + // Clean up JS-injected variables marked for removal + injectedDeclsToRemoveAtEnd.forEach(function(injectedDecl) { + injectedDecl.remove(); + }); + //console.log('map', map); diff --git a/test/fixtures/js-defined-preserve-cleanInjected.css b/test/fixtures/js-defined-preserve-cleanInjected.css new file mode 100644 index 0000000..135dd4a --- /dev/null +++ b/test/fixtures/js-defined-preserve-cleanInjected.css @@ -0,0 +1,5 @@ +.box1 { + width: var(--js-defined1); + height: var(--js-defined2); + background: var(--js-defined-no-prefix); +} diff --git a/test/fixtures/js-defined-preserve-cleanInjected.expected.css b/test/fixtures/js-defined-preserve-cleanInjected.expected.css new file mode 100644 index 0000000..c217850 --- /dev/null +++ b/test/fixtures/js-defined-preserve-cleanInjected.expected.css @@ -0,0 +1,8 @@ +.box1 { + width: 75px; + width: var(--js-defined1); + height: 80px; + height: var(--js-defined2); + background: #ff0000; + background: var(--js-defined-no-prefix); +} diff --git a/test/fixtures/js-defined-preserve.css b/test/fixtures/js-defined-preserve.css new file mode 100644 index 0000000..135dd4a --- /dev/null +++ b/test/fixtures/js-defined-preserve.css @@ -0,0 +1,5 @@ +.box1 { + width: var(--js-defined1); + height: var(--js-defined2); + background: var(--js-defined-no-prefix); +} diff --git a/test/fixtures/js-defined-preserve.expected.css b/test/fixtures/js-defined-preserve.expected.css new file mode 100644 index 0000000..ad4a6eb --- /dev/null +++ b/test/fixtures/js-defined-preserve.expected.css @@ -0,0 +1,18 @@ +:root { + --js-defined-no-prefix: #ff0000; +} +:root { + --js-defined2: 80px; +} +:root { + --js-defined1: 75px; +} + +.box1 { + width: 75px; + width: var(--js-defined1); + height: 80px; + height: var(--js-defined2); + background: #ff0000; + background: var(--js-defined-no-prefix); +} diff --git a/test/test.js b/test/test.js index 4accea7..d3f90f5 100644 --- a/test/test.js +++ b/test/test.js @@ -12,6 +12,14 @@ var cssvariables = require('../'); var CleanCSS = require('clean-css'); +var MOCK_JS_VARIABLES = { + '--js-defined1': '75px', + '--js-defined2': { + value: '80px' + }, + // Should be automatically prefixed with `--` + 'js-defined-no-prefix': '#ff0000' +}; var testPlugin = function(filePath, expectedFilePath, options) { options = options || {}; @@ -146,20 +154,27 @@ describe('postcss-css-variables', function() { test( 'should work with JS defined variables', 'js-defined', + { variables: MOCK_JS_VARIABLES } + ); + test( + 'should preserve -- declarations and var() values with `options.variables` AND `options.preserve`', + 'js-defined-preserve', + { + variables: MOCK_JS_VARIABLES, + preserve: true + } + ); + test( + 'should preserve var() values and clean injected declarations with `options.variables` AND `options.preserve` AND `options.cleanInjectedVariables`', + 'js-defined-preserve-cleanInjected', { - variables: { - '--js-defined1': '75px', - '--js-defined2': { - value: '80px' - }, - // Should be automatically prefixed with `--` - 'js-defined-no-prefix': '#ff0000' - } + variables: MOCK_JS_VARIABLES, + preserve: true, + cleanInjectedVariables: true } ); }); - describe('with `options.preserve`', function() { test( 'preserves variables when `preserve` is `true`', From 92f986a3b91fbcda4dbc1b9da5e3414e38689d08 Mon Sep 17 00:00:00 2001 From: Adam Detrick Date: Mon, 25 Jun 2018 12:09:56 -0400 Subject: [PATCH 2/5] rename new option to preserveInjectedVariables --- index.js | 8 ++++---- ...cleanInjected.css => js-defined-preserve-injected.css} | 0 ...cted.css => js-defined-preserve-injected.expected.css} | 0 test/test.js | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) rename test/fixtures/{js-defined-preserve-cleanInjected.css => js-defined-preserve-injected.css} (100%) rename test/fixtures/{js-defined-preserve-cleanInjected.expected.css => js-defined-preserve-injected.expected.css} (100%) diff --git a/index.js b/index.js index b7b43b7..b4baa27 100644 --- a/index.js +++ b/index.js @@ -65,7 +65,7 @@ var defaults = { variables: {}, // Remove variables injected via JS with the `variables` option above // before serializing to CSS - cleanInjectedVariables: false + preserveInjectedVariables: true }; module.exports = postcss.plugin('postcss-css-variables', function(options) { @@ -86,7 +86,7 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { var nodesToRemoveAtEnd = []; // List of declarations injected from `opts.variables` to remove at the end - // if user passes `opts.cleanInjectedVariables` + // if user passes `opts.preserveInjectedVariables` var injectedDeclsToRemoveAtEnd = []; // Map of variable names to a list of declarations @@ -113,9 +113,9 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { }); variableRootRule.append(varDecl); - // mark JS-injected variables for removal if `cleanInjectedVariables` + // mark JS-injected variables for removal if `preserveInjectedVariables` // option is passed. - if (opts.cleanInjectedVariables) { + if (!opts.preserveInjectedVariables) { injectedDeclsToRemoveAtEnd.push(varDecl); } diff --git a/test/fixtures/js-defined-preserve-cleanInjected.css b/test/fixtures/js-defined-preserve-injected.css similarity index 100% rename from test/fixtures/js-defined-preserve-cleanInjected.css rename to test/fixtures/js-defined-preserve-injected.css diff --git a/test/fixtures/js-defined-preserve-cleanInjected.expected.css b/test/fixtures/js-defined-preserve-injected.expected.css similarity index 100% rename from test/fixtures/js-defined-preserve-cleanInjected.expected.css rename to test/fixtures/js-defined-preserve-injected.expected.css diff --git a/test/test.js b/test/test.js index d3f90f5..5e1a011 100644 --- a/test/test.js +++ b/test/test.js @@ -165,12 +165,12 @@ describe('postcss-css-variables', function() { } ); test( - 'should preserve var() values and clean injected declarations with `options.variables` AND `options.preserve` AND `options.cleanInjectedVariables`', - 'js-defined-preserve-cleanInjected', + 'should preserve var() values and clean injected declarations with `options.variables` AND `options.preserve` AND `options.preserveInjectedVariables: false`', + 'js-defined-preserve-injected', { variables: MOCK_JS_VARIABLES, preserve: true, - cleanInjectedVariables: true + preserveInjectedVariables: false, } ); }); From ee6f3d417ae6b596a4d8365a1fe2d8f5b70dbc35 Mon Sep 17 00:00:00 2001 From: Adam Detrick Date: Mon, 25 Jun 2018 12:49:06 -0400 Subject: [PATCH 3/5] update code comments and README --- README.md | 7 ++++--- index.js | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a1d6b52..acdf7a6 100644 --- a/README.md +++ b/README.md @@ -368,12 +368,13 @@ Can be a simple key-value pair or an object with a `value` property and an optio The object keys are automatically prefixed with `--` (according to CSS custom property syntax) if you do not provide it. -### `cleanInjectedVariables` (default: `false`) +### `preserveInjectedVariables` (default: `true`) -Removes custom property declarations inserted via the `variables` option from final output. +Whether to preserve the custom property declarations inserted via the `variables` option from final output. A typical use case is [CSS Modules](https://github.com/css-modules/css-modules), where you would want to avoid -repeating custom property definitions in every module passed through this plugin. +repeating custom property definitions in every module passed through this plugin. Setting this option to `false` +prevents JS-injected variables from appearing in output CSS. ```js var postcss = require('postcss'); diff --git a/index.js b/index.js index b4baa27..1991d3b 100644 --- a/index.js +++ b/index.js @@ -63,8 +63,8 @@ var defaults = { // Simple key-value pair // or an object with a `value` property and an optional `isImportant` bool property variables: {}, - // Remove variables injected via JS with the `variables` option above - // before serializing to CSS + // Preserve variables injected via JS with the `variables` option above + // before serializing to CSS (`false` will remove these variables from output) preserveInjectedVariables: true }; From 0e06dde87e7164d4e39d177bf8fabea3676de742 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 26 Jun 2018 14:16:03 -0500 Subject: [PATCH 4/5] Update comments to straightforward when they apply `opts.preserveInjectedVariables = false` means remove JS injected variables --- index.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 1991d3b..fef5769 100644 --- a/index.js +++ b/index.js @@ -85,8 +85,8 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { // We use this because we don't want to modify the AST when we still need to reference these later on var nodesToRemoveAtEnd = []; - // List of declarations injected from `opts.variables` to remove at the end - // if user passes `opts.preserveInjectedVariables` + // Keep track of the injected from `opts.variables` to remove at the end + // if user passes `opts.preserveInjectedVariables = false` var injectedDeclsToRemoveAtEnd = []; // Map of variable names to a list of declarations @@ -113,8 +113,7 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { }); variableRootRule.append(varDecl); - // mark JS-injected variables for removal if `preserveInjectedVariables` - // option is passed. + // Colect JS-injected variables for removal if `opts.preserveInjectedVariables = false` if (!opts.preserveInjectedVariables) { injectedDeclsToRemoveAtEnd.push(varDecl); } From d969b4a46811477ef2dd6de010635295cd1fe4f5 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 26 Jun 2018 14:21:37 -0500 Subject: [PATCH 5/5] Correct "collect" typo --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index fef5769..7bf375c 100644 --- a/index.js +++ b/index.js @@ -113,7 +113,7 @@ module.exports = postcss.plugin('postcss-css-variables', function(options) { }); variableRootRule.append(varDecl); - // Colect JS-injected variables for removal if `opts.preserveInjectedVariables = false` + // Collect JS-injected variables for removal if `opts.preserveInjectedVariables = false` if (!opts.preserveInjectedVariables) { injectedDeclsToRemoveAtEnd.push(varDecl); }