You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Engine.ts file is a large ~2500 LOC file that holds the logic for controller initialization and is used in many places in the app. The goal here is to establish a pattern for splitting out the file so that it is:
Easier to manage
Better testability
Reduce non-relevant codeowner reviews (saves time on PRs)
More scalable solution
Scenario
No response
Design
No response
Technical Details
Since controllers tend to be dependent on one another, the imports from abstracted files can be functions that take dependent controllers as args to ensure initialization order is respected. For example, if TokenListController is dependent on NetworkController, the abstracted TokenListController file could look like initializeTokenListController(networkController: NetworkController)
Threat Modeling Framework
No response
Acceptance Criteria
App behavior should remain the same
Abstract at least one area as POV that can be applied to other pieces later on
Stakeholder review needed before the work gets merged
Engineering (needed in most cases)
Design
Product
QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
This PR establishes a pattern for splitting out controllers from the
main `Engine.ts` file, which provides the following benefits
- Reduce friction regarding code ownership especially during reviews
(this is the main purpose of this effort)
- Easier to manage, test, and handling controller initialization
failures
The POC here applies to the `AccountsController` initialization logic,
which has been mostly abstracted out into `AccountsController`
directory. This pattern should be applicable to other controllers in the
Engine file as well.
This pattern also enables us to redirect users to vault recovery in the
event of controller initialization failures and should reduce the
chances of users being "bricked". These changes may be introduced in
future interations
## **Related issues**
Fixes: #12184
## **Manual testing steps**
The changes should not affect app behavior
- Run the app as usual
- Ensure that everything behaves as usual (Auth, dapp connections,
navigation, etc)
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
App still runs normally
https://github.com/user-attachments/assets/dc7fc017-1a26-4681-b84a-e04c63952281
## **Pre-merge author checklist**
- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **Pre-merge reviewer checklist**
- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
What is this about?
The
Engine.ts
file is a large ~2500 LOC file that holds the logic for controller initialization and is used in many places in the app. The goal here is to establish a pattern for splitting out the file so that it is:Scenario
No response
Design
No response
Technical Details
TokenListController
is dependent onNetworkController
, the abstracted TokenListController file could look likeinitializeTokenListController(networkController: NetworkController)
Threat Modeling Framework
No response
Acceptance Criteria
Stakeholder review needed before the work gets merged
References
The text was updated successfully, but these errors were encountered: