feat: Convert AppStateController to typescript #27572

kanthesha · 2024-10-02T15:43:05Z

Description

As a prerequisite for migrating AppStateController to BaseController v2, and to support the TypeScript migration effort for the extension, we want to convert AppStateController to TypeScript along with its tests.

Related issues

Fixes: #25922

Manual testing steps

N/A

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
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.

github-actions · 2024-10-02T15:43:21Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2024-10-02T16:42:47Z

Builds ready [23a89c3]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1862 ± 100 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	397	2390	1790	378	181
		domContentLoaded	1569	2335	1834	200	96
		load	1604	2397	1862	208	100
		domInteractive	23	164	51	33	16

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.41 KiB (0.04%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

kanthesha · 2024-10-02T17:18:05Z

This PR is blocked by PreferencesController upgrade to BaseControllerV2.

app/scripts/controllers/app-state-controller.ts

mikesposito

We should be able to remove app/script/app-state.d.ts now

app/scripts/controllers/app-state-controller.ts

mikesposito · 2024-10-03T10:39:39Z

This PR is blocked by PreferencesController upgrade to BaseControllerV2.

Curious: why is this dependent on PreferencesController upgrade?

kanthesha · 2024-10-04T11:31:19Z

This PR is blocked by PreferencesController upgrade to BaseControllerV2.

Curious: why is this dependent on PreferencesController upgrade?

It's not a complete blocker. But good to wait. AppStateController has a preferencesStore constructor param. In the mentioned PR, this has been updated with the preferencesState. So we don't have to worry about something which will be irrelevant once another PR merges.

metamaskbot · 2024-10-04T16:53:59Z

Builds ready [bfe0d3f]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2035 ± 114 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	328	2684	1696	695	334
		domContentLoaded	1776	2678	2006	235	113
		load	1784	2684	2035	238	114
		domInteractive	14	246	70	60	29

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.76 KiB (0.04%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2024-10-14T13:10:16Z

Builds ready [744300f]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1893 ± 129 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	324	2787	1823	435	209
		domContentLoaded	1629	2693	1868	257	123
		load	1653	2796	1893	268	129
		domInteractive	15	88	37	20	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.75 KiB (0.04%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

…MetaMask/metamask-extension into feat/app-state-controller-to-typescript

sonarqubecloud · 2024-10-16T15:53:57Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-10-16T16:33:10Z

Builds ready [dff8439]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1905 ± 97 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	282	2503	1828	402	193
		domContentLoaded	1588	2460	1862	178	85
		load	1595	2515	1905	202	97
		domInteractive	19	97	49	21	10
		backgroundConnect	8	213	49	44	21
		firstReactRender	47	166	81	33	16
		getState	5	113	19	27	13
		initialActions	0	1	0	0	0
		loadScripts	1063	2027	1380	183	88
		setupStore	13	108	40	27	13
		uiStartup	1775	2995	2129	264	127

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.91 KiB (0.05%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

mikesposito · 2024-10-17T10:18:50Z

app/scripts/controllers/mmi-controller.ts

@@ -70,8 +69,6 @@ export default class MMIController extends EventEmitter {
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  public keyringController: any;

-  public preferencesController: PreferencesController;


Is this change related to AppStateController?
I think that this PR joins the AppStateController typescript conversion, an initial migration to BaseControllerV2, and also some post-merge work derived from PreferencesController

If that's correct, perhaps splitting these changes in multiple specific PR would make it easier to go through all the changes

The changes in mmi-controller were a small cleanup to remove the unused PreferencesController, which I can remove from this PR. And as you mentioned, there is also some preliminary work to prepare for the V2 upgrade. This prep work should help when we take-up the upgrade ticket.

That said, if it complicates the review, I’m happy to remove these changes from this PR and handle them separately. Let me know what works best!

metamaskbot · 2024-10-17T12:00:52Z

Builds ready [a958732]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1824 ± 73 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	260	2242	1746	376	180
		domContentLoaded	1584	2232	1796	155	75
		load	1640	2244	1824	152	73
		domInteractive	16	106	41	23	11
		backgroundConnect	8	84	29	22	11
		firstReactRender	49	196	102	43	21
		getState	4	84	19	24	11
		initialActions	0	48	2	10	5
		loadScripts	1185	1840	1349	152	73
		setupStore	12	73	32	24	11
		uiStartup	1788	2451	2072	199	96

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.91 KiB (0.05%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

DDDDDanica · 2024-10-18T11:53:20Z

development/ts-migration-dashboard/files-to-convert.json

@@ -4,7 +4,6 @@
  "app/scripts/constants/contracts.js",
  "app/scripts/constants/on-ramp.js",
  "app/scripts/contentscript.js",
-  "app/scripts/controllers/app-state.js",


metamaskbot · 2024-10-18T12:00:05Z

Builds ready [2e86cfe]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2091 ± 126 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	370	2967	2017	459	220
		domContentLoaded	1710	2955	2063	266	128
		load	1747	2968	2091	263	126
		domInteractive	15	102	62	28	13
		backgroundConnect	7	103	34	28	13
		firstReactRender	49	294	116	52	25
		getState	5	66	25	23	11
		initialActions	0	0	0	0	0
		loadScripts	1277	2059	1547	184	89
		setupStore	11	66	21	15	7
		uiStartup	1956	3182	2349	291	140

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.91 KiB (0.04%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2024-10-18T12:58:34Z

Builds ready [ebe4f3d]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1752 ± 85 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1571	2336	1751	177	85
		domContentLoaded	1515	2325	1720	182	87
		load	1571	2336	1752	177	85
		domInteractive	16	108	36	21	10
		backgroundConnect	7	74	31	23	11
		firstReactRender	46	288	99	57	28
		getState	4	53	14	16	8
		initialActions	0	1	0	0	0
		loadScripts	1078	1813	1262	158	76
		setupStore	10	96	35	29	14
		uiStartup	1715	2549	1974	245	118

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 1.91 KiB (0.04%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

convert AppStateController to typescript

6635a06

kanthesha added the team-wallet-framework label Oct 2, 2024

kanthesha requested a review from a team October 2, 2024 15:43

kanthesha self-assigned this Oct 2, 2024

update controller

23a89c3

kanthesha added the DO-NOT-MERGE Pull requests that should not be merged label Oct 2, 2024

kanthesha marked this pull request as ready for review October 2, 2024 17:14

kanthesha requested review from kumavis and a team as code owners October 2, 2024 17:14

mikesposito reviewed Oct 3, 2024

View reviewed changes

app/scripts/controllers/app-state-controller.ts Outdated Show resolved Hide resolved

app/scripts/controllers/app-state-controller.ts Show resolved Hide resolved

cryptodev-2s reviewed Oct 3, 2024

View reviewed changes

app/scripts/controllers/app-state-controller.ts Outdated Show resolved Hide resolved

cryptodev-2s reviewed Oct 3, 2024

View reviewed changes

app/scripts/controllers/app-state-controller.ts Outdated Show resolved Hide resolved

cryptodev-2s reviewed Oct 3, 2024

View reviewed changes

app/scripts/controllers/app-state-controller.ts Outdated Show resolved Hide resolved

mikesposito reviewed Oct 3, 2024

View reviewed changes

app/scripts/controllers/app-state-controller.ts Outdated Show resolved Hide resolved

mikesposito reviewed Oct 3, 2024

View reviewed changes

app/scripts/controllers/app-state-controller.ts Outdated Show resolved Hide resolved

changes reflecting from review

bfe0d3f

replace preferences store with messenger call

744300f

kanthesha removed the DO-NOT-MERGE Pull requests that should not be merged label Oct 14, 2024

kanthesha added 5 commits October 14, 2024 14:44

resolve merge conflict

40a3938

Merge branch 'develop' into feat/app-state-controller-to-typescript

ffda478

replace private identifier in AppState

15dd5e5

Merge branch 'feat/app-state-controller-to-typescript' of github.com:…

19d6846

…MetaMask/metamask-extension into feat/app-state-controller-to-typescript

unit tests added

041c435

cryptodev-2s previously approved these changes Oct 16, 2024

View reviewed changes

mikesposito reviewed Oct 17, 2024

View reviewed changes

revert unused preferences controller removal from mmi-controller

a958732

kanthesha dismissed cryptodev-2s’s stale review via a958732 October 17, 2024 11:26

cryptodev-2s previously approved these changes Oct 17, 2024

View reviewed changes

resolve merge conflict

d54d158

kanthesha dismissed cryptodev-2s’s stale review via d54d158 October 18, 2024 11:04

remove not needed actions and events

2e86cfe

cryptodev-2s approved these changes Oct 18, 2024

View reviewed changes

DDDDDanica reviewed Oct 18, 2024

View reviewed changes

DDDDDanica approved these changes Oct 18, 2024

View reviewed changes

DDDDDanica added this pull request to the merge queue Oct 18, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 18, 2024

Merge branch 'develop' into feat/app-state-controller-to-typescript

ebe4f3d

kanthesha enabled auto-merge October 18, 2024 12:27

kanthesha added this pull request to the merge queue Oct 18, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 18, 2024

kanthesha added this pull request to the merge queue Oct 18, 2024

Merged via the queue into develop with commit 36bde61 Oct 18, 2024
76 checks passed

kanthesha deleted the feat/app-state-controller-to-typescript branch October 18, 2024 13:48

github-actions bot locked and limited conversation to collaborators Oct 18, 2024

metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Oct 18, 2024

gauthierpetetin added release-12.6.0 Issue or pull request that will be included in release 12.6.0 and removed release-12.8.0 Issue or pull request that will be included in release 12.8.0 labels Oct 21, 2024

feat: Convert AppStateController to typescript #27572

feat: Convert AppStateController to typescript #27572

Uh oh!

Conversation

kanthesha commented Oct 2, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Oct 2, 2024

Uh oh!

metamaskbot commented Oct 2, 2024

Uh oh!

kanthesha commented Oct 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikesposito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mikesposito commented Oct 3, 2024

Uh oh!

kanthesha commented Oct 4, 2024

Uh oh!

metamaskbot commented Oct 4, 2024

Uh oh!

metamaskbot commented Oct 14, 2024

Uh oh!

sonarqubecloud bot commented Oct 16, 2024

Quality Gate passed

Uh oh!

metamaskbot commented Oct 16, 2024

Uh oh!

mikesposito Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

kanthesha Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Oct 17, 2024

Uh oh!

DDDDDanica Oct 18, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Oct 18, 2024

Uh oh!

Uh oh!

metamaskbot commented Oct 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kanthesha commented Oct 2, 2024 •

edited by github-actions bot

Loading