[8.x] Resolve false positives with eslint no color rule (#204848) (#2…

…04906) # Backport This will backport the following commits from `main` to `8.x`: - [Resolve false positives with eslint no color rule (#204848)](#204848)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) ","sha":"7cbd9b0c1d19358470ad4c8a36dcc0a02b46f532","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:SharedUX","backport:prev-minor"],"title":"Resolve false positives with eslint no color rule ","number":204848,"url":"https://github.com/elastic/kibana/pull/204848","mergeCommit":{"message":"Resolve false positives with eslint no color rule (#204848)\n\n## Summary\r\n\r\nFixes error in lint rule that resulted in false positives, also added a\r\ntest case to ascertain the issue has been fixed. For context the error\r\nhappens in instances where specific CSS declarations that are\r\nderivatives of shorthand declarations that can apply color to the HTML\r\nelement or text nodes where found, because the check we had simply\r\nchecked if we got a string back instead of asserting that it was a falsy\r\nvalue.\r\n\r\n## Before\r\n![Screenshot 2024-12-17 at 10 27\r\n18 PM](https://github.com/user-attachments/assets/b0918d37-22f6-4778-a6d0-2cafe11b18e1)\r\n\r\n![Screenshot 2024-12-17 at 8 03\r\n33 PM](https://github.com/user-attachments/assets/d70c733d-e88f-42d6-956a-e266d53724f9)\r\n\r\n## After\r\n\r\n<img width=\"755\" alt=\"Screenshot 2024-12-19 at 10 25 41\"\r\nsrc=\"https://github.com/user-attachments/assets/3e334785-d657-46ac-86c7-59d37f176c86\"\r\n/>\r\n\r\n\r\n<img width=\"915\" alt=\"Screenshot 2024-12-19 at 10 23 33\"\r\nsrc=\"https://github.com/user-attachments/assets/87860189-92c5-4807-b5b0-2520b9e75778\"\r\n/>\r\n\r\n","sha":"7cbd9b0c1d19358470ad4c8a36dcc0a02b46f532"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204848","number":204848,"mergeCommit":{"message":"Resolve false positives with eslint no color rule (#204848)\n\n## Summary\r\n\r\nFixes error in lint rule that resulted in false positives, also added a\r\ntest case to ascertain the issue has been fixed. For context the error\r\nhappens in instances where specific CSS declarations that are\r\nderivatives of shorthand declarations that can apply color to the HTML\r\nelement or text nodes where found, because the check we had simply\r\nchecked if we got a string back instead of asserting that it was a falsy\r\nvalue.\r\n\r\n## Before\r\n![Screenshot 2024-12-17 at 10 27\r\n18 PM](https://github.com/user-attachments/assets/b0918d37-22f6-4778-a6d0-2cafe11b18e1)\r\n\r\n![Screenshot 2024-12-17 at 8 03\r\n33 PM](https://github.com/user-attachments/assets/d70c733d-e88f-42d6-956a-e266d53724f9)\r\n\r\n## After\r\n\r\n<img width=\"755\" alt=\"Screenshot 2024-12-19 at 10 25 41\"\r\nsrc=\"https://github.com/user-attachments/assets/3e334785-d657-46ac-86c7-59d37f176c86\"\r\n/>\r\n\r\n\r\n<img width=\"915\" alt=\"Screenshot 2024-12-19 at 10 23 33\"\r\nsrc=\"https://github.com/user-attachments/assets/87860189-92c5-4807-b5b0-2520b9e75778\"\r\n/>\r\n\r\n","sha":"7cbd9b0c1d19358470ad4c8a36dcc0a02b46f532"}}]}] BACKPORT--> Co-authored-by: Eyo O. Eyo <[email protected]>
elastic · Dec 19, 2024 · 91115ab · 91115ab
1 parent f1e12df
commit 91115ab
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 15 deletions.
diff --git a/packages/kbn-eslint-plugin-css/src/rules/no_css_color.test.ts b/packages/kbn-eslint-plugin-css/src/rules/no_css_color.test.ts
@@ -45,7 +45,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
     code: `
     import React from 'react';
-    
+
     function TestComponent() {
       return (
         <EuiCode style={{ color: '#dd4040' }}>This is a test</EuiCode>
@@ -86,7 +86,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
     code: `
     import React from 'react';
-    
+
     function TestComponent() {
       const baseStyle = { background: 'rgb(255, 255, 255)' };
 
@@ -116,7 +116,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
     code: `
     import React from 'react';
-    
+
     function TestComponent() {
       return (
         <EuiCode style={{ background: '#dd4040' }}>This is a test</EuiCode>
@@ -129,7 +129,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
     code: `
     import React from 'react';
-    
+
     function TestComponent() {
       return (
         <EuiCode css={{ color: '#dd4040' }}>This is a test</EuiCode>
@@ -153,7 +153,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     code: `
     import React from 'react';
     import { css } from '@emotion/css';
-    
+
     function TestComponent() {
       return (
         <EuiCode css={css\` color: #dd4040; \`}>This is a test</EuiCode>
@@ -171,7 +171,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     const codeCss = css({
       color: '#dd4040',
     })
-    
+
     function TestComponent() {
       return (
         <EuiCode css={codeCss}>This is a test</EuiCode>
@@ -187,7 +187,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     import { css } from '@emotion/css';
 
     const codeCss = css\` color: #dd4040; \`
-    
+
     function TestComponent() {
       return (
         <EuiCode css={codeCss}>This is a test</EuiCode>
@@ -200,7 +200,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
     code: `
     import React from 'react';
-    
+
     function TestComponent() {
       return (
         <EuiCode css={() => ({ color: '#dd4040' })}>This is a test</EuiCode>
@@ -213,7 +213,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
     code: `
     import React from 'react';
-    
+
     function TestComponent() {
       return (
         <EuiCode css={function () { return { color: '#dd4040' }; }}>This is a test</EuiCode>
@@ -227,7 +227,7 @@ const invalid: RuleTester.InvalidTestCase[] = [
     code: `
     import React from 'react';
     import { css } from '@emotion/css';
-    
+
     function TestComponent() {
       return (
         <EuiCode className={css\` color: #dd4040; \`}>This is a test</EuiCode>
@@ -237,7 +237,24 @@ const invalid: RuleTester.InvalidTestCase[] = [
   },
 ];
 
-const valid: RuleTester.ValidTestCase[] = [];
+const valid: RuleTester.ValidTestCase[] = [
+  {
+    name: 'Does not raise an error when a CSS color is not used in a JSX css prop attribute',
+    filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
+    code: `
+    import React from 'react';
+    import { EuiCode } from '@elastic/eui';
+    import { css } from '@emotion/react';
+    function TestComponent() {
+      return (
+        <EuiCode css={css\`
+            border-top: none;
+            border-radius: 0 0 6px 6px;
+          \`}>This is a test</EuiCode>
+      )
+    }`,
+  },
+];
 
 for (const [name, tester] of [tsTester, babelTester]) {
   describe(name, () => {

diff --git a/packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts b/packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts
@@ -30,8 +30,8 @@ const checkPropertySpecifiesInvalidCSSColor = ([property, value]: string[]) => {
 
   const style = new CSSStyleDeclaration();
 
-  // @ts-ignore the types for this packages specifics an index signature of number, alongside other valid CSS properties
-  style[property] = value;
+  // @ts-ignore the types for this packages specifies an index signature of number, alongside other valid CSS properties
+  style[property.trim()] = typeof value === 'string' ? value.trim() : value;
 
   const anchor = propertiesSupportingCssColor.find((resolvedProperty) =>
     property.includes(resolvedProperty)
@@ -42,9 +42,9 @@ const checkPropertySpecifiesInvalidCSSColor = ([property, value]: string[]) => {
   // build the resolved color property to check if the value is a string after parsing the style declaration
   const resolvedColorProperty = anchor === 'color' ? 'color' : anchor + 'Color';
 
-  // in trying to keep this rule simple, it's enough if a string is used to define a color to mark it as invalid
+  // in trying to keep this rule simple, it's enough if we get a value back, because if it was an identifier we would have been able to set a value within this invocation
   // @ts-ignore the types for this packages specifics an index signature of number, alongside other valid CSS properties
-  return typeof style[resolvedColorProperty] === 'string';
+  return Boolean(style[resolvedColorProperty]);
 };
 
 const resolveMemberExpressionRoot = (node: TSESTree.MemberExpression): TSESTree.Identifier => {