-
-
Notifications
You must be signed in to change notification settings - Fork 266
chore: create new assets controller package #7587
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
base: main
Are you sure you want to change the base?
Conversation
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.
Two questions:
-
What do you think about copying and pasting one of the controllers in the
sample-controllerspackage? This way we can keep the general look and feel for a controller consistent across all packages. -
I see that we are creating a new package called
assets-controller, when we already have a package calledassets-controllers(with an "s"). I'm a bit worried this will be a constant source of confusion, and so perhaps we shouldn't make a new package yet? Or — are there any near-term plans to split apartassets-controllers?
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.
@mcmire yes, there are near-term plans to split apart assets-controllers. The goal is to deprecate the existing assets-controllers (plural) package and break it into several focused, single-responsibility packages:
@metamask/assets-controller- consolidated asset tracking (balances, detection)- Other controllers from the package will be split into their own dedicated packages as well
The naming is intentional:
assets-controllers= legacy "kitchen sink" package (to be deprecated)assets-controller= new single unified controller
This follows the pattern we've used elsewhere in the monorepo where packages contain a single controller (e.g., @metamask/network-controller, @metamask/keyring-controller).
Once the migration is complete and assets-controllers is deprecated and deleted, the confusion will be resolved. In the meantime, we can add a note to the assets-controllers README indicating it's being deprecated and pointing to the new packages.
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.
@salimtb Okay, that sounds like a great plan. Thanks for clarifying!
Explanation
This PR introduces a new
@metamask/assets-controllerpackage as a placeholder for future consolidation of asset tracking functionality.Current state:
Asset tracking functionality (account balances, token balances, asset detection) is currently spread across multiple controllers in the
@metamask/assets-controllerspackage.Solution:
This PR creates an empty
@metamask/assets-controllerpackage as a foundation for future work to consolidate asset tracking into a single, unified controller. The package currently exports nothing and serves as a placeholder with TODOs indicating the intended scope:No functional changes are included in this PR.
References
Checklist
Note
Introduces a new placeholder package for future asset tracking consolidation.
@metamask/assets-controllerwith a minimalAssetsController(empty state), exports, unit tests, README, LICENSE, CHANGELOG, Jest/TypeDoc/TS configs, and build scriptsREADME(list + graph),CODEOWNERS,teams.json, roottsconfig.jsonandtsconfig.build.jsonreferences, andyarn.lockentryWritten by Cursor Bugbot for commit a0bae1b. This will update automatically on new commits. Configure here.