Skip to content

Commit 90d8093

Browse files
authored
feat(account-tree-controller): always fire :selectedAccountGroupChange on init (#8427)
## Explanation We used to not persist the `selectedAccountGroup` property before this change: - #8245 As a side-effect of this new persistence, we're not re-sending `:selectedAccountGroupChange` on `init` and a lot of controllers were relying on this behavior. We re-introduce this behavior so every controllers can decide how to handle that. ## References N/A ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes event-emission semantics so `selectedAccountGroupChange` now fires on every `init`/`reinit`, which may trigger extra startup work or duplicate handling in subscribers that assumed it only emitted on changes. > > **Overview** > Ensures `AccountTreeController` *always* publishes `AccountTreeController:selectedAccountGroupChange` at the end of `init` (and therefore `reinit`), even when the selected group value is unchanged. > > Adds a regression test covering the reinit-with-same-accounts case and updates the package changelog to document the restored behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0689a7b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ec1b5fc commit 90d8093

File tree

3 files changed

+53
-12
lines changed

3 files changed

+53
-12
lines changed

packages/account-tree-controller/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Changed
1111

12+
- Always fire `:selectedAccountGroupChange` during `init` call ([#8427](https://github.com/MetaMask/core/pull/8427))
13+
- Many controllers were actually relying on this behavior when this used to be dynamic, so we re-introduce this behavior.
1214
- Now persist `accounTree` ([#8437](https://github.com/MetaMask/core/pull/8437))
1315
- The tree is not persisted and can be used before `init` is called.
1416
- This allow consumers (like the assets controllers) to rely on the selected group's accounts right away.

packages/account-tree-controller/src/AccountTreeController.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4586,6 +4586,48 @@ describe('AccountTreeController', () => {
45864586
);
45874587
});
45884588

4589+
it('always emits selectedAccountGroupChange on init even if the selected group did not change', () => {
4590+
const { controller, messenger, mocks } = setup({
4591+
accounts: [MOCK_HD_ACCOUNT_1],
4592+
keyrings: [MOCK_HD_KEYRING_1],
4593+
});
4594+
4595+
mocks.AccountsController.getSelectedMultichainAccount.mockImplementation(
4596+
() => MOCK_HD_ACCOUNT_1,
4597+
);
4598+
4599+
controller.init();
4600+
4601+
const defaultAccountGroupId = toMultichainAccountGroupId(
4602+
toMultichainAccountWalletId(MOCK_HD_ACCOUNT_1.options.entropy.id),
4603+
MOCK_HD_ACCOUNT_1.options.entropy.groupIndex,
4604+
);
4605+
4606+
expect(controller.state.selectedAccountGroup).toStrictEqual(
4607+
defaultAccountGroupId,
4608+
);
4609+
4610+
const selectedAccountGroupChangeListener = jest.fn();
4611+
messenger.subscribe(
4612+
'AccountTreeController:selectedAccountGroupChange',
4613+
selectedAccountGroupChangeListener,
4614+
);
4615+
4616+
// Re-init with the same accounts — the selected group still exists and has not changed.
4617+
controller.reinit();
4618+
4619+
expect(controller.state.selectedAccountGroup).toStrictEqual(
4620+
defaultAccountGroupId,
4621+
);
4622+
// The event must fire even though the group did not change, so that
4623+
// subscribers that missed the first init can still react accordingly.
4624+
expect(selectedAccountGroupChangeListener).toHaveBeenCalledWith(
4625+
defaultAccountGroupId,
4626+
defaultAccountGroupId,
4627+
);
4628+
expect(selectedAccountGroupChangeListener).toHaveBeenCalledTimes(1);
4629+
});
4630+
45894631
it('emits selectedAccountGroupChange when setSelectedAccountGroup is called', () => {
45904632
// Use different keyring types to ensure different groups
45914633
const { controller, messenger } = setup({

packages/account-tree-controller/src/AccountTreeController.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -418,18 +418,15 @@ export class AccountTreeController extends BaseController<
418418
}
419419
});
420420

421-
// We still compare the previous and new value, the previous one could have been
422-
// an empty string and `#getDefaultSelectedAccountGroup` could also return an
423-
// empty string too, thus, we would re-use the same value here again. In that
424-
// case, no need to fire any event.
425-
if (previousSelectedAccountGroup !== this.state.selectedAccountGroup) {
426-
log(`Selected (initial) group is: [${this.state.selectedAccountGroup}]`);
427-
this.messenger.publish(
428-
`${controllerName}:selectedAccountGroupChange`,
429-
this.state.selectedAccountGroup,
430-
previousSelectedAccountGroup,
431-
);
432-
}
421+
// We always fire the current selected group after init, even if it did not change, to ensure that
422+
// all subscribers are aware of it and can react accordingly (e.g. by fetching accounts from
423+
// the selected group on startup).
424+
log(`Selected (initial) group is: [${this.state.selectedAccountGroup}]`);
425+
this.messenger.publish(
426+
`${controllerName}:selectedAccountGroupChange`,
427+
this.state.selectedAccountGroup,
428+
previousSelectedAccountGroup,
429+
);
433430

434431
log('Initialized!');
435432
this.#initialized = true;

0 commit comments

Comments
 (0)