Skip to content

Commit 74d38a1

Browse files
authored
fix: silent @typescript-eslint/no-redundant-type-constituents + fix local linting issues (#437)
Partially reverting this PR: - #407 It turns out, the CI was happy with those changes, but it was giving us some other lint issues locally. Still unsure what causes that (since it works fine on the CI). That seems to be a false-positive, and since this directive is not a "strong requirement" for increased type-safety, it's ok to disable it IMO. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Streamlines linting and removes prior workarounds. > > - Introduces `tsCommonRules` in `.eslintrc.js` to disable `@typescript-eslint/no-redundant-type-constituents` and applies it across multiple package overrides > - Removes extra `@metamask/*` path entries from `jest.config.packages.js` and `tsconfig.packages.json` used as a workaround > - Adds a scoped ESLint disable for duplicate type constituents in `packages/keyring-internal-api/src/types.ts` > - Refactors small test helpers in Ledger/Trezor keyring tests to use safer array access patterns > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 056615a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 3398691 commit 74d38a1

6 files changed

Lines changed: 39 additions & 15 deletions

File tree

.eslintrc.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ const parserOptions = {
55
sourceType: 'module',
66
};
77

8+
const tsCommonRules = {
9+
// This rule triggers false positives and doesn't add real type-safety value.
10+
// See: https://typescript-eslint.io/rules/no-redundant-type-constituents/#when-not-to-use-it
11+
'@typescript-eslint/no-redundant-type-constituents': 'off',
12+
};
13+
814
module.exports = {
915
root: true,
1016
extends: ['@metamask/eslint-config', '@metamask/eslint-config-nodejs'],
@@ -56,6 +62,8 @@ module.exports = {
5662
extends: ['@metamask/eslint-config-typescript'],
5763
parserOptions,
5864
rules: {
65+
...tsCommonRules,
66+
5967
// Enable rules that are disabled in `@metamask/eslint-config-typescript`
6068
'@typescript-eslint/no-explicit-any': 'error',
6169
},
@@ -72,6 +80,8 @@ module.exports = {
7280
extends: ['@metamask/eslint-config-typescript'],
7381
parserOptions,
7482
rules: {
83+
...tsCommonRules,
84+
7585
// TODO: re-lint everything once the migration is done
7686
'@typescript-eslint/no-explicit-any': 'off',
7787
},
@@ -82,6 +92,8 @@ module.exports = {
8292
extends: ['@metamask/eslint-config-typescript'],
8393
parserOptions,
8494
rules: {
95+
...tsCommonRules,
96+
8597
// TODO: re-lint everything once the migration is done
8698
'import/order': 'off',
8799
'jsdoc/newline-after-description': 'off',
@@ -93,6 +105,8 @@ module.exports = {
93105
extends: ['@metamask/eslint-config-typescript'],
94106
parserOptions,
95107
rules: {
108+
...tsCommonRules,
109+
96110
// FIXME: for some reason, it seems eslint is not able to infere those (this
97111
// works on the original repository, so there might be some side-effects now that
98112
// we are building in a monorepo)
@@ -132,6 +146,8 @@ module.exports = {
132146
extends: ['@metamask/eslint-config-typescript'],
133147
parserOptions,
134148
rules: {
149+
...tsCommonRules,
150+
135151
// TODO: re-lint everything once the migration is done
136152
'@typescript-eslint/consistent-type-imports': 'off',
137153
'@typescript-eslint/naming-convention': 'off',
@@ -156,6 +172,8 @@ module.exports = {
156172
extends: ['@metamask/eslint-config-typescript'],
157173
parserOptions,
158174
rules: {
175+
...tsCommonRules,
176+
159177
// TODO: re-lint everything once the migration is done
160178
'@typescript-eslint/consistent-type-definitions': 'off',
161179
'@typescript-eslint/consistent-type-imports': 'off',
@@ -183,6 +201,8 @@ module.exports = {
183201
extends: ['@metamask/eslint-config-typescript'],
184202
parserOptions,
185203
rules: {
204+
...tsCommonRules,
205+
186206
// TODO: re-lint everything once the migration is done
187207
'@typescript-eslint/no-explicit-any': 'off',
188208
// FIXME: for some reason, it seems eslint is not able to infere those (this
@@ -209,6 +229,8 @@ module.exports = {
209229
extends: ['@metamask/eslint-config-typescript'],
210230
parserOptions,
211231
rules: {
232+
...tsCommonRules,
233+
212234
// TODO: re-lint everything once the migration is done
213235
'@typescript-eslint/no-explicit-any': 'off',
214236
// FIXME: for some reason, it seems eslint is not able to infere those (this

jest.config.packages.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ module.exports = {
8888
moduleNameMapper: {
8989
'^@metamask/(.+)$': [
9090
'<rootDir>/../$1/src',
91-
// While still unclear why, adding the line below seems to fix the @typescript-eslint/no-redundant-type-constituents errors throughout the monorepo
92-
'<rootDir>/packages/$1/src',
9391
// Some @metamask/* packages we are referencing aren't in this monorepo,
9492
// so in that case use their published versions
9593
'<rootDir>/../../node_modules/@metamask/$1',

packages/keyring-eth-ledger-bridge/src/ledger-keyring-v2.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,11 @@ const EXPECTED_METHODS = [
5656
* @returns The first account.
5757
*/
5858
function getFirstAccount(accounts: LedgerAccount[]): LedgerAccount {
59-
if (accounts.length === 0) {
59+
const [account] = accounts;
60+
if (!account) {
6061
throw new Error('Expected at least one account');
6162
}
62-
return accounts[0] as LedgerAccount;
63+
return account;
6364
}
6465

6566
/**
@@ -70,10 +71,11 @@ function getFirstAccount(accounts: LedgerAccount[]): LedgerAccount {
7071
* @returns The account at the index.
7172
*/
7273
function getAccountAt(accounts: LedgerAccount[], index: number): LedgerAccount {
73-
if (accounts.length <= index) {
74+
const account = accounts[index];
75+
if (!account) {
7476
throw new Error(`Expected account at index ${index}`);
7577
}
76-
return accounts[index] as LedgerAccount;
78+
return account;
7779
}
7880

7981
/**

packages/keyring-eth-trezor/src/trezor-keyring-v2.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,11 @@ const EXPECTED_METHODS = [
6161
* @returns The first account.
6262
*/
6363
function getFirstAccount(accounts: TrezorAccount[]): TrezorAccount {
64-
if (accounts.length === 0) {
64+
const [account] = accounts;
65+
if (!account) {
6566
throw new Error('Expected at least one account');
6667
}
67-
return accounts[0] as TrezorAccount;
68+
return account;
6869
}
6970

7071
/**
@@ -75,10 +76,11 @@ function getFirstAccount(accounts: TrezorAccount[]): TrezorAccount {
7576
* @returns The account at the index.
7677
*/
7778
function getAccountAt(accounts: TrezorAccount[], index: number): TrezorAccount {
78-
if (accounts.length <= index) {
79+
const account = accounts[index];
80+
if (!account) {
7981
throw new Error(`Expected account at index ${index}`);
8082
}
81-
return accounts[index] as TrezorAccount;
83+
return account;
8284
}
8385

8486
/**

packages/keyring-internal-api/src/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
// FIXME: eslint is complaning about our account union even if those accounts
2+
// types should all be different, so we disable this for now:
3+
/* eslint-disable @typescript-eslint/no-duplicate-type-constituents */
4+
15
import {
26
BtcAccountType,
37
EthAccountType,

tsconfig.packages.json

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,7 @@
2828
* `jest.config.packages.js`.
2929
*/
3030
"paths": {
31-
"@metamask/*": [
32-
"../*/src",
33-
// While still unclear why, adding "./packages/*/src" seems to fix the @typescript-eslint/no-redundant-type-constituents errors throughout the monorepo
34-
"./packages/*/src"
35-
]
31+
"@metamask/*": ["../*/src"]
3632
}
3733
}
3834
}

0 commit comments

Comments
 (0)