refactor: ENABLE_MV3 flag cleanup by HowardBraham · Pull Request #26059 · MetaMask/metamask-extension

HowardBraham · 2024-07-23T18:01:35Z

Description

The way that the ENABLE_MV3 flag was processed was kind of a mess. Sometimes it wasn't checking both options of 'true' and undefined, sometimes it was only checking browser.runtime.getManifest().manifest_version, and it was always loquacious.

This PR centralizes it and defines it as:

const runtimeManifest =
  global.chrome?.runtime.getManifest() || global.browser?.runtime.getManifest();

const isManifestV3 = runtimeManifest
  ? runtimeManifest.manifest_version === 3
  : process.env.ENABLE_MV3 === 'true' || // Tests on Node.js processes
    process.env.ENABLE_MV3 === undefined;

Related issues

Supersedes: #25242
(@legobeat started working on an aspect of this, but it got stale. This PR incorporates his work.)

Manual testing steps

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.

fix: skip MV3-only tests on MV2 instead of marking them as passing

- push-platform-notifications

github-actions · 2024-07-23T18:01:52Z

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-07-25T05:54:19Z

Builds ready [3bb6394]

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
- common: 0, 1, 10, 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 (259 ± 251 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	286	119	44	21
		domContentLoaded	10	85	33	21	10
		load	41	1963	259	524	251
		domInteractive	10	85	33	21	10

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

background: 214 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: -110 Bytes (-0.00%)

codecov · 2024-07-30T01:00:58Z

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Project coverage is 69.97%. Comparing base (3f09c80) to head (7c61ca7).
Report is 2 commits behind head on develop.

Files	Patch %	Lines
...tform-notifications/push-platform-notifications.ts	0.00%	3 Missing ⚠️
shared/modules/mv3.utils.js	75.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26059      +/-   ##
===========================================
- Coverage    69.97%   69.97%   -0.00%     
===========================================
  Files         1411     1411              
  Lines        49995    50000       +5     
  Branches     13805    13809       +4     
===========================================
+ Hits         34981    34984       +3     
- Misses       15014    15016       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DDDDDanica · 2024-07-31T15:21:45Z

Looks solid to me 👍🏻 thanks for the refactor !

metamaskbot · 2024-07-31T23:25:54Z

Builds ready [c180fad]

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
- common: 0, 1, 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 (255 ± 251 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	307	115	48	23
		domContentLoaded	10	195	31	40	19
		load	43	1902	255	523	251
		domInteractive	10	195	31	40	19

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

background: 214 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: -110 Bytes (-0.00%)

hjetpoluru · 2024-08-01T02:06:06Z

@HowardBraham, all the tests are passing and code looks good. I ran the below tests using the below commands.

yarn
yarn start:test
yarn test:e2e:single test/e2e/tests/account/lockdown.spec.js --browser=chrome
yarn test:e2e:single test/e2e/tests/account/import-flow.spec.js --browser=chrome

yarn
ENABLE_MV3=false yarn start:test
ENABLE_MV3=false yarn test:e2e:single test/e2e/tests/account/lockdown.spec.js --browser=firefox 
ENABLE_MV3=false yarn test:e2e:single test/e2e/tests/account/import-flow.spec.js --browser=firefox

Could you make the changes to these files too.
shared/modules/mv3.utils.js
test/e2e/background-socket/socket-background-to-mocha.ts

metamaskbot · 2024-08-01T02:49:50Z

Builds ready [0088293]

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, 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 (443 ± 323 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	65	370	140	99	47
		domContentLoaded	9	73	24	20	10
		load	40	1681	443	674	323
		domInteractive	9	73	24	20	10

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

background: 345 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: -110 Bytes (-0.00%)

hjetpoluru

LGTM

sonarqubecloud · 2024-08-01T13:52:28Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

metamaskbot · 2024-08-01T14:26:47Z

Builds ready [7c61ca7]

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, 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 (568 ± 412 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	72	249	121	45	22
		domContentLoaded	10	182	31	37	18
		load	48	2387	568	858	412
		domInteractive	10	182	31	37	18

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

background: 345 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: -110 Bytes (-0.00%)

legobeat and others added 10 commits June 20, 2024 15:26

chore(test): Fix ENABLE_MV3 parsing consistency

0829976

chore(test): Fix ENABLE_MV3 parsing consistency

8a0d0e0

fix: skip MV3-only tests on MV2 instead of marking them as passing

fix(push-platform-notifications): Fix ENABLE_MV3 parsing consistency

eed4d31

lint:fix

7e16413

fix: use isManifestV3 instead of process.env.ENABLE_MV3 in runtime

4c804cf

- push-platform-notifications

chore: make isManifestV3 consider env.ENABLE_MV3 if env.IN_TEST

4dd763c

lint:fix

ec429a4

Merge branch 'fix-notifications-mv3-toggle' into mv3-flag-cleanup

76603e7

mv3 flag cleanup

433777e

Merge branch 'develop' into mv3-flag-cleanup

5bd105b

HowardBraham added the team-contributor-experience label Jul 23, 2024

HowardBraham self-assigned this Jul 23, 2024

metamaskbot added the team-accounts-framework Accounts team label Jul 23, 2024

Merge branch 'develop' into mv3-flag-cleanup

58a7a30

legobeat mentioned this pull request Jul 24, 2024

chore: eslintignore import/unambiguous for mv3.utils.js #26070

Closed

fix logic for unit tests

3f30e87

HowardBraham removed the team-accounts-framework Accounts team label Jul 24, 2024

legobeat and others added 3 commits July 25, 2024 12:56

Merge branch 'develop' into mv3-flag-cleanup

588b63e

fix: interpret multipart errors correctly and allow ignore

a29ce9f

Merge branch 'fix-multipart-errors' into mv3-flag-cleanup

3bb6394

HowardBraham changed the base branch from develop to fix-multipart-errors July 25, 2024 04:22

HowardBraham added the type-tech-debt Technical debt label Jul 26, 2024

Base automatically changed from fix-multipart-errors to develop July 29, 2024 16:07

Merge branch 'develop' into mv3-flag-cleanup

09e7c52

HowardBraham marked this pull request as ready for review July 30, 2024 01:01

HowardBraham requested review from a team and kumavis as code owners July 30, 2024 01:01

legobeat previously approved these changes Jul 31, 2024

View reviewed changes

Merge branch 'develop' into mv3-flag-cleanup

c180fad

added to socket-background-to-mocha.ts

3842060

HowardBraham dismissed stale reviews from legobeat and DDDDDanica via 87180b4 August 1, 2024 02:09

Merge branch 'develop' into mv3-flag-cleanup

0088293

HowardBraham force-pushed the mv3-flag-cleanup branch from 87180b4 to 0088293 Compare August 1, 2024 02:11

hjetpoluru requested a review from Prithpal-Sooriya August 1, 2024 02:15

hjetpoluru added the team-notifications-deprecated DEPRECATED: please use "team-assets" instead label Aug 1, 2024

hjetpoluru approved these changes Aug 1, 2024

View reviewed changes

hjetpoluru requested review from DDDDDanica and legobeat August 1, 2024 12:41

legobeat approved these changes Aug 1, 2024

View reviewed changes

Merge branch 'develop' into mv3-flag-cleanup

7c61ca7

Prithpal-Sooriya approved these changes Aug 1, 2024

View reviewed changes

HowardBraham merged commit 80ac1b2 into develop Aug 1, 2024

HowardBraham deleted the mv3-flag-cleanup branch August 1, 2024 15:23

github-actions bot locked and limited conversation to collaborators Aug 1, 2024

github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Aug 1, 2024

metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 1, 2024

gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.0 labels Sep 11, 2024

HowardBraham added contributor experience An issue that impacts, or planned improvement to, the contributor experience. and removed team-contributor-experience labels Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: ENABLE_MV3 flag cleanup#26059

refactor: ENABLE_MV3 flag cleanup#26059
HowardBraham merged 21 commits intodevelopfrom
mv3-flag-cleanup

HowardBraham commented Jul 23, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jul 23, 2024

Uh oh!

metamaskbot commented Jul 25, 2024

Uh oh!

codecov bot commented Jul 30, 2024 •

edited

Loading

Uh oh!

DDDDDanica commented Jul 31, 2024

Uh oh!

metamaskbot commented Jul 31, 2024

Uh oh!

hjetpoluru commented Aug 1, 2024 •

edited

Loading

Uh oh!

metamaskbot commented Aug 1, 2024

Uh oh!

hjetpoluru left a comment

Uh oh!

sonarqubecloud bot commented Aug 1, 2024

Uh oh!

metamaskbot commented Aug 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

HowardBraham commented Jul 23, 2024 • edited 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 Jul 23, 2024

Uh oh!

metamaskbot commented Jul 25, 2024

Uh oh!

codecov bot commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DDDDDanica commented Jul 31, 2024

Uh oh!

metamaskbot commented Jul 31, 2024

Uh oh!

hjetpoluru commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metamaskbot commented Aug 1, 2024

Uh oh!

hjetpoluru left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 1, 2024

Quality Gate passed

Uh oh!

metamaskbot commented Aug 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

HowardBraham commented Jul 23, 2024 •

edited

Loading

codecov bot commented Jul 30, 2024 •

edited

Loading

hjetpoluru commented Aug 1, 2024 •

edited

Loading