feat(27256): implement remote feature flag feature #28684

DDDDDanica · 2024-11-25T13:10:41Z

Description

Consume feature-flag-controller built in Create a new feature-flag-controller #27254.
And When Basic Functionality toggle is OFF, feature flag request should not be made, not call getFeatureFlags, which can be aligned with https://github.com/MetaMask/mobile-planning/issues/1975

Related issues

Fixes: #27256

Manual testing steps

Load the extension
Option 1: input chrome.storage.local.get(console.log) in dev tools console, and you'll find in data => RemoteFeatureFlagController => contains 2 dataset, cacheTimestamp and remoteFeatureFlags with value
Option 2: go to settings => advanced, and click download stage logs button, you'll find remoteFeatureFlags with cached value in your state.
Option 3: Settings => about page, if you open console in dev tools, there's a log after Feature flag fetched successfully, which will be removed after we implement 1st feature flag in production

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-11-25T13:10:54Z

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.

sentry-io · 2024-11-25T13:11:17Z

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: app/scripts/background.js

Function	Unhandled Issue
`setupController`	Error: Custom network client not found with ID '8f31c082-5580-4881-b948-616db1204095' ... `Event Count:` 230 `Affected Users:` 0
`initialize`	TypeError: e is not iterable /scripts/app-init.js `Event Count:` 147 `Affected Users:` 0
`setupController`	Error: NetworkController state is invalid: `selectedNetworkClientId` '54875d44-238f-416a-8033-765a3a2307... ... `Event Count:` 71 `Affected Users:` 0

_{Did you find this useful? React with a 👍 or 👎}

DDDDDanica · 2024-11-28T00:39:10Z

ui/pages/settings/settings.container.js

@@ -60,7 +61,7 @@ const mapStateToProps = (state, ownProps) => {
  const {
    metamask: { currencyRates },
  } = state;
-
+  const remoteFeatureFlags = getRemoteFeatureFlags(state);


Will be removed after 1st feature flag is implemented in production

DDDDDanica · 2024-11-28T00:39:17Z

ui/pages/settings/info-tab/info-tab.component.js

@@ -53,6 +57,13 @@ export default class InfoTab extends PureComponent {
  componentDidMount() {
    const { t } = this.context;
    handleSettingsRefs(t, t('about'), this.settingsRefs);
+    if (this.props.remoteFeatureFlags.length > 0) {


Will be removed after 1st feature flag is implemented in production

Instead of (this.props.remoteFeatureFlags.length > 0, can we use an actual feature flag. So that this becomes something like

if (this.props.remoteFeatureFlags[TEST_FLAG] > 0) { console.log(

So we can fully demonstrate how teams would use the feature flag system?

Good idea, added 5204e9c

danjm · 2024-11-28T17:28:24Z

app/scripts/background.js

@@ -885,12 +884,16 @@ export function setupController(
        senderUrl?.origin === `chrome-extension://${browser.runtime.id}`;
    }

+    controller.remoteFeatureFlagController.getRemoteFeatureFlags();


we will need error handling here now

and should we remove the controller.remoteFeatureFlagController.getRemoteFeatureFlags() call on 887, because we have one on 896?

Yes this line is removed, it was for testing purpose 🙏🏻

Error handling within initializeRemoteFeatureFlags can be found 5204e9c

app/scripts/background.js

socket-security · 2024-11-28T19:56:49Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

…ation

This reverts commit 5a8a088.

metamaskbot · 2024-12-03T01:25:58Z

Builds ready [7421c8e]

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, 7
- common: 0, 1, 10, 11, 12, 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 (1790 ± 41 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1635	1955	1792	85	41
		domContentLoaded	1622	1945	1765	86	41
		load	1645	1956	1790	85	41
		domInteractive	24	68	30	12	6
		backgroundConnect	9	52	23	13	6
		firstReactRender	15	38	18	5	2
		getState	109	317	141	56	27
		initialActions	0	1	0	0	0
		loadScripts	1188	1492	1344	76	36
		setupStore	7	77	11	15	7
		uiStartup	1885	2641	2095	179	86

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

background: 10.38 KiB (0.19%)
ui: 420 Bytes (0.01%)
common: 169 Bytes (0.00%)

DDDDDanica · 2024-12-03T01:51:48Z

@metamaskbot update-policies

metamaskbot · 2024-12-03T02:03:29Z

No policy changes

metamaskbot · 2024-12-03T02:23:58Z

Builds ready [6d474f7]

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, 7
- common: 0, 1, 10, 11, 12, 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 (2456 ± 138 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	284	2784	1701	924	444
		domContentLoaded	1835	3137	2426	299	144
		load	1895	3151	2456	288	138
		domInteractive	30	116	58	22	11
		backgroundConnect	7	76	34	23	11
		firstReactRender	17	51	28	9	4
		getState	109	323	167	44	21
		initialActions	0	1	0	0	0
		loadScripts	1423	2547	1875	260	125
		setupStore	7	75	14	14	7
		uiStartup	2214	3479	2868	329	158

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

background: 10.38 KiB (0.19%)
ui: 420 Bytes (0.01%)
common: 169 Bytes (0.00%)

app/scripts/constants/sentry-state.ts

Gudahtt

LGTM!

metamaskbot · 2024-12-03T14:32:51Z

Builds ready [2818e50]

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, 7
- common: 0, 1, 10, 11, 12, 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 (2185 ± 102 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1886	2634	2190	210	101
		domContentLoaded	1863	2502	2151	197	95
		load	1892	2586	2185	212	102
		domInteractive	27	76	41	15	7
		backgroundConnect	8	83	37	24	12
		firstReactRender	17	24	20	3	1
		getState	98	307	147	40	19
		initialActions	0	1	0	0	0
		loadScripts	1416	2021	1651	169	81
		setupStore	7	83	13	16	8
		uiStartup	2183	3360	2528	282	135

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

background: 10.38 KiB (0.19%)
ui: 420 Bytes (0.01%)
common: 144 Bytes (0.00%)

itsyoboieltr

LGTM! great work on this!

itsyoboieltr · 2024-12-03T15:41:28Z

app/scripts/background.js

@@ -1094,6 +1095,22 @@ export function setupController(
    }
  }

+  /**
+   * Initializes remote feature flags by making a request to fetch them from the clientConfigApi.
+   * This function is called when MM is during internal process.


Suggested change

* This function is called when MM is during internal process.

* This function is called during initialization.

just left a small nit

metamaskbot added the team-extension-platform label Nov 25, 2024

DDDDDanica force-pushed the feature/27256-remote-feature-flag branch 13 times, most recently from 3942cd2 to 9bddc8d Compare November 27, 2024 23:55

DDDDDanica commented Nov 28, 2024

View reviewed changes

danjm reviewed Nov 28, 2024

View reviewed changes

DDDDDanica force-pushed the feature/27256-remote-feature-flag branch from 0e78bb0 to e1c605a Compare November 28, 2024 17:43

danjm reviewed Nov 28, 2024

View reviewed changes

app/scripts/background.js Outdated Show resolved Hide resolved

DDDDDanica force-pushed the feature/27256-remote-feature-flag branch 2 times, most recently from d6d0a91 to a2b12b6 Compare November 28, 2024 19:56

DDDDDanica self-assigned this Nov 28, 2024

DDDDDanica force-pushed the feature/27256-remote-feature-flag branch from a2b12b6 to 79cdc1c Compare November 28, 2024 21:31

DDDDDanica marked this pull request as ready for review November 28, 2024 21:33

DDDDDanica requested review from a team as code owners November 28, 2024 21:33

metamaskbot and others added 10 commits December 2, 2024 21:43

Update LavaMoat policies

b99f174

Update ui/pages/settings/info-tab/info-tab.component.js

63b6f86

feat(27256): add controller state to sentry state

abb6709

feat(27256): modify initializeRemoteFeatureFlags to async and add jsdoc

e96d032

feat(27256): address types and required props

2b2da83

feat(27256): remove unused actions and events in controller initializ…

e76b5cc

…ation

Update LavaMoat policies

49f72e1

Revert "Update LavaMoat policies"

47f5df8

This reverts commit 5a8a088.

feat(27256): refactor types for _getConfigForRemoteFeatureFlagRequest

3d6ce59

Update LavaMoat policies

c8c2ec9

DDDDDanica force-pushed the feature/27256-remote-feature-flag branch 2 times, most recently from 517fd2c to 7421c8e Compare December 3, 2024 00:52

DDDDDanica requested a review from a team as a code owner December 3, 2024 00:52

update snapshot

6d474f7

DDDDDanica force-pushed the feature/27256-remote-feature-flag branch from 7421c8e to 6d474f7 Compare December 3, 2024 01:51

Gudahtt reviewed Dec 3, 2024

View reviewed changes

app/scripts/constants/sentry-state.ts Outdated Show resolved Hide resolved

feat(27256): remove duplicated FeatureFlagController: {},

2818e50

Gudahtt approved these changes Dec 3, 2024

View reviewed changes

itsyoboieltr approved these changes Dec 3, 2024

View reviewed changes

DDDDDanica added this pull request to the merge queue Dec 3, 2024

Merged via the queue into main with commit 589d7e4 Dec 3, 2024
75 checks passed

DDDDDanica deleted the feature/27256-remote-feature-flag branch December 3, 2024 16:29

github-actions bot locked and limited conversation to collaborators Dec 3, 2024

metamaskbot added the release-12.10.0 Issue or pull request that will be included in release 12.10.0 label Dec 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(27256): implement remote feature flag feature #28684

feat(27256): implement remote feature flag feature #28684

DDDDDanica commented Nov 25, 2024 •

edited

Loading

github-actions bot commented Nov 25, 2024

sentry-io bot commented Nov 25, 2024

DDDDDanica Nov 28, 2024

DDDDDanica Nov 28, 2024

danjm Nov 28, 2024

DDDDDanica Nov 28, 2024

danjm Nov 28, 2024

danjm Nov 28, 2024

DDDDDanica Nov 28, 2024

DDDDDanica Nov 28, 2024

socket-security bot commented Nov 28, 2024 •

edited

Loading

metamaskbot commented Dec 3, 2024

DDDDDanica commented Dec 3, 2024

metamaskbot commented Dec 3, 2024

metamaskbot commented Dec 3, 2024

Gudahtt left a comment

metamaskbot commented Dec 3, 2024

itsyoboieltr left a comment

itsyoboieltr Dec 3, 2024

itsyoboieltr Dec 3, 2024

	* This function is called when MM is during internal process.
	* This function is called during initialization.

feat(27256): implement remote feature flag feature #28684

feat(27256): implement remote feature flag feature #28684

Conversation

DDDDDanica commented Nov 25, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Nov 25, 2024

sentry-io bot commented Nov 25, 2024

🔍 Existing Issues For Review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

socket-security bot commented Nov 28, 2024 • edited Loading

metamaskbot commented Dec 3, 2024

DDDDDanica commented Dec 3, 2024

metamaskbot commented Dec 3, 2024

metamaskbot commented Dec 3, 2024

Gudahtt left a comment

Choose a reason for hiding this comment

metamaskbot commented Dec 3, 2024

itsyoboieltr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DDDDDanica commented Nov 25, 2024 •

edited

Loading

socket-security bot commented Nov 28, 2024 •

edited

Loading