Browse Source

ref(eslint): Consolidate `import` rules (#82798)

These are all the recommended `import` rules:
https://github.com/import-js/eslint-plugin-import/blob/main/config/flat/recommended.js

But typescript has us covered for 5/6 of them:
https://typescript-eslint.io/troubleshooting/typed-linting/performance/#slow-eslint-rules

So we don't need the recommended set. Instead, lets just enable the ones
we want. And anything that's set to `'off'` we can remove from the
config.
Ryan Albrecht 2 months ago
parent
commit
046cec4ad3

+ 50 - 129
eslint.config.mjs

@@ -209,133 +209,7 @@ const baseRules = {
   ],
 };
 
-const reactImportRules = {
-  // Not recommended to be enabled with typescript-eslint
-  // https://typescript-eslint.io/linting/troubleshooting/performance-troubleshooting/#eslint-plugin-import
-  'import/no-unresolved': ['off'],
-  'import/named': ['off'],
-  'import/default': ['off'],
-  'import/export': ['off'],
-  'import/no-named-as-default-member': ['off'],
-
-  // Redflags
-  // do not allow a default import name to match a named export (airbnb: error)
-  // Issue with `DefaultIssuePlugin` and `app/plugins/index`
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-named-as-default.md
-  'import/no-named-as-default': ['off'],
-
-  // disallow use of jsdoc-marked-deprecated imports
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-deprecated.md
-  'import/no-deprecated': ['off'],
-
-  // Forbid mutable exports (airbnb: error)
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-mutable-exports.md
-  // TODO: enable?
-  'import/no-mutable-exports': ['off'],
-
-  // disallow require()
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-commonjs.md
-  'import/no-commonjs': ['off'],
-
-  // disallow AMD require/define
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-amd.md
-  'import/no-amd': ['error'],
-
-  // disallow duplicate imports
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-duplicates.md
-  'import/no-duplicates': ['error'],
-
-  // disallow namespace imports
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-namespace.md
-  'import/no-namespace': ['off'],
-
-  // Ensure consistent use of file extension within the import path
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/extensions.md
-  // TODO this fucks up getsentry
-  'import/extensions': [
-    'off',
-    'always',
-    {
-      js: 'never',
-      jsx: 'never',
-    },
-  ],
-
-  // Require a newline after the last import/require in a group
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/newline-after-import.md
-  'import/newline-after-import': ['error'],
-
-  // Require modules with a single export to use a default export (airbnb: error)
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/prefer-default-export.md
-  'import/prefer-default-export': ['off'],
-
-  // Restrict which files can be imported in a given folder
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-restricted-paths.md
-  'import/no-restricted-paths': ['off'],
-
-  // Forbid modules to have too many dependencies
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/max-dependencies.md
-  'import/max-dependencies': ['off', {max: 10}],
-
-  // Forbid import of modules using absolute paths
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-absolute-path.md
-  'import/no-absolute-path': ['error'],
-
-  // Forbid require() calls with expressions (airbnb: error)
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-dynamic-require.md
-  'import/no-dynamic-require': ['off'],
-
-  // Use webpack default chunk names
-  'import/dynamic-import-chunkname': ['off'],
-
-  // prevent importing the submodules of other modules
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-internal-modules.md
-  'import/no-internal-modules': [
-    'off',
-    {
-      allow: [],
-    },
-  ],
-
-  // Warn if a module could be mistakenly parsed as a script by a consumer
-  // leveraging Unambiguous JavaScript Grammar
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/unambiguous.md
-  // this should not be enabled until this proposal has at least been *presented* to TC39.
-  // At the moment, it"s not a thing.
-  'import/unambiguous': ['off'],
-
-  // Forbid Webpack loader syntax in imports
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-webpack-loader-syntax.md
-  'import/no-webpack-loader-syntax': ['error'],
-
-  // Prevent unassigned imports
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-unassigned-import.md
-  // importing for side effects is perfectly acceptable, if you need side effects.
-  'import/no-unassigned-import': ['off'],
-
-  // Prevent importing the default as if it were named
-  // https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-named-default.md
-  'import/no-named-default': ['error'],
-
-  // Reports if a module"s default export is unnamed
-  // https://github.com/benmosher/eslint-plugin-import/blob/d9b712ac7fd1fddc391f7b234827925c160d956f/docs/rules/no-anonymous-default-export.md
-  'import/no-anonymous-default-export': [
-    'error',
-    {
-      allowArray: false,
-      allowArrowFunction: false,
-      allowAnonymousClass: false,
-      allowAnonymousFunction: false,
-      allowCallExpression: true,
-      allowLiteral: false,
-      allowObject: false,
-    },
-  ],
-};
-
 const reactRules = {
-  ...reactImportRules,
-
   /**
    * Custom
    */
@@ -483,7 +357,6 @@ const appRules = {
    * Better import sorting
    */
   'sort-imports': 'off',
-  'import/order': 'off',
   'simple-import-sort/imports': [
     'error',
     {
@@ -712,15 +585,63 @@ export default typescript.config([
    *   ...myPlugin.configs.recommended,
    *   rules: {
    *     ...myPlugin.configs.recommended.rules,
-   *     ['the-rule']: 'warning',
+   *     ['the-rule']: 'warn',
    *   }
    * },
    * Finally, once all warnings are fixed, update from 'warning' to 'error', or
    * remove the override and rely on the recommended rules again.
    */
   {
-    name: 'import/recommended',
+    name: 'import',
     ...importPlugin.flatConfigs.recommended,
+    rules: {
+      // We override all the rules that are in the recommended, react, and typescript rulesets
+
+      // From the recommended ruleset:
+      // https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/export.md
+      'import/export': 'error',
+
+      // 5 rules not recommended to be enabled with typescript-eslint
+      // https://typescript-eslint.io/troubleshooting/typed-linting/performance/#slow-eslint-rules
+      'import/named': 'off',
+      'import/namespace': 'off',
+      'import/default': 'off',
+      'import/no-named-as-default-member': 'off',
+      'import/no-unresolved': 'off',
+
+      // Require a newline after the last import/require in a group
+      // Why doesn't prettier handle this? https://prettier.io/docs/en/rationale.html#empty-lines
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/newline-after-import.md
+      'import/newline-after-import': 'error',
+
+      // do not allow a default import name to match a named export (airbnb: error)
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-named-as-default.md
+      'import/no-named-as-default': 'off',
+
+      // Prevent importing the default as if it were named
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-named-default.md
+      'import/no-named-default': 'error',
+
+      // disallow AMD require/define
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-amd.md
+      'import/no-amd': 'error',
+
+      // disallow duplicate imports
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md
+      'import/no-duplicates': 'error',
+
+      // Forbid import of modules using absolute paths
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-absolute-path.md
+      'import/no-absolute-path': 'error',
+
+      // Forbid Webpack loader syntax in imports
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-webpack-loader-syntax.md
+      'import/no-webpack-loader-syntax': 'error',
+
+      // Reports if a module"s default export is unnamed
+      // https://github.com/benmosher/eslint-plugin-import/blob/main/docs/rules/no-anonymous-default-export.md
+      'import/no-anonymous-default-export': 'error',
+    },
   },
   {
     name: 'deprecations',

+ 0 - 1
static/app/bootstrap/initializeLocale.tsx

@@ -72,7 +72,6 @@ export async function initializeLocale(config: Config) {
     // No need to import english
     if (languageCode !== 'en') {
       await import(`moment/locale/${languageCode}`);
-      // eslint-disable-next-line import/namespace
       moment.locale(languageCode);
     }
   } catch (err) {

+ 0 - 1
static/app/icons/icons.stories.tsx

@@ -1319,7 +1319,6 @@ function Section({section}: {section: TSection}) {
       <Grid style={{gridTemplateColumns: 'repeat(4, 1fr)'}}>
         {section.icons.map(icon => {
           const name = icon.name.startsWith('Icon') ? icon.name : `Icon${icon.name}`;
-          // eslint-disable-next-line import/namespace
           const Component = Icons[name];
 
           const props = {color: 'gray500', size: 'sm', ...icon.defaultProps};

+ 0 - 1
static/app/views/dashboards/contexts/widgetSyncContext.tsx

@@ -28,7 +28,6 @@ export function WidgetSyncContextProvider({
 }: WidgetSyncContextProviderProps) {
   const register: RegistrationFunction = chart => {
     chart.group = groupName;
-    // eslint-disable-next-line import/namespace
     echarts?.connect(groupName);
   };
 

+ 0 - 1
static/app/views/dashboards/detail.spec.tsx

@@ -636,7 +636,6 @@ describe('Dashboards > Detail', function () {
 
     it('hides add widget option', async function () {
       // @ts-expect-error this is assigning to readonly property...
-      // eslint-disable-next-line import/namespace
       types.MAX_WIDGETS = 1;
 
       render(

+ 3 - 1
tests/js/sentry-test/reactTestingLibrary.tsx

@@ -210,13 +210,15 @@ function waitForDrawerToHide(ariaLabel: string) {
  */
 instrumentUserEvent();
 
-// eslint-disable-next-line no-restricted-imports
+// eslint-disable-next-line no-restricted-imports, import/export
 export * from '@testing-library/react';
 
 export {
+  // eslint-disable-next-line import/export
   render,
   renderGlobalModal,
   userEvent,
+  // eslint-disable-next-line import/export
   fireEvent,
   waitForDrawerToHide,
   makeAllTheProviders,