Skip to content

Commit

Permalink
[composable-controller] perf: Optimize expensive reduce operations, f…
Browse files Browse the repository at this point in the history
…ix metadata instantiation (#4968)

## Motivation

Expensive operations during `ComposableController` instantiation
contribute to downstream performance issues in our client apps'
initialization times. Optimizing these operations may be especially
impactful since `ComposableController` takes large inputs consisting of
all of the controllers used by a given client.

## Explanation 

- Remove wasteful `reduce` operations that create temporary objects on
each iteration and replace with implementations that re-use their output
objects.
- Refactor `ComposableController` to accept an object instead of an
array. Now that the mobile Engine creates a controllers object, this
saves us an extra operation for converting the object into an array.
- For downstream application, see
MetaMask/metamask-mobile@5ceb81c
- An auxiliary benefit of this is slightly improved type safety. Since
we were previously using a type array instead of a type tuple, we were
vulnerable to duplicate entries in the `controllers` input. This is now
resolved by using a type object.
- Fixes issues with 'state', 'metadata' field instantiation (see
changelogs).

## References

- For more information on the optimizing reduce, see
MetaMask/metamask-mobile#12374
- Blocks MetaMask/metamask-mobile#10441
- E2E passing with preview build from this PR:
MetaMask/metamask-mobile#10441 (comment)
  - Blocks MetaMask/metamask-mobile#12509

## Changelog

## `@metamask/composable-controller`

### Changed

- **BREAKING:** `ComposableController` constructor option `controllers`
and generic type argument `ChildControllers` are re-defined from an
array of controller instances to an object that maps controller names to
controller instances.
- **BREAKING:** `ComposableController` class field objects `state` and
`metadata` exclude child controllers that do not extend from
`BaseController` or `BaseControllerV1`. Any non-controller entries that
are passed into the constructor will be removed automatically.

### Fixed

- **BREAKING:** `ComposableController` class field object `metadata` now
assigns the `StateMetadataProperty`-type object `{ persist: true,
anonymous: true }` to each child controller name.
- Previously, V2 child controllers were erroneously assigned their own
metadata object. This issue was introduced in
`@metamask/[email protected]`.

## 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
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  • Loading branch information
MajorLift authored Dec 5, 2024
1 parent a6937e3 commit 3a7d680
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 86 deletions.
10 changes: 10 additions & 0 deletions packages/composable-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- **BREAKING:** `ComposableController` constructor option `controllers` and generic type argument `ChildControllers` are re-defined from an array of controller instances to an object that maps controller names to controller instances ([#4968](https://github.com/MetaMask/core/pull/4968))
- **BREAKING:** `ComposableController` class field objects `state` and `metadata` exclude child controllers that do not extend from `BaseController` or `BaseControllerV1`. Any non-controller entries that are passed into the constructor will be removed automatically ([#4968](https://github.com/MetaMask/core/pull/4968))

### Fixed

- **BREAKING:** `ComposableController` class field object `metadata` now assigns the `StateMetadataProperty`-type object `{ persist: true, anonymous: true }` to each child controller name ([#4968](https://github.com/MetaMask/core/pull/4968))
- Previously, V2 child controllers were erroneously assigned their own metadata object. This issue was introduced in `@metamask/[email protected]`.

## [9.0.1]

### Fixed
Expand Down
Loading

0 comments on commit 3a7d680

Please sign in to comment.