-
-
Notifications
You must be signed in to change notification settings - Fork 183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: update tokens controllers to use selectedAccountId instead of selectedAddress #4219
fix: update tokens controllers to use selectedAccountId instead of selectedAddress #4219
Conversation
@metamaskbot publish-preview |
78ef3b5
to
6c0595d
Compare
@metamaskbot publish-preview |
1 similar comment
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
cb17e99
to
1e319a0
Compare
* @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. | ||
* @param options.trackMetaMetricsEvent - Sets options for MetaMetrics event tracking. | ||
*/ | ||
constructor({ | ||
selectedAddress, | ||
selectedAccountId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment as here — what are your thoughts on using the messenger to get the current value of this later on in the constructor rather than having the controller take this as an argument?
onPreferencesStateChange, | ||
selectedAccountId, | ||
getInternalAccount, | ||
onSelectedAccountChange, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment as above, but in addition, instead of continuing with the callback pattern is it possible to listen to the AccountsController:selectedAccountChange
event instead?
* @param options.provider - Network provider. | ||
* @param options.state - Initial state to set on this controller. | ||
* @param options.messenger - The controller messenger. | ||
*/ | ||
constructor({ | ||
chainId: initialChainId, | ||
selectedAddress, | ||
selectedAccountId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment as above.
packages/assets-controllers/src/TokenBalancesController.test.ts
Outdated
Show resolved
Hide resolved
…ns-controllers-to-use-internal-accounts
…ns-controllers-to-use-internal-accounts
const selectedAccount = this.messagingSystem.call( | ||
'AccountsController:getAccount', | ||
this.#selectedAccountId, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const selectedAccount = this.messagingSystem.call( | |
'AccountsController:getAccount', | |
this.#selectedAccountId, | |
); | |
const selectedAddress = this.#getSelectedAddress(); |
And use selectedAddress
afterward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about this suggestion ⬆️?
const selectedAccount = this.messagingSystem.call( | ||
'AccountsController:getAccount', | ||
this.#selectedAccountId, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about this suggestion ⬆️?
…ns-controllers-to-use-internal-accounts
@metamaskbot publish-preview |
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job :) this was a big one! :)
@metamaskbot publish-preview |
…lectedAddress (#4219) ## Explanation This PR updates the `selectedAccount` to `selectedAccountId` in the token controllers ## References Fixes MetaMask/accounts-planning#381 ## Changelog ### `@metamask/assets-controllers` - **BREAKING**: `TokenBalancesController` update `PreferencesConrtollerGetStateAction` to `AccountsControllerGetSelectedAccountAction` - **BREAKING**: `TokenDetectionController` change `selectedAddress` to `selectedAccountId` - **ADDED**: `TokenDetectionController` add `getAccountAction` - **BREAKING**: `TokenRatesController` change `selectedAddress` to `selectedAccountId` - **BREAKING**: `onPreferencesStateChange` arg removed and `getInternalAccount` and `onSelectedAccountChange` added in `TokenRatesController` - **ADDED**: `getAccountAction` added in`TokensController` - **BREAKING**: Changed `selectedAddress` to `selectedAccountId` and `PreferencesControllerStateChangeEvent` to `AccountsControllerSelectedEvmAccountChangeEvent` in the `TokensController` - **ADDED**: `getAccountAction` added in`TokensController` ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Explanation
This PR updates the
selectedAccount
toselectedAccountId
in the token controllersReferences
Fixes https://github.com/MetaMask/accounts-planning/issues/381
Changelog
@metamask/assets-controllers
TokenBalancesController
messenger must allow the actionAccountsController:getSelectedAccount
and removePreferencesController:getState
.TokenDetectionController
messenger must allow the actionAccountsController:getAccount
.TokenDetectionController
messenger must allow the eventAccountsController:selectedEvmAccountChange
and removeAccountsController:selectedAccountChange
.TokenDetectionController
removesselectedAddress
constructor argument.TokenRatesController
messenger must allow the actionAccountsController:getAccount
,AccountsController:getSelectedAccount
and removePreferencesController:getState
.TokenRatesController
messenger must allow the eventAccountsController:selectedEvmAccountChange
and removePreferencesController:stateChange
.TokensController
messenger must allow the actionAccountsController:getAccount
,AccountsController:getSelectedAccount
.TokensController
messenger must allow the eventAccountsController:selectedEvmAccountChange
.TokensController
removesselectedAddress
constructor argument.Checklist