From 5f1f877a5be75d244d1fce5ada24f0cb77771467 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 15 Dec 2023 10:29:00 -0700 Subject: [PATCH 1/3] Only enable Node lint rules for Node files In a previous commit, we mistakenly introduced a Node-specific function for testing deep equality, and this ended up crashing the extension. This would usually have been caught by our ESLint rules, as they prohibit use of Node libraries by default. However, the ESLint configuration for this repo imports rules from our `@metamask/eslint-config-nodejs` package and applies them to all files, marking everything as Node-compatible, thereby allowing such usage. This is incorrect: these rules should only apply to test files and scripts. This commit fix the ESLint configuration to match. Doing so revealed a couple of categories of violations, which this commit also fixes: - Some packages import and make use of EventEmitter from Node's `events` module. We add a notice to the README for these packages which advises consumers that these packages are designed to be used in a Node-compatible environment. - Some packages import and make use of Node's `assert` module to check values at runtime. It isn't strictly necessary to use this module, and so we replace its usage with simpler code. - Some packages import and make use of Node's `inspect` utility function. We don't strictly need this either, and we replace its usage with simpler code as well. --- .eslintrc.js | 17 ++++- packages/message-manager/README.md | 4 ++ .../src/AbstractMessageManager.ts | 2 + .../src/NetworkController.ts | 69 ++++++++----------- .../user-storage/UserStorageController.ts | 2 + packages/signature-controller/README.md | 4 ++ .../src/SignatureController.ts | 2 + packages/transaction-controller/README.md | 4 ++ .../src/TransactionController.ts | 2 + .../src/helpers/GasFeePoller.ts | 2 + .../src/helpers/IncomingTransactionHelper.ts | 2 + .../src/helpers/PendingTransactionTracker.ts | 2 + packages/user-operation-controller/README.md | 4 ++ .../src/UserOperationController.ts | 2 + .../helpers/PendingUserOperationTracker.ts | 2 + scripts/create-package/utils.test.ts | 9 ++- scripts/create-package/utils.ts | 4 +- 17 files changed, 87 insertions(+), 46 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index f71ff43b3ed..4b28719e520 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,9 +1,9 @@ module.exports = { root: true, - extends: ['@metamask/eslint-config', '@metamask/eslint-config-nodejs'], + extends: ['@metamask/eslint-config'], ignorePatterns: [ '!.eslintrc.js', - '!jest.config.js', + '!.prettierrc.js', 'node_modules', '**/dist', '**/docs', @@ -12,6 +12,19 @@ module.exports = { 'scripts/create-package/package-template', ], overrides: [ + { + files: [ + '**/jest.config.js', + '**/jest.environment.js', + '**/tests/**/*.{ts,js}', + '*.js', + '*.test.{ts,js}', + 'scripts/*.ts', + 'scripts/create-package/*.ts', + 'yarn.config.cjs', + ], + extends: ['@metamask/eslint-config-nodejs'], + }, { files: ['*.test.{ts,js}', '**/tests/**/*.{ts,js}'], extends: ['@metamask/eslint-config-jest'], diff --git a/packages/message-manager/README.md b/packages/message-manager/README.md index 54c03863d65..f49f66919a7 100644 --- a/packages/message-manager/README.md +++ b/packages/message-manager/README.md @@ -10,6 +10,10 @@ or `npm install @metamask/message-manager` +## Compatibility + +This package relies implicitly upon the `EventEmitter` module. This module is available natively in Node.js, but when using this package for the browser, make sure to use a polyfill such as `events`. + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 9ede9ce7a10..bdd8401f54c 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -2,6 +2,8 @@ import type { BaseConfig, BaseState } from '@metamask/base-controller'; 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 import { EventEmitter } from 'events'; import { v1 as random } from 'uuid'; diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 6b42d86d273..91b5df59d16 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -19,13 +19,11 @@ import { errorCodes } from '@metamask/rpc-errors'; import { createEventEmitterProxy } from '@metamask/swappable-obj-proxy'; import type { SwappableProxy } from '@metamask/swappable-obj-proxy'; import type { Hex } from '@metamask/utils'; -import { isStrictHexString, hasProperty, isPlainObject } from '@metamask/utils'; -import { strict as assert } from 'assert'; +import { hasProperty, isPlainObject, isStrictHexString } from '@metamask/utils'; import type { Draft } from 'immer'; import type { Logger } from 'loglevel'; import { createSelector } from 'reselect'; import * as URI from 'uri-js'; -import { inspect } from 'util'; import { v4 as uuidV4 } from 'uuid'; import { INFURA_BLOCKED_KEY, NetworkStatus } from './constants'; @@ -788,9 +786,9 @@ function validateNetworkControllerState(state: NetworkState) { if (!networkClientIds.includes(state.selectedNetworkClientId)) { throw new Error( - `NetworkController state is invalid: \`selectedNetworkClientId\` ${inspect( - state.selectedNetworkClientId, - )} does not refer to an RPC endpoint within a network configuration`, + // False negative - this is a string. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `NetworkController state is invalid: \`selectedNetworkClientId\` '${state.selectedNetworkClientId}' does not refer to an RPC endpoint within a network configuration`, ); } } @@ -1354,19 +1352,20 @@ export class NetworkController extends BaseController< * removed in a future release */ async setProviderType(type: InfuraNetworkType) { - assert.notStrictEqual( - type, - NetworkType.rpc, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `NetworkController - cannot call "setProviderType" with type "${NetworkType.rpc}". Use "setActiveNetwork"`, - ); - assert.ok( - isInfuraNetworkType(type), - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Unknown Infura provider type "${type}".`, - ); + if ((type as unknown) === NetworkType.rpc) { + throw new Error( + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `NetworkController - cannot call "setProviderType" with type "${NetworkType.rpc}". Use "setActiveNetwork"`, + ); + } + if (!isInfuraNetworkType(type)) { + throw new Error( + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `Unknown Infura provider type "${type}".`, + ); + } await this.setActiveNetwork(type); } @@ -1635,9 +1634,7 @@ export class NetworkController extends BaseController< if (existingNetworkConfiguration === undefined) { throw new Error( - `Could not update network: Cannot find network configuration for chain ${inspect( - chainId, - )}`, + `Could not update network: Cannot find network configuration for chain '${chainId}'`, ); } @@ -1899,7 +1896,7 @@ export class NetworkController extends BaseController< if (existingNetworkConfiguration === undefined) { throw new Error( - `Cannot find network configuration for chain ${inspect(chainId)}`, + `Cannot find network configuration for chain '${chainId}'`, ); } @@ -2031,9 +2028,7 @@ export class NetworkController extends BaseController< !isSafeChainId(networkFields.chainId) ) { throw new Error( - `${errorMessagePrefix}: Invalid \`chainId\` ${inspect( - networkFields.chainId, - )} (must start with "0x" and not exceed the maximum)`, + `${errorMessagePrefix}: Invalid \`chainId\` '${networkFields.chainId}' (must start with "0x" and not exceed the maximum)`, ); } @@ -2082,9 +2077,9 @@ export class NetworkController extends BaseController< for (const rpcEndpointFields of networkFields.rpcEndpoints) { if (!isValidUrl(rpcEndpointFields.url)) { throw new Error( - `${errorMessagePrefix}: An entry in \`rpcEndpoints\` has invalid URL ${inspect( - rpcEndpointFields.url, - )}`, + // False negative - this is a string. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `${errorMessagePrefix}: An entry in \`rpcEndpoints\` has invalid URL '${rpcEndpointFields.url}'`, ); } const networkClientId = @@ -2113,13 +2108,9 @@ export class NetworkController extends BaseController< ) ) { throw new Error( - `${errorMessagePrefix}: RPC endpoint '${ - // This is a string. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - rpcEndpointFields.url - }' refers to network client ${inspect( - networkClientId, - )} that does not exist`, + // This is a string. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `${errorMessagePrefix}: RPC endpoint '${rpcEndpointFields.url}' refers to network client '${networkClientId}' that does not exist`, ); } @@ -2561,7 +2552,7 @@ export class NetworkController extends BaseController< /* istanbul ignore if */ if (!possibleAutoManagedNetworkClient) { throw new Error( - `No Infura network client found with ID ${inspect(networkClientId)}`, + `No Infura network client found with ID '${networkClientId}'`, ); } @@ -2573,9 +2564,7 @@ export class NetworkController extends BaseController< ]; if (!possibleAutoManagedNetworkClient) { - throw new Error( - `No network client found with ID ${inspect(networkClientId)}`, - ); + throw new Error(`No network client found with ID '${networkClientId}'`); } autoManagedNetworkClient = possibleAutoManagedNetworkClient; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts index 9020e24ccad..f87bb8da29f 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -346,6 +346,8 @@ export default class UserStorageController extends BaseController< mapInternalAccountToUserStorageAccount(internalAccount); await this.performSetStorage( + // False negative. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions `accounts.${internalAccount.address}`, JSON.stringify(mappedAccount), ); diff --git a/packages/signature-controller/README.md b/packages/signature-controller/README.md index 76ea0cad92e..179546cd6b2 100644 --- a/packages/signature-controller/README.md +++ b/packages/signature-controller/README.md @@ -10,6 +10,10 @@ or `npm install @metamask/signature-controller` +## Compatibility + +This package relies implicitly upon the `EventEmitter` module. This module is available natively in Node.js, but when using this package for the browser, make sure to use a polyfill such as `events`. + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/signature-controller/src/SignatureController.ts b/packages/signature-controller/src/SignatureController.ts index 0b6da72927c..ba295057506 100644 --- a/packages/signature-controller/src/SignatureController.ts +++ b/packages/signature-controller/src/SignatureController.ts @@ -28,6 +28,8 @@ import { type AddLog, } from '@metamask/logging-controller'; import type { Hex, Json } from '@metamask/utils'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import { v1 as random } from 'uuid'; diff --git a/packages/transaction-controller/README.md b/packages/transaction-controller/README.md index 8c7b8af337c..16447e874ff 100644 --- a/packages/transaction-controller/README.md +++ b/packages/transaction-controller/README.md @@ -10,6 +10,10 @@ or `npm install @metamask/transaction-controller` +## Compatibility + +This package relies implicitly upon the `EventEmitter` module. This module is available natively in Node.js, but when using this package for the browser, make sure to use a polyfill such as `events`. + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index eec71928213..3bb77d9881c 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -48,6 +48,8 @@ import type { Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { MethodRegistry } from 'eth-method-registry'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import { EventEmitter } from 'events'; import { cloneDeep, mapValues, merge, pickBy, sortBy, isEqual } from 'lodash'; import { v1 as random } from 'uuid'; diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.ts b/packages/transaction-controller/src/helpers/GasFeePoller.ts index 1aaff0ea067..57cde316b76 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.ts @@ -6,6 +6,8 @@ import type { import type { NetworkClientId, Provider } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import { projectLogger } from '../logger'; diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts index ed9b0f1cd33..48e3ecaec3a 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.ts @@ -2,6 +2,8 @@ import type { AccountsController } from '@metamask/accounts-controller'; import type { BlockTracker } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import { incomingTransactionsLogger as log } from '../logger'; diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts index dc4e12d8c7b..98002d0ba2f 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.ts @@ -4,6 +4,8 @@ import type { BlockTracker, NetworkClientId, } from '@metamask/network-controller'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import { cloneDeep, merge } from 'lodash'; diff --git a/packages/user-operation-controller/README.md b/packages/user-operation-controller/README.md index 4994989e8da..2359ee1e9ae 100644 --- a/packages/user-operation-controller/README.md +++ b/packages/user-operation-controller/README.md @@ -10,6 +10,10 @@ or `npm install @metamask/user-operation-controller` +## Compatibility + +This package relies implicitly upon the `EventEmitter` module. This module is available natively in Node.js, but when using this package for the browser, make sure to use a polyfill such as `events`. + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/user-operation-controller/src/UserOperationController.ts b/packages/user-operation-controller/src/UserOperationController.ts index 492233d33c4..2c70bd80c66 100644 --- a/packages/user-operation-controller/src/UserOperationController.ts +++ b/packages/user-operation-controller/src/UserOperationController.ts @@ -25,6 +25,8 @@ import { type TransactionType, } from '@metamask/transaction-controller'; import { add0x } from '@metamask/utils'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import type { Patch } from 'immer'; import { cloneDeep } from 'lodash'; diff --git a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts index 2d5c1ca366d..54fa8ce6023 100644 --- a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts +++ b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts @@ -7,6 +7,8 @@ import type { } from '@metamask/network-controller'; import { BlockTrackerPollingControllerOnly } from '@metamask/polling-controller'; import { createModuleLogger, type Hex } from '@metamask/utils'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import { projectLogger } from '../logger'; diff --git a/scripts/create-package/utils.test.ts b/scripts/create-package/utils.test.ts index b33e268b5a5..1c5615fb95b 100644 --- a/scripts/create-package/utils.test.ts +++ b/scripts/create-package/utils.test.ts @@ -14,6 +14,7 @@ jest.mock('fs', () => ({ mkdir: jest.fn(), readFile: jest.fn(), writeFile: jest.fn(), + stat: jest.fn(), }, })); @@ -86,7 +87,9 @@ describe('create-package/utils', () => { nodeVersions: '>=18.0.0', }; - (fs.existsSync as jest.Mock).mockReturnValueOnce(false); + (fs.promises.stat as jest.Mock).mockResolvedValueOnce({ + isDirectory: () => false, + }); (fsUtils.readAllFiles as jest.Mock).mockResolvedValueOnce({ 'src/index.ts': 'export default 42;', @@ -167,7 +170,9 @@ describe('create-package/utils', () => { nodeVersions: '20.0.0', }; - (fs.existsSync as jest.Mock).mockReturnValueOnce(true); + (fs.promises.stat as jest.Mock).mockResolvedValueOnce({ + isDirectory: () => true, + }); await expect( finalizeAndWriteData(packageData, monorepoFileData), diff --git a/scripts/create-package/utils.ts b/scripts/create-package/utils.ts index 02a731e2f6a..85c24d10a8d 100644 --- a/scripts/create-package/utils.ts +++ b/scripts/create-package/utils.ts @@ -1,5 +1,5 @@ import execa from 'execa'; -import { existsSync, promises as fs } from 'fs'; +import { promises as fs } from 'fs'; import path from 'path'; import { format as prettierFormat } from 'prettier'; import type { Options as PrettierOptions } from 'prettier'; @@ -94,7 +94,7 @@ export async function finalizeAndWriteData( monorepoFileData: MonorepoFileData, ) { const packagePath = path.join(PACKAGES_PATH, packageData.directoryName); - if (existsSync(packagePath)) { + if ((await fs.stat(packagePath)).isDirectory()) { throw new Error(`The package directory already exists: ${packagePath}`); } From 28db5e95f73ab65cecf069393d4b4f7c07def57e Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 19 Nov 2024 09:20:10 -0700 Subject: [PATCH 2/3] Ignore another lint violation --- packages/network-controller/src/NetworkController.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 322f25628bf..9484b2317a9 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -1354,6 +1354,8 @@ export class NetworkController extends BaseController< async setProviderType(type: InfuraNetworkType) { if ((type as unknown) === NetworkType.rpc) { throw new Error( + // This ESLint rule mistakenly produces an error. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions `NetworkController - cannot call "setProviderType" with type "${NetworkType.rpc}". Use "setActiveNetwork"`, ); } From 8f11f0b698b12c66b73ff359b7e6cf68e88a65a3 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 26 Nov 2024 14:13:42 -0700 Subject: [PATCH 3/3] Fix lint violation --- packages/transaction-controller/src/helpers/MethodDataHelper.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/transaction-controller/src/helpers/MethodDataHelper.ts b/packages/transaction-controller/src/helpers/MethodDataHelper.ts index 864aee8c2cb..54dbdb6ab4a 100644 --- a/packages/transaction-controller/src/helpers/MethodDataHelper.ts +++ b/packages/transaction-controller/src/helpers/MethodDataHelper.ts @@ -2,6 +2,8 @@ import type { NetworkClientId, Provider } from '@metamask/network-controller'; import { createModuleLogger } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { MethodRegistry } from 'eth-method-registry'; +// This package purposefully relies on Node's EventEmitter module. +// eslint-disable-next-line import/no-nodejs-modules import EventEmitter from 'events'; import { projectLogger } from '../logger';