From 716b3ed4a7be0b581ea5bb81df842d8321e9540c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20Sj=C3=B6qvist?= Date: Mon, 10 May 2021 01:11:06 +0200 Subject: [PATCH 1/3] test: add test case for the current handling of empty local blocks --- .../expected.css | 37 +++++++++++++++++++ .../options-exportEmptyLocals-true/source.css | 29 +++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 test/test-cases/options-exportEmptyLocals-true/expected.css create mode 100644 test/test-cases/options-exportEmptyLocals-true/source.css diff --git a/test/test-cases/options-exportEmptyLocals-true/expected.css b/test/test-cases/options-exportEmptyLocals-true/expected.css new file mode 100644 index 0000000..82a99c3 --- /dev/null +++ b/test/test-cases/options-exportEmptyLocals-true/expected.css @@ -0,0 +1,37 @@ +/* leaf node with contents */ +._input__layer1A { + color: red; +} + +._input__layer2A /* doesn't add anything new */ { +} + +._input__layer1B { + /* totally empty, except for this comment */ +} + +._input__layer2B { + background: blue; +} + +._input__layer3 { +} + +._input__foo > ._input__bar { +} + +._input__baz > ._input__qux { + font-style: italic; +} + +:export { + layer1A: _input__layer1A; + layer2A: _input__layer2A _input__layer1A; + layer1B: _input__layer1B; + layer2B: _input__layer2B _input__layer1B; + layer3: _input__layer3 _input__layer2A _input__layer1A _input__layer2B _input__layer1B; + foo: _input__foo; + bar: _input__bar; + baz: _input__baz; + qux: _input__qux; +} diff --git a/test/test-cases/options-exportEmptyLocals-true/source.css b/test/test-cases/options-exportEmptyLocals-true/source.css new file mode 100644 index 0000000..022cf15 --- /dev/null +++ b/test/test-cases/options-exportEmptyLocals-true/source.css @@ -0,0 +1,29 @@ +/* leaf node with contents */ +:local(.layer1A) { + color: red; +} + +:local(.layer2A) /* doesn't add anything new */ { + composes: layer1A; +} + +:local(.layer1B) { + /* totally empty, except for this comment */ +} + +:local(.layer2B) { + background: blue; + composes: layer1B; +} + +:local(.layer3) { + composes: layer2A; + composes: layer2B; +} + +:local(.foo) /* empty */ > :local(.bar) { +} + +:local(.baz) /* non-empty */ > :local(.qux) { + font-style: italic; +} From 677df7f895ba0630176e8fad1ab41bbfb372b754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20Sj=C3=B6qvist?= Date: Mon, 10 May 2021 01:13:47 +0200 Subject: [PATCH 2/3] fix: remove second argument from call to single-parameter `localizeNode` --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 3197752..9a2f2b5 100644 --- a/src/index.js +++ b/src/index.js @@ -149,7 +149,7 @@ const plugin = (options = {}) => { throw new Error('Unexpected comma (",") in :local block'); } - const selector = localizeNode(node.first, node.spaces); + const selector = localizeNode(node.first); // move the spaces that were around the psuedo selector to the first // non-container node selector.first.spaces = node.spaces; From 91954d894f721951136ca880ef50ed0d75cfbc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20Sj=C3=B6qvist?= Date: Mon, 10 May 2021 01:17:59 +0200 Subject: [PATCH 3/3] feat: add option to omit locals with empty declaration blocks fixes css-modules/css-modules#127, fixes css-modules/css-modules#269 --- src/index.js | 43 +++++++++++++------ .../expected.error.txt | 1 + .../options.js | 3 ++ .../source.css | 3 ++ .../expected.css | 34 +++++++++++++++ .../options.js | 3 ++ .../source.css | 29 +++++++++++++ 7 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/expected.error.txt create mode 100644 test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/options.js create mode 100644 test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/source.css create mode 100644 test/test-cases/options-exportEmptyLocals-false/expected.css create mode 100644 test/test-cases/options-exportEmptyLocals-false/options.js create mode 100644 test/test-cases/options-exportEmptyLocals-false/source.css diff --git a/src/index.js b/src/index.js index 9a2f2b5..492a3ff 100644 --- a/src/index.js +++ b/src/index.js @@ -85,13 +85,19 @@ const plugin = (options = {}) => { const generateExportEntry = (options && options.generateExportEntry) || plugin.generateExportEntry; const exportGlobals = options && options.exportGlobals; + const exportEmptyLocals = + !options || + (typeof options.exportEmptyLocals === "undefined" || + options.exportEmptyLocals === null + ? true + : options.exportEmptyLocals); return { postcssPlugin: "postcss-modules-scope", Once(root, { rule }) { const exports = Object.create(null); - function exportScopedName(name, rawName) { + function exportScopedName(name, rawName, includeSelfReference) { const scopedName = generateScopedName( rawName ? rawName : name, root.source.input.from, @@ -107,30 +113,32 @@ const plugin = (options = {}) => { exports[key] = exports[key] || []; - if (exports[key].indexOf(value) < 0) { + if (includeSelfReference && exports[key].indexOf(value) < 0) { exports[key].push(value); } return scopedName; } - function localizeNode(node) { + function localizeNode(node, exportSelfReference) { switch (node.type) { case "selector": - node.nodes = node.map(localizeNode); + node.nodes = node.map((n) => localizeNode(n, exportSelfReference)); return node; case "class": return selectorParser.className({ value: exportScopedName( node.value, - node.raws && node.raws.value ? node.raws.value : null + node.raws && node.raws.value ? node.raws.value : null, + exportSelfReference ), }); case "id": { return selectorParser.id({ value: exportScopedName( node.value, - node.raws && node.raws.value ? node.raws.value : null + node.raws && node.raws.value ? node.raws.value : null, + exportSelfReference ), }); } @@ -141,7 +149,7 @@ const plugin = (options = {}) => { ); } - function traverseNode(node) { + function traverseNode(node, exportSelfReference) { switch (node.type) { case "pseudo": if (node.value === ":local") { @@ -149,7 +157,7 @@ const plugin = (options = {}) => { throw new Error('Unexpected comma (",") in :local block'); } - const selector = localizeNode(node.first); + const selector = localizeNode(node.first, exportSelfReference); // move the spaces that were around the psuedo selector to the first // non-container node selector.first.spaces = node.spaces; @@ -172,7 +180,7 @@ const plugin = (options = {}) => { /* falls through */ case "root": case "selector": { - node.each(traverseNode); + node.each((n) => traverseNode(n, exportSelfReference)); break; } case "id": @@ -197,8 +205,15 @@ const plugin = (options = {}) => { // Find any :local selectors root.walkRules((rule) => { let parsedSelector = selectorParser().astSync(rule); + const containsOwnDeclarations = rule.nodes.some( + (node) => + node.type !== "comment" && !/^compose(s|-with)$/i.test(node.prop) + ); - rule.selector = traverseNode(parsedSelector.clone()).toString(); + rule.selector = traverseNode( + parsedSelector.clone(), + exportEmptyLocals || containsOwnDeclarations + ).toString(); rule.walkDecls(/composes|compose-with/i, (decl) => { const localNames = getSingleLocalNamesForComposes(parsedSelector); @@ -249,7 +264,7 @@ const plugin = (options = {}) => { const input = localMatch.input; const matchPattern = localMatch[0]; const matchVal = localMatch[1]; - const newVal = exportScopedName(matchVal); + const newVal = exportScopedName(matchVal, undefined, true); result = input.replace(matchPattern, newVal); } else { @@ -274,11 +289,13 @@ const plugin = (options = {}) => { return; } - atRule.params = exportScopedName(localMatch[1]); + atRule.params = exportScopedName(localMatch[1], undefined, true); }); // If we found any :locals, insert an :export rule - const exportedNames = Object.keys(exports); + const exportedNames = Object.keys(exports).filter( + (exportedName) => exports[exportedName].length !== 0 + ); if (exportedNames.length > 0) { const exportRule = rule({ selector: ":export" }); diff --git a/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/expected.error.txt b/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/expected.error.txt new file mode 100644 index 0000000..21e1788 --- /dev/null +++ b/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/expected.error.txt @@ -0,0 +1 @@ +referenced class name "otherClassName" in composes not found diff --git a/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/options.js b/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/options.js new file mode 100644 index 0000000..f64076a --- /dev/null +++ b/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/options.js @@ -0,0 +1,3 @@ +module.exports = { + exportEmptyLocals: false, +}; diff --git a/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/source.css b/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/source.css new file mode 100644 index 0000000..d572771 --- /dev/null +++ b/test/test-cases/error-composes-not-defined-class-option-exportEmptyLocals-false/source.css @@ -0,0 +1,3 @@ +:local(.className) { + composes: otherClassName; +} diff --git a/test/test-cases/options-exportEmptyLocals-false/expected.css b/test/test-cases/options-exportEmptyLocals-false/expected.css new file mode 100644 index 0000000..7691792 --- /dev/null +++ b/test/test-cases/options-exportEmptyLocals-false/expected.css @@ -0,0 +1,34 @@ +/* leaf node with contents */ +._input__layer1A { + color: red; +} + +._input__layer2A /* doesn't add anything new */ { +} + +._input__layer1B { + /* totally empty, except for this comment */ +} + +._input__layer2B { + background: blue; +} + +._input__layer3 { +} + +._input__foo > ._input__bar { +} + +._input__baz > ._input__qux { + font-style: italic; +} + +:export { + layer1A: _input__layer1A; + layer2A: _input__layer1A; + layer2B: _input__layer2B; + layer3: _input__layer1A _input__layer2B; + baz: _input__baz; + qux: _input__qux; +} diff --git a/test/test-cases/options-exportEmptyLocals-false/options.js b/test/test-cases/options-exportEmptyLocals-false/options.js new file mode 100644 index 0000000..f64076a --- /dev/null +++ b/test/test-cases/options-exportEmptyLocals-false/options.js @@ -0,0 +1,3 @@ +module.exports = { + exportEmptyLocals: false, +}; diff --git a/test/test-cases/options-exportEmptyLocals-false/source.css b/test/test-cases/options-exportEmptyLocals-false/source.css new file mode 100644 index 0000000..022cf15 --- /dev/null +++ b/test/test-cases/options-exportEmptyLocals-false/source.css @@ -0,0 +1,29 @@ +/* leaf node with contents */ +:local(.layer1A) { + color: red; +} + +:local(.layer2A) /* doesn't add anything new */ { + composes: layer1A; +} + +:local(.layer1B) { + /* totally empty, except for this comment */ +} + +:local(.layer2B) { + background: blue; + composes: layer1B; +} + +:local(.layer3) { + composes: layer2A; + composes: layer2B; +} + +:local(.foo) /* empty */ > :local(.bar) { +} + +:local(.baz) /* non-empty */ > :local(.qux) { + font-style: italic; +}