Improve declaration overriding logic

Author: elchininet

.gitignore

@@ -10,4 +10,6 @@ coverage/
 /index.d.ts
 /index.js
 /options.d.ts
-/options.js
+/options.js
+.yalc/
+yalc.lock

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Changelog
 
-## [1.8.0] - 2021-05-23
+## [1.8.1] - 2022-07-24
+
+- Improve the overriding logic to avoid edge cases with multiple overrides
+
+## [1.8.0] - 2022-07-23
 
 - Add an option to have more control over the selector prefixing logic (`prefixSelectorTransformer`)
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "postcss-rtlcss",
-  "version": "1.8.0",
+  "version": "1.8.1",
   "description": "PostCSS plugin to build Cascading Style Sheets (CSS) with Left-To-Right (LTR) and Right-To-Left (RTL) rules",
   "keywords": [
     "postcss",

src/@types/index.ts

@@ -1,4 +1,8 @@
-import { Rule, AtRule } from 'postcss';
+import {
+    Rule,
+    AtRule,
+    Declaration
+} from 'postcss';
 
 export interface ObjectWithProps<T> {
     [key: string]: T;
@@ -103,4 +107,6 @@ export interface ControlDirective {
     block?: string;
     directive: string;
     option?: string;
-}
+}
+
+export type DeclarationHashMapProp = Record<string, Record<string, { decl: Declaration; value: string; }>>;

src/parsers/declarations.ts

@@ -1,6 +1,17 @@
-import postcss, { Rule, Node, Declaration, Comment, vendor } from 'postcss';
+import postcss, {
+    Rule,
+    Node,
+    Declaration,
+    Comment,
+    vendor
+} from 'postcss';
 import rtlcss from 'rtlcss';
-import { Mode, Autorename, ControlDirective } from '@types';
+import {
+    Mode,
+    Autorename,
+    ControlDirective,
+    DeclarationHashMapProp
+} from '@types';
 import {
     DECLARATION_TYPE,
     FLIP_PROPERTY_REGEXP,
@@ -19,7 +30,9 @@ import {
     allDeclarations,
     initialValues,
     appendDeclarationToRule,
-    hasIgnoreDirectiveInRaws
+    hasIgnoreDirectiveInRaws,
+    hasSameUpcomingDeclaration,
+    hasMirrorDeclaration
 } from '@utilities/declarations';
 import { walkContainer } from '@utilities/containers';
 import {
@@ -52,10 +65,15 @@ export const parseDeclarations = (
     const ruleBoth = ruleFlipped.clone();
     const ruleSafe = ruleFlipped.clone();   
 
-    const declarationHashMap = Array.prototype.reduce.call(rule.nodes, (obj: Record<string, string>, node: Node): Record<string, string> => {
+    const declarationHashMap = Array.prototype.reduce.call(rule.nodes, (obj: DeclarationHashMapProp, node: Node): DeclarationHashMapProp => {
         if (node.type === DECLARATION_TYPE) {
             const decl = node as Declaration;
-            obj[decl.prop] = decl.value.trim();
+            const index = rule.index(decl);
+            obj[decl.prop] = obj[decl.prop] || {};
+            obj[decl.prop][index] = {
+                decl,
+                value: decl.value.trim()
+            };
         }
         return obj;
     }, {});
@@ -168,6 +186,10 @@ export const parseDeclarations = (
             ) {
                 return;
             }
+
+            if (hasSameUpcomingDeclaration(rule, decl, declarationHashMap)) {
+                return;
+            }
 
             if (isAnimation) {
 
@@ -237,7 +259,7 @@ export const parseDeclarations = (
                         deleteDeclarations.push(decl);
                     }
                     return;                   
-                } else  if (declarationHashMap[declFlipped.prop] === declFlippedValue) {
+                } else  if (hasMirrorDeclaration(rule, declFlipped, declarationHashMap)) {
                     simetricRules = true;
                     if (isConflictedDeclaration && !hasIgnoreDirectiveInRaws(decl)) {
                         appendDeclarationToRule(decl, ruleSafe);                       

src/utilities/declarations.ts

@@ -1,5 +1,9 @@
 import { Rule, Declaration } from 'postcss';
-import { ObjectWithProps, DeclarationsData } from '@types';
+import {
+    ObjectWithProps,
+    DeclarationsData,
+    DeclarationHashMapProp
+} from '@types';
 import { COMMENT_TYPE, RTL_COMMENT_IGNORE_REGEXP } from '@constants';
 import shorthandDeclarationsJson from '@data/shorthand-declarations.json';
 import notShorthandDeclarationsJson from '@data/not-shorthand-declarations.json';
@@ -58,10 +62,42 @@ const hasIgnoreDirectiveInRaws = (decl: Declaration): boolean => {
     return false;
 };
 
+const hasSameUpcomingDeclaration = (
+    rule: Rule,
+    decl: Declaration,
+    declarationHashMap: DeclarationHashMapProp
+): boolean => {
+    const index = rule.index(decl);
+    const indexes = Object.keys(declarationHashMap[decl.prop]).map(Number);
+    if (indexes.length === 1) {
+        return false;
+    }
+    return indexes.some((i: number) => i > index);
+};
+
+const hasMirrorDeclaration = (
+    rule: Rule,
+    declFlipped: Declaration,
+    declarationHashMap: DeclarationHashMapProp
+): boolean => {
+    if (declarationHashMap[declFlipped.prop]) {
+        const entries = Object.entries(declarationHashMap[declFlipped.prop]);
+        return entries.some((entry) => {
+            return (
+                entry[1].value === declFlipped.value.trim() &&
+                !hasSameUpcomingDeclaration(rule, entry[1].decl, declarationHashMap)
+            );
+        });
+    }
+    return false;
+};
+
 export {
     declarations,
     allDeclarations,
     initialValues,
     appendDeclarationToRule,
-    hasIgnoreDirectiveInRaws
+    hasIgnoreDirectiveInRaws,
+    hasSameUpcomingDeclaration,
+    hasMirrorDeclaration
 };

tests/__snapshots__/combined-autorename.test.ts.snap

@@ -25,17 +25,7 @@ exports[`Combined Tests Autorename Combined Autorename: flexible 1`] = `
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }
 
@@ -624,17 +614,7 @@ exports[`Combined Tests Autorename Combined Autorename: flexible with custom str
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }
 
@@ -1223,17 +1203,7 @@ exports[`Combined Tests Autorename Combined Autorename: flexible, greedy: true 1
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }
 
@@ -1822,17 +1792,7 @@ exports[`Combined Tests Autorename Combined Autorename: only control directives
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }
 
@@ -2421,17 +2381,7 @@ exports[`Combined Tests Autorename Combined Autorename: only control directives,
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }
 
@@ -3020,17 +2970,7 @@ exports[`Combined Tests Autorename Combined Autorename: strict 1`] = `
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }
 
@@ -3619,17 +3559,7 @@ exports[`Combined Tests Autorename Combined Autorename: strict, greedy: true 1`]
 
 .test2 {
     color: red;
-}
-
-[dir=\\"ltr\\"] .test2 {
     text-align: left;
-}
-
-[dir=\\"rtl\\"] .test2 {
-    text-align: right;
-}
-
-[dir] .test2 {
     text-align: center;
 }