Skip to content

Commit 6cac875

Browse files
authored
fix(accounts-controller): workaround compiler error for recursive type (MetaMask#6432)
## Explanation Narrowing the `AccountControllerState` internally to avoid facing this issue: - MetaMask/utils#168 This has no impact on the controller behavior or interface, it's purely related to the type-system and compiler errors. I had errors when I was running the test locally with the compiler, but the CI ran fine. Now, it runs fine on both environment with no error/warning. ## References N/A ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent f386e24 commit 6cac875

2 files changed

Lines changed: 48 additions & 19 deletions

File tree

packages/accounts-controller/src/AccountsController.ts

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import type { WritableDraft } from 'immer/dist/internal.js';
4040
import { cloneDeep } from 'lodash';
4141

4242
import type { MultichainNetworkControllerNetworkDidChangeEvent } from './types';
43+
import type { AccountsControllerStrictState } from './typing';
4344
import type { HdSnapKeyringAccount } from './utils';
4445
import {
4546
getEvmDerivationPathForIndex,
@@ -479,14 +480,8 @@ export class AccountsController extends BaseController<
479480
};
480481

481482
this.#update((state) => {
482-
// FIXME: Using the state as-is cause the following error: "Type instantiation is excessively
483-
// deep and possibly infinite.ts(2589)" (https://github.com/MetaMask/utils/issues/168)
484-
// Using a type-cast workaround this error and is slightly better than using a @ts-expect-error
485-
// which sometimes fail when compiling locally.
486-
(state as AccountsControllerState).internalAccounts.accounts[account.id] =
487-
internalAccount;
488-
(state as AccountsControllerState).internalAccounts.selectedAccount =
489-
account.id;
483+
state.internalAccounts.accounts[account.id] = internalAccount;
484+
state.internalAccounts.selectedAccount = account.id;
490485
});
491486

492487
this.messagingSystem.publish(
@@ -529,12 +524,7 @@ export class AccountsController extends BaseController<
529524
};
530525

531526
this.#update((state) => {
532-
// FIXME: Using the state as-is cause the following error: "Type instantiation is excessively
533-
// deep and possibly infinite.ts(2589)" (https://github.com/MetaMask/utils/issues/168)
534-
// Using a type-cast workaround this error and is slightly better than using a @ts-expect-error
535-
// which sometimes fail when compiling locally.
536-
(state as AccountsControllerState).internalAccounts.accounts[accountId] =
537-
internalAccount;
527+
state.internalAccounts.accounts[accountId] = internalAccount;
538528
});
539529

540530
if (metadata.name) {
@@ -615,9 +605,11 @@ export class AccountsController extends BaseController<
615605
*/
616606
loadBackup(backup: AccountsControllerState): void {
617607
if (backup.internalAccounts) {
618-
this.update((currentState) => {
619-
currentState.internalAccounts = backup.internalAccounts;
620-
});
608+
this.update(
609+
(currentState: WritableDraft<AccountsControllerStrictState>) => {
610+
currentState.internalAccounts = backup.internalAccounts;
611+
},
612+
);
621613
}
622614
}
623615

@@ -906,13 +898,15 @@ export class AccountsController extends BaseController<
906898
*
907899
* @param callback - Callback for updating state, passed a draft state object.
908900
*/
909-
#update(callback: (state: WritableDraft<AccountsControllerState>) => void) {
901+
#update(
902+
callback: (state: WritableDraft<AccountsControllerStrictState>) => void,
903+
) {
910904
// The currently selected account might get deleted during the update, so keep track
911905
// of it before doing any change.
912906
const previouslySelectedAccount =
913907
this.state.internalAccounts.selectedAccount;
914908

915-
this.update((state) => {
909+
this.update((state: WritableDraft<AccountsControllerStrictState>) => {
916910
callback(state);
917911

918912
// If the account no longer exists (or none is selected), we need to re-select another one.
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import type { KeyringAccountEntropyOptions } from '@metamask/keyring-api';
2+
import type { InternalAccount } from '@metamask/keyring-internal-api';
3+
4+
import type { AccountsControllerState } from './AccountsController';
5+
6+
/**
7+
* Type constraint to ensure a type is compatible with {@link AccountsControllerState}.
8+
* If the constraint is not matching, this type will resolve to `never` and thus, fails
9+
* to compile.
10+
*/
11+
type IsAccountControllerState<Type extends AccountsControllerState> = Type;
12+
13+
/**
14+
* A type compatible with {@link InternalAccount} which removes any use of recursive-type.
15+
*/
16+
export type StrictInternalAccount = Omit<InternalAccount, 'options'> & {
17+
// Use stricter options, which are relying on `Json` (which sometimes
18+
// cause compiler errors because of instanciation "too deep".
19+
// In anyway, we should rarely have to use those "untyped" options.
20+
options: {
21+
entropy?: KeyringAccountEntropyOptions;
22+
exportable?: boolean;
23+
};
24+
};
25+
26+
/**
27+
* A type compatible with {@link AccountControllerState} which can be used to
28+
* avoid recursive-type issue with `internalAccounts`.
29+
*/
30+
export type AccountsControllerStrictState = IsAccountControllerState<{
31+
internalAccounts: {
32+
accounts: Record<InternalAccount['id'], StrictInternalAccount>;
33+
selectedAccount: InternalAccount['id'];
34+
};
35+
}>;

0 commit comments

Comments
 (0)