ci: Remove duplicate eslint rule definitions (#82260)

Favoring the most-strict rules. I removed the no-import rules for: - enzyme - react-bootstrap because those npm packages are fully removed from the app, the rule doesnt apply anymore I also removed `no-danger-with-children` which seemed like a typo of `react/no-danger-with-children` Next step is to convert the file to eslint.config.js format. References getsentry/frontend-tsc#82
getsentry · Dec 17, 2024 · 6c87e19 · 6c87e19
1 parent 4b5389d
commit 6c87e19
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 123 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -152,20 +152,25 @@ const baseRules = {
   // https://eslint.org/docs/rules/radix
   radix: ['error'],
 
+  // Disabled because of prettier
   // https://eslint.org/docs/rules/space-in-brackets.html
-  'computed-property-spacing': ['error', 'never'],
+  'computed-property-spacing': ['off'],
 
+  // Disabled because of prettier
   // https://eslint.org/docs/rules/space-in-brackets.html
-  'array-bracket-spacing': ['error', 'never'],
+  'array-bracket-spacing': ['off'],
 
+  // Disabled because of prettier
   // https://eslint.org/docs/rules/space-in-brackets.html
-  'object-curly-spacing': ['error', 'never'],
+  'object-curly-spacing': ['off'],
 
+  // TODO(ryan953): Enable this rule
   // https://eslint.org/docs/rules/object-shorthand
-  'object-shorthand': ['error', 'properties'],
+  'object-shorthand': ['off', 'properties'],
 
+  // Disabled because of prettier
   // https://eslint.org/docs/rules/space-infix-ops.html
-  'space-infix-ops': ['error'],
+  'space-infix-ops': ['off'],
 
   // https://eslint.org/docs/rules/vars-on-top
   'vars-on-top': ['off'],
@@ -458,7 +463,10 @@ const reactRules = {
   /**
    * React hooks
    */
-  'react-hooks/exhaustive-deps': 'error',
+  'react-hooks/exhaustive-deps': [
+    'error',
+    {additionalHooks: '(useEffectAfterFirstRender|useMemoWithPrevious)'},
+  ],
   // Biome not yet enforcing all parts of this rule https://github.com/biomejs/biome/issues/1984
   'react-hooks/rules-of-hooks': 'error',
 
@@ -554,12 +562,13 @@ const appRules = {
   'no-restricted-imports': [
     'error',
     {
-      paths: [
+      patterns: [
         {
-          name: 'enzyme',
-          message:
-            'Please import from `sentry-test/enzyme` instead. See: https://github.com/getsentry/frontend-handbook#undefined-theme-properties-in-tests for more information',
+          group: ['sentry/components/devtoolbar/*'],
+          message: 'Do not depend on toolbar internals',
         },
+      ],
+      paths: [
         {
           name: '@testing-library/react',
           message:
@@ -585,7 +594,6 @@ const appRules = {
           message:
             "Please import marked from 'app/utils/marked' so that we can ensure sanitation of marked output",
         },
-
         {
           name: 'lodash',
           message:
@@ -596,11 +604,6 @@ const appRules = {
           message:
             'Optional chaining `?.` and nullish coalescing operators `??` are available and preferred over using `lodash/get`. See https://github.com/getsentry/frontend-handbook#new-syntax for more information',
         },
-        {
-          name: 'react-bootstrap',
-          message:
-            'Avoid usage of any react-bootstrap components as it will soon be removed',
-        },
         {
           name: 'sentry/utils/theme',
           importNames: ['lightColors', 'darkColors'],
@@ -619,6 +622,14 @@ const appRules = {
           message:
             "Use 'useLocation', 'useParams', 'useNavigate', 'useRoutes' from sentry/utils instead.",
         },
+        {
+          name: 'qs',
+          message: 'Please use query-string instead of qs',
+        },
+        {
+          name: 'moment',
+          message: 'Please import moment-timezone instead of moment',
+        },
       ],
     },
   ],
@@ -827,98 +838,6 @@ module.exports = {
     ...reactRules,
     ...appRules,
     ...strictRules,
-    'react-hooks/rules-of-hooks': 'error',
-    'react-hooks/exhaustive-deps': [
-      'error',
-      {additionalHooks: '(useEffectAfterFirstRender|useMemoWithPrevious)'},
-    ],
-    'no-restricted-imports': [
-      'error',
-      {
-        patterns: [
-          {
-            group: ['sentry/components/devtoolbar/*'],
-            message: 'Do not depend on toolbar internals',
-          },
-        ],
-        paths: [
-          {
-            name: '@testing-library/react',
-            message:
-              'Please import from `sentry-test/reactTestingLibrary` instead so that we can ensure consistency throughout the codebase',
-          },
-          {
-            name: '@testing-library/react-hooks',
-            message:
-              'Please import from `sentry-test/reactTestingLibrary` instead so that we can ensure consistency throughout the codebase',
-          },
-          {
-            name: '@testing-library/user-event',
-            message:
-              'Please import from `sentry-test/reactTestingLibrary` instead so that we can ensure consistency throughout the codebase',
-          },
-          {
-            name: '@sentry/browser',
-            message:
-              'Please import from `@sentry/react` to ensure consistency throughout the codebase.',
-          },
-          {
-            name: 'marked',
-            message:
-              "Please import marked from 'app/utils/marked' so that we can ensure sanitation of marked output",
-          },
-          {
-            name: 'lodash',
-            message:
-              "Please import lodash utilities individually. e.g. `import isEqual from 'lodash/isEqual';`. See https://github.com/getsentry/frontend-handbook#lodash from for information",
-          },
-          {
-            name: 'lodash/get',
-            message:
-              'Optional chaining `?.` and nullish coalescing operators `??` are available and preferred over using `lodash/get`. See https://github.com/getsentry/frontend-handbook#new-syntax for more information',
-          },
-          {
-            name: 'sentry/utils/theme',
-            importNames: ['lightColors', 'darkColors'],
-            message:
-              "'lightColors' and 'darkColors' exports intended for use in Storybook only. Instead, use theme prop from emotion or the useTheme hook.",
-          },
-          {
-            name: 'react-router',
-            importNames: ['withRouter'],
-            message:
-              "Use 'useLocation', 'useParams', 'useNavigate', 'useRoutes' from sentry/utils instead.",
-          },
-          {
-            name: 'sentry/utils/withSentryRouter',
-            importNames: ['withSentryRouter'],
-            message:
-              "Use 'useLocation', 'useParams', 'useNavigate', 'useRoutes' from sentry/utils instead.",
-          },
-          {
-            name: 'qs',
-            message: 'Please use query-string instead of qs',
-          },
-          {
-            name: 'moment',
-            message: 'Please import moment-timezone instead of moment',
-          },
-        ],
-      },
-    ],
-
-    // TODO(@anonrig): Remove this from eslint-sentry-config
-    'space-infix-ops': 'off',
-    'object-shorthand': 'off',
-    'object-curly-spacing': 'off',
-    'import/no-amd': 'off',
-    'no-danger-with-children': 'off',
-    'no-fallthrough': 'off',
-    'no-obj-calls': 'off',
-    'array-bracket-spacing': 'off',
-    'computed-property-spacing': 'off',
-    'react/no-danger-with-children': 'off',
-    'jest/no-disabled-tests': 'off',
   },
   // JSON file formatting is handled by Biome. ESLint should not be linting
   // and formatting these files.
@@ -931,6 +850,7 @@ module.exports = {
           'error',
           {
             paths: [
+              ...appRules['no-restricted-imports'][1].paths,
               {
                 name: 'sentry/utils/queryClient',
                 message:
@@ -949,18 +869,6 @@ module.exports = {
         ...reactRules,
         ...appRules,
         ...strictRules,
-        // TODO(@anonrig): Remove this from eslint-sentry-config
-        'space-infix-ops': 'off',
-        'object-shorthand': 'off',
-        'object-curly-spacing': 'off',
-        'import/no-amd': 'off',
-        'no-danger-with-children': 'off',
-        'no-fallthrough': 'off',
-        'no-obj-calls': 'off',
-        'array-bracket-spacing': 'off',
-        'computed-property-spacing': 'off',
-        'react/no-danger-with-children': 'off',
-        'jest/no-disabled-tests': 'off',
       },
     },
     {

diff --git a/biome.json b/biome.json
@@ -65,7 +65,6 @@
         "noMisrefactoredShorthandAssign": "error",
         "useAwait": "error",
         "useNamespaceKeyword": "error",
-        "noSkippedTests": "error",
         "noFocusedTests": "error",
         "noDuplicateTestHooks": "error"
       },

diff --git a/static/app/views/issueList/customViewsHeader.spec.tsx b/static/app/views/issueList/customViewsHeader.spec.tsx
@@ -354,7 +354,7 @@ describe('CustomViewsHeader', () => {
       );
     });
 
-    // biome-ignore lint/suspicious/noSkippedTests: <This behavior works when testing in browser, need to debug why its failing tests>
+    // eslint-disable-next-line jest/no-disabled-tests
     it.skip('retains unsaved changes after switching tabs', async () => {
       render(<CustomViewsIssueListHeader {...defaultProps} router={unsavedTabRouter} />, {
         router: unsavedTabRouter,
@@ -725,7 +725,7 @@ describe('CustomViewsHeader', () => {
         expect(unsavedTabRouter.push).not.toHaveBeenCalled();
       });
 
-      // biome-ignore lint/suspicious/noSkippedTests: Works in browser, unclear why its not passing this test
+      // eslint-disable-next-line jest/no-disabled-tests
       it.skip('should save changes when hitting ctrl+s', async () => {
         const mockPutRequest = MockApiClient.addMockResponse({
           url: `/organizations/org-slug/group-search-views/`,