simplify the algorithm and test cases

Author: romainmenke

plugins/postcss-nesting/CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ### Unreleased (patch)
 
-- Fix reorder of declarations inside nested `@mixin` rules
+- Fix order of final CSS with complex usage of both nesting and mixins, by @pciarach
 
 ### 12.0.3
 

plugins/postcss-nesting/dist/index.cjs

@@ -1,5 +1,4 @@
 import type { ChildNode, Container } from 'postcss';
-import { isAtRule, isMixinRule, isRule } from "./is-type-of-rule";
 
 export default function groupDeclarations(node: Container<ChildNode>) {
 	// https://drafts.csswg.org/css-nesting/#mixing
@@ -10,60 +9,54 @@ export default function groupDeclarations(node: Container<ChildNode>) {
 	// For the purpose of determining the Order Of Appearance,
 	// nested style rules and nested conditional group rules are considered to come after their parent rule.
 
-	let indexOfLastDeclarationInFirstDeclarationList = -1;
+	const declarationLikeThings: Array<ChildNode> = [];
+	const ruleLikeThings: Array<ChildNode> = [];
 
-	node.each((child, index) => {
-		if (child.type === 'decl') {
-			if (indexOfLastDeclarationInFirstDeclarationList === index - 1) {
-				indexOfLastDeclarationInFirstDeclarationList = index;
-				return;
-			}
-
-			child.remove();
-			node.insertAfter(indexOfLastDeclarationInFirstDeclarationList, child);
-			indexOfLastDeclarationInFirstDeclarationList = node.index(child);
+	node.each((child) => {
+		if (isDeclarationLike(child, ruleLikeThings.length > 0)) {
+			declarationLikeThings.push(child);
 			return;
 		}
 
-		if (isMixinRule(child)) {
-			let prev = child.prev();
-			// We assume that
-			// - a mixin after declarations will resolve to more declarations
-			// - a mixin after rules or at-rules will resolve to more rules or at-rules (except after another mixin)
-			while (prev) {
-				if ((prev.type === 'rule' || (isAtRule(prev) && !isMixinRule(prev)))) {
-					return;
-				}
-
-				prev = prev.prev();
+		if (child.type === 'comment') {
+			let next = child.next();
+			while (next && next.type === 'comment') {
+				next = next.next();
 			}
 
-			if (indexOfLastDeclarationInFirstDeclarationList === index - 1) {
-				indexOfLastDeclarationInFirstDeclarationList = index;
+			if (isDeclarationLike(next, ruleLikeThings.length > 0)) {
+				declarationLikeThings.push(child);
 				return;
 			}
-
-			child.remove();
-			node.insertAfter(indexOfLastDeclarationInFirstDeclarationList, child);
-			indexOfLastDeclarationInFirstDeclarationList = node.index(child);
-			return;
 		}
 
-		if (child.type === 'comment') {
-			const next = child.next();
-			if (next && (next.type === 'comment' || isRule(next) || (isAtRule(next) && !isMixinRule(next)))) {
-				return;
-			}
+		ruleLikeThings.push(child);
+	});
 
-			if (indexOfLastDeclarationInFirstDeclarationList === index - 1) {
-				indexOfLastDeclarationInFirstDeclarationList = index;
-				return;
-			}
+	node.removeAll();
 
-			child.remove();
-			node.insertAfter(indexOfLastDeclarationInFirstDeclarationList, child);
-			indexOfLastDeclarationInFirstDeclarationList = node.index(child);
-			return;
-		}
+	declarationLikeThings.forEach((child) => {
+		node.append(child);
 	});
+
+	ruleLikeThings.forEach((child) => {
+		node.append(child);
+	});
+}
+
+function isDeclarationLike(node: ChildNode | undefined, didSeeRuleLikeThings: boolean): boolean {
+	if (!node) {
+		return false;
+	}
+
+	if (node.type === 'decl') {
+		return true;
+	}
+
+	// We assume that
+	// - a mixin after declarations will resolve to declarations
+	// - a mixin after rules or at-rules will resolve to rules or at-rules
+	return node.type === 'atrule' &&
+		node.name.toLowerCase() === 'mixin' &&
+		!didSeeRuleLikeThings;
 }

plugins/postcss-nesting/dist/index.mjs

@@ -8,10 +8,6 @@ export function isNestRule(node?: Node): node is AtRule {
 	return node && isAtRule(node) && node.name === 'nest';
 }
 
-export function isMixinRule(node?: Node): node is AtRule {
-	return node && isAtRule(node) && node.name.toLowerCase() === 'mixin';
-}
-
 export function isRule(node?: Node): node is Rule {
 	return node && node.type === 'rule';
 }

plugins/postcss-nesting/src/lib/group-declarations.ts

@@ -32,11 +32,11 @@ const mixinPluginNestedRules = () => {
 		postcssPlugin: 'mixin',
 		AtRule: {
 			mixin(node, { postcss }) {
-				if (node.params === 'nestedMixins') {
-					node.replaceWith(postcss.parse('@mixin mixinWithDecl; @mixin mixinToOverride; &:disabled { color: white; }', {from : 'mixin.css'}));
-				} else if (node.params === 'mixinWithDecl') {
+				if (node.params === 'alpha') {
+					node.replaceWith(postcss.parse('@mixin alpha-1; @mixin alpha-2; & { color: white; }', {from : 'mixin.css'}));
+				} else if (node.params === 'alpha-1') {
 					node.replaceWith(postcss.parse('color: blue;', {from : 'mixin.css'}));
-				} else if (node.params === 'mixinToOverride') {
+				} else if (node.params === 'alpha-2') {
 					node.replaceWith(postcss.parse('display: flex;', {from : 'mixin.css'}));
 				}
 			},
@@ -191,10 +191,6 @@ postcssTape(plugin)({
 		message: 'supports mixin with nested rules',
 		plugins: [mixinPluginNestedRules(), plugin()],
 	},
-	'mixin-nested-rules-after-media': {
-		message: 'supports mixin with nested rules',
-		plugins: [mixinPluginNestedRules(), plugin()],
-	},
 	'spec-examples': {
 		message: 'supports all spec examples',
 	},

plugins/postcss-nesting/src/lib/is-type-of-rule.ts

@@ -1019,8 +1019,6 @@ article {
 		This declaration is preserved
 	*/
 	color: red;
-
-	/* valid! */
 }
 
 article {
@@ -1030,3 +1028,5 @@ article {
 article.foo {
 		color: yellow;
 	}
+
+/* valid! */

plugins/postcss-nesting/test/_tape.mjs

@@ -1019,8 +1019,6 @@ article {
 		This declaration is preserved
 	*/
 	color: red;
-
-	/* valid! */
 }
 
 article {
@@ -1030,3 +1028,5 @@ article {
 article.foo {
 		color: yellow;
 	}
+
+/* valid! */

plugins/postcss-nesting/test/at-nest.expect.css

@@ -1019,8 +1019,6 @@ article {
 		This declaration is preserved
 	*/
 	color: red;
-
-	/* valid! */
 }
 
 article {
@@ -1030,3 +1028,5 @@ article {
 article.foo {
 		color: yellow;
 	}
+
+/* valid! */

plugins/postcss-nesting/test/at-nest.no-is-pseudo-selector.expect.css

@@ -1,8 +1,6 @@
 a {
-	color: yellow;
-
 	& b {
-		@mixin nestedMixins;
+		@mixin alpha;
 		display: inline-flex;
 	}
 }

plugins/postcss-nesting/test/at-nest.silent.expect.css

@@ -1,7 +1,5 @@
-a {
-	color: yellow;
-}
-a b {color: blue;display: flex;
+
+	a b {color: blue;display: flex;
 		display: inline-flex;
 	}
-a b:disabled { color: white; }
+a b { color: white; }

plugins/postcss-nesting/test/mixin-nested-rules-after-media.css

@@ -5,10 +5,10 @@
 
 a .in.deep { color: blue; }
 
-/* a comment */
-
 f g {
 		color: red;
 	}
 
+/* a comment */
+
 f .in.deep { color: blue; }

plugins/postcss-nesting/test/mixin-nested-rules-after-media.expect.css

@@ -5,10 +5,10 @@
 
 a .in.deep { color: blue; }
 
-/* a comment */
-
 f g {
 		color: red;
 	}
 
+/* a comment */
+
 f .in.deep { color: blue; }

plugins/postcss-nesting/test/mixin-nested-rules.css

@@ -376,8 +376,6 @@ article {
 		This declaration is preserved
 	*/
 	color: red;
-
-	/* valid! */
 }
 
 article {
@@ -387,3 +385,5 @@ article {
 article.foo {
 		color: yellow;
 	}
+
+/* valid! */

plugins/postcss-nesting/test/mixin-nested-rules.expect.css

@@ -376,8 +376,6 @@ article {
 		This declaration is preserved
 	*/
 	color: red;
-
-	/* valid! */
 }
 
 article {
@@ -387,3 +385,5 @@ article {
 article.foo {
 		color: yellow;
 	}
+
+/* valid! */