Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: add alternative build process to enable faster developer builds #22506

Merged
merged 58 commits into from
Aug 2, 2024

Conversation

davidmurdoch
Copy link
Contributor

@davidmurdoch davidmurdoch commented Jan 11, 2024

Closes https://github.com/MetaMask/MetaMask-planning/issues/1477 and https://github.com/MetaMask/MetaMask-planning/issues/1903

Prereq PR: #25456

About this PR

This PR adds an alternative build process that is much faster than the gulp build we have now, which is quite slow and doesn't make use of modern build system improvements. The speed up is possible by making use of the SWC compiler, and more modern build system. The build system is also a bit simpler and hopefully more maintainable.

This build doesn't yet support:

  • HMR/chromereload (requires we get rid of our circular dependencies)
  • lavamoat (neither running the build system in lavamoat, or adding lavamoat protections)
  • production builds (because of not supporting lavamoat)
  • MV3 (requires writing a webpack plugin)

Proposed Review Tactics

There is a lot to review in this PR. Depending on your own review methodologies, you might like the idea of breaking up review into several review passes, each one in a different categories/tactics. Some useful categories may cover things like:

  • Style (eslint customizations, exports, imports, use of require)
  • Architecture & Organization (naming, files and folder location, code separation)
  • Accuracy (are there bugs, unhandled exceptions, etc?)
  • Comments, Documentation, etc. (typos, inaccuracies, missing information?)
  • API (the new cli as well as yarn scripts)
  • Test Coverage and Test Organization, CI decisions (are tests lacking, are tests themselves maintainable and extensible, are there any unnecessary or missing tests?)

What's bad and should be discussed:

  • We now have to maintain 2 build systems

Linking the test ticket here https://github.com/MetaMask/MetaMask-planning/issues/2705

Copy link
Contributor

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 metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 11, 2024
@davidmurdoch davidmurdoch mentioned this pull request Jan 11, 2024
@metamaskbot metamaskbot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 11, 2024
@davidmurdoch davidmurdoch added the DO-NOT-MERGE Pull requests that should not be merged label Jan 12, 2024
Copy link

socket-security bot commented Jan 31, 2024

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
Network access npm/http-proxy-middleware@2.0.6 🚫
Network access npm/http-proxy@1.18.1 🚫
Network access npm/http-proxy@1.18.1 🚫
Network access npm/serve-index@1.9.1 🚫
Network access npm/spdy@4.0.2 🚫
Network access npm/spdy@4.0.2 🚫
Network access npm/spdy@4.0.2 🚫
Network access npm/spdy@4.0.2 🚫
Network access npm/spdy@4.0.2 🚫
Network access npm/multicast-dns@7.2.5 🚫
Network access npm/multicast-dns@7.2.5 🚫
Network access npm/sockjs@0.3.24 🚫
Shell access npm/launch-editor@2.6.1 🚫
Network access npm/webpack-dev-server@5.0.3 🚫
Network access npm/webpack-dev-server@5.0.3 🚫
Network access npm/webpack-dev-server@5.0.3 🚫
Network access npm/@lydell/node-pty@1.0.1 🚫
Shell access npm/@lydell/node-pty@1.0.1 🚫
Network access npm/@metamask/snaps-utils@8.0.0 🚫

View full report↗︎

Next steps

What is network access?

This module accesses the network.

Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

What is shell access?

This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code.

Packages should avoid accessing the shell which can reduce portability, and make it easier for malicious shell access to be introduced.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore npm/http-proxy-middleware@2.0.6
  • @SocketSecurity ignore npm/http-proxy@1.18.1
  • @SocketSecurity ignore npm/serve-index@1.9.1
  • @SocketSecurity ignore npm/spdy@4.0.2
  • @SocketSecurity ignore npm/multicast-dns@7.2.5
  • @SocketSecurity ignore npm/sockjs@0.3.24
  • @SocketSecurity ignore npm/launch-editor@2.6.1
  • @SocketSecurity ignore npm/webpack-dev-server@5.0.3
  • @SocketSecurity ignore npm/@lydell/node-pty@1.0.1
  • @SocketSecurity ignore npm/@metamask/snaps-utils@8.0.0

@davidmurdoch davidmurdoch force-pushed the webpack-dm-rebased branch 12 times, most recently from 1713a7f to 63f591f Compare March 4, 2024 18:26
@davidmurdoch davidmurdoch force-pushed the webpack-dm-rebased branch 8 times, most recently from 17a1d41 to 3f04e83 Compare March 8, 2024 16:10
@metamaskbot
Copy link
Collaborator

Builds ready [dea1e34]
Page Load Metrics (622 ± 371 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint663271428139
domContentLoaded9135383818
load412136622773371
domInteractive9135383818
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 81.68 KiB (2.36%)
  • ui: -11 Bytes (-0.00%)
  • common: 31.38 KiB (0.45%)

HowardBraham
HowardBraham previously approved these changes Aug 1, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [b7dc749]
Page Load Metrics (146 ± 161 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint753301135426
domContentLoaded96924168
load441604146335161
domInteractive96924168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 81.68 KiB (2.37%)
  • ui: -11 Bytes (-0.00%)
  • common: 31.38 KiB (0.45%)

@metamaskbot
Copy link
Collaborator

Builds ready [9ec67bd]
Page Load Metrics (151 ± 205 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6213185199
domContentLoaded9381794
load382012151427205
domInteractive9381794
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 81.68 KiB (2.37%)
  • ui: -11 Bytes (-0.00%)
  • common: 31.38 KiB (0.45%)

@hjetpoluru
Copy link
Contributor

@davidmurdoch, looks like this step is failing Socket Security: Pull Request Alerts any idea.

.circleci/config.yml Outdated Show resolved Hide resolved
@hjetpoluru hjetpoluru self-requested a review August 1, 2024 17:54
hjetpoluru
hjetpoluru previously approved these changes Aug 1, 2024
Copy link
Contributor

@hjetpoluru hjetpoluru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!! Awesome work @davidmurdoch

@davidmurdoch
Copy link
Contributor Author

davidmurdoch commented Aug 1, 2024

@davidmurdoch, looks like this step is failing Socket Security: Pull Request Alerts any idea.

This will go away once we merge.

.circleci/config.yml Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Aug 1, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [27872ae]
Page Load Metrics (237 ± 244 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint69145103199
domContentLoaded96221136
load411876237508244
domInteractive96221136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 81.68 KiB (2.37%)
  • ui: -11 Bytes (-0.00%)
  • common: 31.38 KiB (0.45%)

@davidmurdoch davidmurdoch merged commit 090cb0b into develop Aug 2, 2024
77 of 78 checks passed
@davidmurdoch davidmurdoch deleted the webpack-dm-rebased branch August 2, 2024 21:12
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
@metamaskbot metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Aug 2, 2024
@gauthierpetetin 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-contributor-experience MetaMask Contributor Experience Group team-extension-platform
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.