Fix ESLint config #5132

mcmire · 2025-01-11T00:54:53Z

Explanation

The upgrade to ESLint 9 which occurred in a previous commit was not performed correctly. It seems that an array must be passed to createConfig instead of a series of arguments, and that was not done. As a result, a lot of lint rules were disabled accidentally.

This commit fixes the ESLint config and corrects any lint violations. Unsurprisingly, upgrading the ESLint packages created a bunch of new lint violations, so those have been turned into warnings rather than errors in this commit. To prevent new instances of these violations from occurring, however, a script has been added which acts as a quality gate, running eslint as before, but then capturing existing warning counts and comparing them to a previously generated threshold file, exiting with non-zero if there is an increase in warnings for any particular ESLint rule. This script is also now used for yarn lint:eslint so that CI will fail if the count increases.

References

Changelog

N/A (developer change only)

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've highlighted breaking changes using the "BREAKING" category above as appropriate
I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

socket-security · 2025-01-11T00:55:48Z

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/[email protected] 🔁 npm/[email protected]	None	`0`	85.8 kB	diego

View full report↗︎

The upgrade to ESLint 9 which occurred in a previous commit was not done correctly. It seems that an array must be passed to `createConfig` instead of a series of arguments. As a result, a lot of lint rules were accidentally disabled. This commit fixes the ESLint config and corrects any lint violations. Unsurprisingly, upgrading the ESLint packages created a bunch of new lint violations, so those have been turned into warnings rather than errors in this commit. To prevent new instances of these violations from occurring, however, a script has been added which acts as a quality gate, running `eslint` as before, but then capturing existing warning counts and comparing them to a previously generated threshold file, exiting with non-zero if there is an increase in warnings for any particular ESLint rule. This script is also now used for `yarn lint:eslint` so that CI will fail if the count increases.

mcmire · 2025-01-13T19:37:39Z

eslint.config.mjs

@@ -62,45 +58,54 @@ const config = createConfig(
  },
  {
    files: [
-      '**/jest.config.js',


This list of globs should be functionally equivalent as we had some overlap previously.

mcmire · 2025-01-13T19:39:40Z

eslint.config.mjs

    ],
    extends: [nodejs],
+    rules: {
+      // TODO: Re-enable this
+      'n/no-sync': 'off',


This rule was previously disabled earlier in this file but the Node rules may re-enable this rule so we here we explicitly disable it.

mcmire · 2025-01-13T19:40:27Z

eslint.config.mjs

  },
  {
-    files: ['*.test.{ts,js}', '**/tests/**/*.{ts,js}'],


Globs changed in ESLint 9. You need to prepend the glob with **/ if you want to look for files in any directory, not just the root.

mcmire · 2025-01-13T19:41:21Z

eslint.config.mjs

+        // Disable `projectService` because we run into out-of-memory issues.
+        // See this ticket for inspiration out how to solve this:
+        // <https://github.com/typescript-eslint/typescript-eslint/issues/1192>
+        projectService: false,


Disabling projectService is inconsistent with the module template, but I had to do this in order to even run eslint. Disabling projectService does not seem to make eslint run more slowly.

mcmire · 2025-01-13T19:42:21Z

eslint.config.mjs

@@ -110,6 +115,8 @@ const config = createConfig(
      '@typescript-eslint/promise-function-async': 'off',

      // TODO: re-enable most of these rules
+      '@typescript-eslint/explicit-function-return-type': 'off',
+      '@typescript-eslint/naming-convention': 'off',


This rule was previously disabled earlier in this file but the TypeScript rules may re-enable this rule so we here we explicitly disable it.

mcmire · 2025-01-13T19:55:22Z

packages/keyring-controller/src/KeyringController.ts

@@ -1580,7 +1580,6 @@ export class KeyringController extends BaseController<
   * @deprecated Use `withKeyring` instead.
   */
  async cancelQRSynchronization(): Promise<void> {
-    // eslint-disable-next-line n/no-sync


I am unsure why the Node rules were getting applied to these files. ESLint was reporting that this rule no longer applied, so I removed it. (Similar for other cases.)

mcmire · 2025-01-13T19:55:39Z

packages/permission-controller/src/PermissionController.ts

@@ -1982,7 +1982,6 @@ export class PermissionController<
    target: string,
  ): void {
    if (!isPlainObject(caveat)) {
-      // eslint-disable-next-line @typescript-eslint/no-throw-literal


ESLint reported that this rule no longer applied.

mcmire · 2025-01-13T19:56:02Z

packages/selected-network-controller/tests/SelectedNetworkController.test.ts

@@ -44,7 +44,7 @@ function buildMessenger() {
 * @param options.getSubjectNames - Permissions controller list of domains with permissions
 * @returns The network controller restricted messenger.
 */
-export function buildSelectedNetworkControllerMessenger({


ESLint was reporting that this should not be exported. It was not used as far as I could tell.

mcmire · 2025-01-13T19:57:38Z

packages/message-manager/src/AbstractMessageManager.ts

@@ -3,7 +3,7 @@ import { BaseControllerV1 } from '@metamask/base-controller';
 import type { ApprovalType } from '@metamask/controller-utils';
 import type { Hex, Json } from '@metamask/utils';
 // This package purposefully relies on Node's EventEmitter module.
-// eslint-disable-next-line import/no-nodejs-modules
+// eslint-disable-next-line import-x/no-nodejs-modules


We are now using eslint-plugin-import-x, a fork of eslint-plugin-import.

mcmire · 2025-01-13T19:58:34Z

packages/phishing-controller/src/PhishingDetector.test.ts

@@ -1110,7 +1110,7 @@ describe('PhishingDetector', () => {
        ];

        // CID should be blocked
-        for await (const entry of expectedToBeBlocked) {


The await was never necessary here and ESLint now reports that it should be removed.

mcmire requested review from a team as code owners January 11, 2025 00:54

mcmire marked this pull request as draft January 13, 2025 15:40

mcmire force-pushed the fix-eslint branch 6 times, most recently from b74fbee to 94f738a Compare January 13, 2025 20:18

mcmire marked this pull request as ready for review January 13, 2025 20:21

mcmire force-pushed the fix-eslint branch from 94f738a to dbd5ce8 Compare January 13, 2025 20:35

mcmire commented Jan 13, 2025

View reviewed changes

owencraston approved these changes Jan 13, 2025

View reviewed changes

gambinish approved these changes Jan 13, 2025

View reviewed changes

Mrtenz approved these changes Jan 13, 2025

View reviewed changes

mirceanis approved these changes Jan 14, 2025

View reviewed changes

imblue-dabadee approved these changes Jan 14, 2025

View reviewed changes

Merge branch 'main' into fix-eslint

26fdb83

matthewwalsh0 approved these changes Jan 14, 2025

View reviewed changes

jiexi approved these changes Jan 14, 2025

View reviewed changes

Merge branch 'main' into fix-eslint

4c0f1f1

mcmire enabled auto-merge (squash) January 14, 2025 17:59

mikesposito approved these changes Jan 14, 2025

View reviewed changes

Merge branch 'main' into fix-eslint

70ae4db

mcmire merged commit 7409b3f into main Jan 14, 2025
226 checks passed

mcmire deleted the fix-eslint branch January 14, 2025 18:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ESLint config #5132

Fix ESLint config #5132

mcmire commented Jan 11, 2025 •

edited

Loading

socket-security bot commented Jan 11, 2025 •

edited

Loading

mcmire Jan 13, 2025

mcmire Jan 13, 2025

mcmire Jan 13, 2025

mcmire Jan 13, 2025 •

edited

Loading

mcmire Jan 13, 2025

mcmire Jan 13, 2025

mcmire Jan 13, 2025

mcmire Jan 13, 2025 •

edited

Loading

mcmire Jan 13, 2025

mcmire Jan 13, 2025

Fix ESLint config #5132

Fix ESLint config #5132

Conversation

mcmire commented Jan 11, 2025 • edited Loading

Explanation

References

Changelog

Checklist

socket-security bot commented Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire commented Jan 11, 2025 •

edited

Loading

socket-security bot commented Jan 11, 2025 •

edited

Loading

mcmire Jan 13, 2025 •

edited

Loading

mcmire Jan 13, 2025 •

edited

Loading