Skip to content

refactor: simplify AssetsService types - #84

Merged
ulissesferreira merged 3 commits into
WPN-1784-shared-assets-servicefrom
me/refactor/assets-service
Jul 31, 2026
Merged

refactor: simplify AssetsService types#84
ulissesferreira merged 3 commits into
WPN-1784-shared-assets-servicefrom
me/refactor/assets-service

Conversation

@mikesposito

Copy link
Copy Markdown
Member

Explanation

Proposal to simplify AssetsService types and returned values, to align it to what's currently returned by AssetsController's actions.

References

Related to #82

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
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

@mikesposito
mikesposito requested a review from a team as a code owner July 31, 2026 17:49
Comment thread packages/snap-networks-utils/src/services/assets/AssetsService.test.ts Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ulissesferreira given this file is now just a couple of type declarations, we could consider merging it with AssetsService.ts - wdyt?

@ulissesferreira ulissesferreira Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that seems fair so we can maintain less files. Sounds good.

Comment on lines -10 to -14
export type {
AssetsControllerGetAccountAssetByIDAction,
AssetsControllerGetAccountAssetsByIDsAction,
AssetsControllerGetAccountAssetsByScopeAction,
};

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better not to re-export these types from another package, because any breaking change on these types from @metamask/assets-controller would turn into a breaking change for @metamask/snap-networks-utils as well.

Consumers can simply import them from @metamask/assets-controller instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100%

@ulissesferreira
ulissesferreira merged commit 0b342b9 into WPN-1784-shared-assets-service Jul 31, 2026
31 of 40 checks passed
@ulissesferreira
ulissesferreira deleted the me/refactor/assets-service branch July 31, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants