fix(accounts-controller): fire :accountAdded before :selectedAccountChange on KeyringController:stateChange

Author: ccharly

packages/accounts-controller/src/AccountsController.ts

@@ -833,63 +833,71 @@ export class AccountsController extends BaseController<
       added: [] as InternalAccount[],
     };
 
-    this.#update((state) => {
-      const { internalAccounts } = state;
+    this.#update(
+      (state) => {
+        const { internalAccounts } = state;
 
-      for (const patch of [patches.snap, patches.normal]) {
-        for (const account of patch.removed) {
-          delete internalAccounts.accounts[account.id];
+        for (const patch of [patches.snap, patches.normal]) {
+          for (const account of patch.removed) {
+            delete internalAccounts.accounts[account.id];
 
-          diff.removed.push(account.id);
-        }
-
-        for (const added of patch.added) {
-          const account = this.#getInternalAccountFromAddressAndType(
-            added.address,
-            added.keyring,
-          );
+            diff.removed.push(account.id);
+          }
 
-          if (account) {
-            // Re-compute the list of accounts everytime, so we can make sure new names
-            // are also considered.
-            const accounts = Object.values(
-              internalAccounts.accounts,
-            ) as InternalAccount[];
-
-            // Get next account name available for this given keyring.
-            const name = this.getNextAvailableAccountName(
-              account.metadata.keyring.type,
-              accounts,
+          for (const added of patch.added) {
+            const account = this.#getInternalAccountFromAddressAndType(
+              added.address,
+              added.keyring,
             );
 
-            // If it's the first account, we need to select it.
-            const lastSelected =
-              accounts.length === 0 ? this.#getLastSelectedIndex() : 0;
-
-            internalAccounts.accounts[account.id] = {
-              ...account,
-              metadata: {
-                ...account.metadata,
-                name,
-                importTime: Date.now(),
-                lastSelected,
-              },
-            };
-
-            diff.added.push(internalAccounts.accounts[account.id]);
+            if (account) {
+              // Re-compute the list of accounts everytime, so we can make sure new names
+              // are also considered.
+              const accounts = Object.values(
+                internalAccounts.accounts,
+              ) as InternalAccount[];
+
+              // Get next account name available for this given keyring.
+              const name = this.getNextAvailableAccountName(
+                account.metadata.keyring.type,
+                accounts,
+              );
+
+              // If it's the first account, we need to select it.
+              const lastSelected =
+                accounts.length === 0 ? this.#getLastSelectedIndex() : 0;
+
+              internalAccounts.accounts[account.id] = {
+                ...account,
+                metadata: {
+                  ...account.metadata,
+                  name,
+                  importTime: Date.now(),
+                  lastSelected,
+                },
+              };
+
+              diff.added.push(internalAccounts.accounts[account.id]);
+            }
           }
         }
-      }
-    });
-
-    // Now publish events
-    for (const id of diff.removed) {
-      this.messagingSystem.publish('AccountsController:accountRemoved', id);
-    }
+      },
+      // Will get executed after the update, but before re-selecting an account in case
+      // the current one is not valid anymore.
+      () => {
+        // Now publish events
+        for (const id of diff.removed) {
+          this.messagingSystem.publish('AccountsController:accountRemoved', id);
+        }
 
-    for (const account of diff.added) {
-      this.messagingSystem.publish('AccountsController:accountAdded', account);
-    }
+        for (const account of diff.added) {
+          this.messagingSystem.publish(
+            'AccountsController:accountAdded',
+            account,
+          );
+        }
+      },
+    );
 
     // NOTE: Since we also track "updated" accounts with our patches, we could fire a new event
     // like `accountUpdated` (we would still need to check if anything really changed on the account).
@@ -899,9 +907,12 @@ export class AccountsController extends BaseController<
    * Update the state and fixup the currently selected account.
    *
    * @param callback - Callback for updating state, passed a draft state object.
+   * @param beforeAutoSelectAccount - Callback to be executed before auto-selecting an account
+   * if the current one is no longer available.
    */
   #update(
     callback: (state: WritableDraft<AccountsControllerStrictState>) => void,
+    beforeAutoSelectAccount?: () => void,
   ) {
     // The currently selected account might get deleted during the update, so keep track
     // of it before doing any change.
@@ -932,6 +943,9 @@ export class AccountsController extends BaseController<
       }
     });
 
+    // We might want to do some pre-work before selecting a new account.
+    beforeAutoSelectAccount?.();
+
     // Now, we compare the newly selected account, and we send event if different.
     const { selectedAccount } = this.state.internalAccounts;
     if (selectedAccount && selectedAccount !== previouslySelectedAccount) {