From a9a91e7eef989faba6ae8f81ca384beec4a2a7e8 Mon Sep 17 00:00:00 2001 From: Ulyanov Ivan Date: Wed, 30 Oct 2024 02:32:59 +0400 Subject: [PATCH] Correction of the work with comments and general improvements (#163) * Changed the if to switch construction in the Rules method. * The duplicate code has been removed. * Simplified the atruleNames function * Minor changes. * Added a fix for not deleting comments * Changing the tests Added a test to check the work with comments The existing tests have been changed because they contained incorrect data. * Added code for processing comments in the rules. * Updated tests. * Added processing for new lines. * Updated tests, added the use of new lines * The readability of the test has been improved. * Added a test to save the father's comment * Edit to correct the lint error. --- index.js | 158 +++++++++++++++++++++++++++++--------------------- index.test.js | 41 +++++++++++-- 2 files changed, 129 insertions(+), 70 deletions(-) diff --git a/index.js b/index.js index 79831b6..89489b3 100644 --- a/index.js +++ b/index.js @@ -53,12 +53,12 @@ function interpolateAmpInSelector(nodes, parent) { */ function mergeSelectors(parent, child) { let merged = [] - parent.selectors.forEach(sel => { + for (let sel of parent.selectors) { let parentNode = parse(sel, parent) - child.selectors.forEach(selector => { + for (let selector of child.selectors) { if (!selector) { - return + continue } let node = parse(selector, child) let replaced = interpolateAmpInSelector(node, parentNode) @@ -67,22 +67,43 @@ function mergeSelectors(parent, child) { node.prepend(parentNode.clone({})) } merged.push(node.toString()) - }) - }) + } + } return merged } /** - * Move a child and its preceeding comment(s) to after "after" + * Move a child and its preceding comment(s) to after "after" + * ! It is necessary to clarify the comment */ function breakOut(child, after) { - let prev = child.prev() - after.after(child) - while (prev && prev.type === 'comment') { - let nextPrev = prev.prev() - after.after(prev) - prev = nextPrev + let changeParent = true + + for (let node of after.nodes) { + if (!node.nodes) continue + + let prevNode = node.prev() + if (prevNode?.type !== 'comment') continue + + let parentRule = after.toString() + + /* Checking that the comment "describes" the rule following. Like this: + /* comment about the rule below /* + .rule {} + */ + let regexp = new RegExp(`${prevNode.toString()} *\n *${node.toString()}`) + + if (parentRule.match(regexp)) { + changeParent = false + after.after(node).after(prevNode) + } + } + + // It is necessary if the above child has never been moved + if (changeParent) { + after.after(child) } + return child } @@ -104,14 +125,12 @@ function createFnAtruleChilds(bubble) { children.push(child) } }) - if (bubbling) { - if (children.length) { - let clone = rule.clone({ nodes: [] }) - for (let child of children) { - clone.append(child) - } - atrule.prepend(clone) + if (bubbling && children.length) { + let clone = rule.clone({ nodes: [] }) + for (let child of children) { + clone.append(child) } + atrule.prepend(clone) } } } @@ -125,16 +144,23 @@ function pickDeclarations(selector, declarations, after) { after.after(parent) return parent } +function pickAndClearDeclarations(ruleSelector, declarations, after, clear = true) { + if (!declarations.length) return [after, declarations] -function atruleNames(defaults, custom) { - let list = {} - for (let name of defaults) { - list[name] = true + after = pickDeclarations(ruleSelector, declarations, after) + + if (clear) { + declarations = [] } - if (custom) { - for (let name of custom) { - list[name.replace(/^@/, '')] = true - } + + return [after, declarations] +} + +function atruleNames(defaults, custom = '') { + let names = defaults.concat(custom) + let list = {} + for (let name of names) { + list[name.replace(/^@/, '')] = true } return list } @@ -297,10 +323,10 @@ module.exports = (opts = {}) => { postcssPlugin: 'postcss-nested', RootExit(root) { - if (root[hasRootRule]) { - root.walkAtRules(rootRuleName, unwrapRootRule) - root[hasRootRule] = false - } + if (!root[hasRootRule]) return + + root.walkAtRules(rootRuleName, unwrapRootRule) + root[hasRootRule] = false }, Rule(rule) { @@ -310,46 +336,48 @@ module.exports = (opts = {}) => { let declarations = [] rule.each(child => { - if (child.type === 'rule') { - if (declarations.length) { - after = pickDeclarations(rule.selector, declarations, after) - declarations = [] - } - - copyDeclarations = true - unwrapped = true - child.selectors = mergeSelectors(rule, child) - after = breakOut(child, after) - } else if (child.type === 'atrule') { - if (declarations.length) { - after = pickDeclarations(rule.selector, declarations, after) - declarations = [] - } - if (child.name === rootRuleName) { - unwrapped = true - atruleChilds(rule, child, true, child[rootRuleMergeSel]) - after = breakOut(child, after) - } else if (bubble[child.name]) { - copyDeclarations = true - unwrapped = true - atruleChilds(rule, child, true) - after = breakOut(child, after) - } else if (unwrap[child.name]) { + switch (child.type) { + case 'rule': + [after, declarations] = pickAndClearDeclarations(rule.selector, declarations, after) + copyDeclarations = true unwrapped = true - atruleChilds(rule, child, false) + child.selectors = mergeSelectors(rule, child) after = breakOut(child, after) - } else if (copyDeclarations) { - declarations.push(child) - } - } else if (child.type === 'decl' && copyDeclarations) { - declarations.push(child) + + break + case 'atrule': + [after, declarations] = pickAndClearDeclarations(rule.selector, declarations, after) + + if (child.name === rootRuleName) { + unwrapped = true + atruleChilds(rule, child, true, child[rootRuleMergeSel]) + after = breakOut(child, after) + } else if (bubble[child.name]) { + copyDeclarations = true + unwrapped = true + atruleChilds(rule, child, true) + after = breakOut(child, after) + } else if (unwrap[child.name]) { + copyDeclarations = true + unwrapped = true + atruleChilds(rule, child, false) + after = breakOut(child, after) + } else if (copyDeclarations) { + declarations.push(child) + } + + break + case 'decl': + if (copyDeclarations) { + declarations.push(child) + } + + break } }) - if (declarations.length) { - after = pickDeclarations(rule.selector, declarations, after) - } + pickAndClearDeclarations(rule.selector, declarations, after, false) if (unwrapped && preserveEmpty !== true) { rule.raws.semicolon = true diff --git a/index.test.js b/index.test.js index deaafa3..a18af98 100644 --- a/index.test.js +++ b/index.test.js @@ -604,16 +604,47 @@ test('clears empty selector after comma', () => { run('a, b { .one, .two, {} }', 'a .one, a .two, b .one, b .two {}') }) -test('moves comment with rule', () => { - run('a { /*B*/ /*B2*/ b {} }', '/*B*/ /*B2*/ a b {}') +test("Save the parent's comment", () => { + run('a { /*i*/ b {} }', 'a { /*i*/ } a b {}') }) -test('moves comment with at-rule', () => { - run('a { /*B*/ @media { one: 1 } }', '/*B*/ @media {a { one: 1 } }') +test("Save the parent's comment with newline", () => { + run( + `a { + /*i*/ + + b {} }`, + `a { /*i*/ } a b {}` + ) +}) + +test('Save the comments for the parent and child', () => { + run( + `a { + /*i*/ + /*o*/ + b {} }`, + + `a { /*i*/ } /*o*/ a b {}` + ) +}) + +test('Save the comments for the parent and child with at-rule', () => { + run( + `a { /*i*/ + /*o*/ + @media { one: 1 } }`, + + `a { /*i*/ } /*o*/ @media {a { one: 1 } }` + ) }) test('moves comment with declaration', () => { - run('a { @media { /*B*/ one: 1 } }', '@media {a { /*B*/ one: 1 } }') + run('a { @media { /*B*/ one: 1 } }', '@media { a { /*B*/ one: 1 } }') +}) + +test('moves comment with declaration without properties', () => { + run('a { @media { /*B*/ } }', '@media { a { /*B*/ } }') }) test('saves order of rules', () => {