Merge pull request #1 from A7med3bdulBaset/modifiers

feat: detect physical properties with important flag or modifires
AhmedBaset · Sep 8, 2023 · 0545f1c · 0545f1c
2 parents c924acb + 31af094
commit 0545f1c
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,22 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+
+Nothing yet.
+
+## [0.2.0] - 2023-09-08
+
+### Fixed
+
+- `no-physical-properties` rule now correctly handles classes with !important like `!pl-1` flag and modifiers like `hover:` (#1)
+
+## [0.1.0] - 2023-08-24
+
+### Added
+
+- `no-physical-properties` rule
diff --git a/README.md b/README.md
@@ -143,6 +143,7 @@ Welcome your contribution!
 ## TODO:
 
 - [x] Tailwindcss physical properties to logical properties
+- [ ] Add support for advanced className like `cn('pl-2', {...})`[.](https://github.com/francoismassart/eslint-plugin-tailwindcss/blob/6b6c0dd28e123cc118bff83654f951f736fa58e8/lib/rules/no-arbitrary-value.js#L169)
 - [ ] Strict `<html>` to have dir attribute depending on a codition or whatever detecting the language
 - [ ] Strict `<code>` to have `dir="ltr"` to override the parent's direction
 - [ ] in the future maybe throw a warning that `letter-spacing` doesn't work well with RTL languages to disable it in rtl `rtl:***` (NOT SURE)

diff --git a/docs/rules/no-physical-properties.md b/docs/rules/no-physical-properties.md
@@ -10,13 +10,13 @@ This rule enforces the use of css logical properties instead of physical propert
 ## Examples of incorrect code for this rule:
 
 ```jsx
-<div className="ml-1 mr-2 pl-8 pr-2 left-0 right-12 text-left border-l-2 rounded-l-sm"></div>
+<div className="ml-1 !mr-2 sm:pl-8 hover:pr-2 data-[state=active]:left-0 hover:!right-12 text-left border-l-2 rounded-l-sm"></div>
 ```
 
 ## Examples of correct code for this rule:
 
 ```jsx
-<div className="ms-1 me-2 ps-8 pe-2 start-0 end-12 text-start border-s-2 rounded-s-sm"></div>
+<div className="ms-1 !me-2 sm:ps-8 hover:pe-2 data-[state=active]:start-0 hover:!end-12 text-start border-s-2 rounded-s-sm"></div>
 ```
 
 ## To automatically fix all of the problems reported by this rule

diff --git a/src/rules/no-physical-properties.ts b/src/rules/no-physical-properties.ts
@@ -2,6 +2,11 @@ import { Rule } from 'eslint';
 import { logicalProperties } from '../configs/tw-logical-properties';
 import { JSXAttribute } from 'estree-jsx';
 
+/**
+ * **TODO** Refactor this ugly code
+ * **TODO** Add support for `className={cn('ms-1', 'me-2')}`
+ */
+
 const exampleRule: Rule.RuleModule = {
   meta: {
     type: 'suggestion',
@@ -42,7 +47,17 @@ const ruleListener = (ctx: Rule.RuleContext) => {
     const PH_CNs = logicalProperties.map((c) => c.physical);
 
     const conflictClassNames = cnArr.filter((cn) =>
-      PH_CNs.some((c) => cn.startsWith(c))
+      PH_CNs.some((c) => {
+        let isValid = false;
+        [
+          new RegExp(`^${c}.*`),
+          new RegExp(`!${c}.*`),
+          new RegExp(`.+:${c}.*`),
+        ].forEach((regex) => {
+          if (regex.test(cn)) isValid = true;
+        });
+        return isValid;
+      })
     );
 
     if (!conflictClassNames.length) return;
@@ -54,10 +69,19 @@ const ruleListener = (ctx: Rule.RuleContext) => {
         invalid: conflictClassNames.join(' '),
         valid: conflictClassNames
           .map((cn) => {
-            const { logical, physical } = logicalProperties.find((c) =>
-              cn.startsWith(c.physical)
-            )!;
-            return cn.replace(physical, logical);
+            const prop = logicalProperties.find((c) => {
+              let isValid = false;
+              [
+                new RegExp(`^${c.physical}.*`),
+                new RegExp(`!${c.physical}.*`),
+                new RegExp(`.+:${c.physical}.*`),
+              ].forEach((regex) => {
+                if (regex.test(cn)) isValid = true;
+              });
+              return isValid;
+            });
+            if (!prop) return cn;
+            return cn.replace(prop.physical, prop.logical);
           })
           .join(' '),
       },
@@ -66,10 +90,19 @@ const ruleListener = (ctx: Rule.RuleContext) => {
         const fixedClassName = cnArr
           .map((cn) => {
             if (conflictClassNames.includes(cn)) {
-              const { logical, physical } = logicalProperties.find((c) =>
-                cn.startsWith(c.physical)
-              )!;
-              return cn.replace(physical, logical);
+              const prop = logicalProperties.find((c) => {
+                let isValid = false;
+                [
+                  new RegExp(`^${c.physical}.*`),
+                  new RegExp(`!${c.physical}.*`),
+                  new RegExp(`.+:${c.physical}.*`),
+                ].forEach((regex) => {
+                  if (regex.test(cn)) isValid = true;
+                });
+                return isValid;
+              });
+              if (!prop) return cn;
+              return cn.replace(prop.physical, prop.logical);
             }
             return cn;
           })

diff --git a/tests/rules/no-physical-properties.ts b/tests/rules/no-physical-properties.ts
@@ -16,9 +16,17 @@ tester.run('no-physical-properties', logicalProperties, {
       code: `<div className="ms-1 me-2 ps-8 pe-2 start-0 end-12 text-start border-s-2 rounded-e-sm scroll-ms-4 scroll-pe-4">text</div>`,
     },
     {
-      name: 'should work fine with `class` attribute',
+      name: 'should work well with `class` attribute',
       code: `<div class="ms-1 me-2 ps-8 pe-2 start-0 end-12 text-start border-s-2 rounded-e-sm scroll-ms-4 scroll-pe-4">text</div>`,
-    }
+    }, 
+    {
+      name: 'should work well with the important flag',
+      code: `<div className="!ms-1 !me-2 !ps-8 !pe-2 !start-0 !end-12 !text-start !border-s-2 !rounded-e-sm !scroll-ms-4 !scroll-pe-4 !!important">text</div>`,
+    },
+    {
+      name: 'should work well with the prefixes flag',
+      code: `<div className="sm:ms-1 md:me-2 lg:ps-8 xl:pe-2 hover:start-0 group-hover:end-12 @sm:text-start [&>svg]:border-s-2 data-[state=active]:rounded-e-sm supports-[display:flex]:scroll-ms-4 has-[.block]:scroll-pe-4 aria-[hidden]:!important">text</div>`,
+    },
   ],
   invalid: [
     {
@@ -69,5 +77,35 @@ tester.run('no-physical-properties', logicalProperties, {
       output: `<div className="scroll-ms-1 scroll-me-2 scroll-ps-1 scroll-pe-1">text</div>`,
       errors: [{ messageId: 'noPhysicalProperties' }],
     },
+    {
+      name: 'should report if physical properties are used with the important flag and fix them',
+      code: `<div className="!pl-0">text</div>`,
+      output: `<div className="!ps-0">text</div>`,
+      errors: [{ messageId: 'noPhysicalProperties' }],
+    },
+    {
+      name: 'should report if physical properties are used with modifiers and fix them',
+      code: `<div className="sm:ml-1 md:mr-2 lg:pl-1 xl:pr-1">text</div>`,
+      output: `<div className="sm:ms-1 md:me-2 lg:ps-1 xl:pe-1">text</div>`,
+      errors: [{ messageId: 'noPhysicalProperties' }],
+    },
+    {
+      name: 'should report if physical properties are used with modifiers and fix them',
+      code: `<div className="hover:ml-1 focus:mr-2 focus-within:pl-1 @md:pr-1">text</div>`,
+      output: `<div className="hover:ms-1 focus:me-2 focus-within:ps-1 @md:pe-1">text</div>`,
+      errors: [{ messageId: 'noPhysicalProperties' }],
+    },
+    {
+      name: 'should report if physical properties are used with modifiers and fix them',
+      code: `<div className="group-hover:ml-1 data-[state=active]:mr-2 [&>svg]:pl-1 group-[anything]:pr-1">text</div>`,
+      output: `<div className="group-hover:ms-1 data-[state=active]:me-2 [&>svg]:ps-1 group-[anything]:pe-1">text</div>`,
+      errors: [{ messageId: 'noPhysicalProperties' }],
+    },
+    {
+      name: 'should report if physical properties are used with important flag and modifiers and fix them',
+      code: `<div className="md:!pl-0 hover:!mr-[23]">text</div>`,
+      output: `<div className="md:!ps-0 hover:!me-[23]">text</div>`,
+      errors: [{ messageId: 'noPhysicalProperties' }],
+    },
   ],
 });