fix(5955): ensure getEnvironment() function to return the correct environment values #38614
+280
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
DDDDDanica
force-pushed
the
fix/5955-env-logic
branch
from
8195ec3 to
bb5535f
Compare
Collaborator
Builds ready [bb5535f]
UI Startup Metrics (1223 ± 75 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
DDDDDanica
force-pushed
the
fix/5955-env-logic
branch
from
bb5535f to
3d520ee
Compare
Collaborator
Builds ready [3d520ee]
UI Startup Metrics (1220 ± 90 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
DDDDDanica
force-pushed
the
fix/5955-env-logic
branch
from
3d520ee to
c36137b
Compare
Collaborator
Builds ready [ec732ff]
UI Startup Metrics (1313 ± 117 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
DDDDDanica
force-pushed
the
fix/5955-env-logic
branch
from
68259ef to
143a532
Compare
DDDDDanica
commented
Dec 10, 2025
| * @param {string} taskName - The task name or build target. | ||
| * @returns {BUILD_TARGETS | string} The extracted build target. | ||
| */ | ||
| function getBuildTargetFromTask(taskName) { |
Contributor
Author
There was a problem hiding this comment.
Child processes receive task names (e.g., scripts:core:test:standardEntryPoints) instead of build targets (test). This function extracts the correct build target from task names so getEnvironment() works correctly in child processes.
DDDDDanica
commented
Dec 10, 2025
DDDDDanica
commented
Dec 10, 2025
| * @param args - The parsed CLI arguments | ||
| * @returns The resolved environment string | ||
| */ | ||
| export function resolveEnvironment( |
Contributor
Author
There was a problem hiding this comment.
Previously, webpack just used args.test ? 'testing' : env, which meant local --env production builds would report as production to Sentry. Now:
- production is NEVER auto-detected, it must be explicitly set
- Git context is checked for proper environment detection
DDDDDanica
commented
Dec 10, 2025
| if (args.dryRun) { | ||
| console.error(getDryRunMessage(args, features)); | ||
| const resolvedEnv = resolveEnvironment(args); | ||
| console.error(getDryRunMessage(args, features, resolvedEnv)); |
Contributor
Author
There was a problem hiding this comment.
Show the actual resolved environment in dry-run output.
DDDDDanica
force-pushed
the
fix/5955-env-logic
branch
from
143a532 to
b1516e9
Compare
Collaborator
Builds ready [b1516e9]
UI Startup Metrics (1277 ± 106 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Collaborator
Builds ready [6d9fdfb]
UI Startup Metrics (1286 ± 97 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Gudahtt
reviewed
Dec 10, 2025
Gudahtt
reviewed
Dec 10, 2025
Gudahtt
reviewed
Dec 10, 2025
Contributor
Builds ready [b0f9784]
UI Startup Metrics (1269 ± 111 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
HowardBraham
previously approved these changes
Jan 6, 2026
Contributor
Builds ready [86ad1cc]
UI Startup Metrics (1266 ± 125 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Member
|
The webpack changes look good, but the browserify changes are still a bit confusing to me. The bulk of the changes are a new function The description you gave of the problem is here:
But the manifest task is not run in a child process, it's only run in the root process. So effectively it's always correct. The browserify-based build script only uses child processes for the script tasks, the slowest ones. Technically the manifest task is created in each child processes, so if you had added logging to The changes you've made here still make the code easier to follow I think (at least now the manifest tasks in child processes will be correct, even though they're never run). But I just wanted to make sure I wasn't missing something here that would have some impact on the final build. |
Contributor
Author
|
@Gudahtt Thanks for pointing it out. And it is actually correct, after tracing through the code more carefully, I can confirm that:
I've removed those changes in this commit: 60fb0ed However, while investigating I found a real issue in The PR now focuses on:
Updated the description as well. |
Gudahtt
reviewed
Jan 7, 2026
Contributor
Builds ready [60fb0ed]
UI Startup Metrics (1273 ± 96 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [0ac4bd5]
UI Startup Metrics (1248 ± 98 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
HowardBraham
approved these changes
Jan 7, 2026
metamaskbot
added
the
release-13.14.0
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds proper environment detection to the webpack build system and fixes fragile build target detection in the browserify build.
Problem
Webpack: The webpack build was setting
METAMASK_ENVIRONMENTbased solely on--env, making it easy to accidentally pollute the production Sentry project with events from local or CI test builds.Browserify: The
getIgnoredFiles()function used fragile substring matching (target.includes('dev')) to determine build type. This worked by accident because task names likescripts:core:test:standardEntryPointscontain the build target as a substring, but would fail for differently named tasks.Solution
Final Result
Browserify Builds
yarn startdevdevelopmentyarn build:testtesttestingyarn test:e2e:singletestDevdevelopmentyarn distdistotheryarn build:prodprodproductionWebpack Builds
yarn webpack --env developmentdevelopmentyarn webpack --env productionotheryarn webpack --env production --targetEnvironment productionproductionyarn webpack --testtestingChangelog
CHANGELOG entry: null
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/5955
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/6014
Manual testing steps
Test script for webpack builds:
Results:
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Improves build correctness and safety by formalizing environment and build-target detection.
getBuildTargetFromTaskindevelopment/build/utils.jsand use it indevelopment/build/index.jsto reliably detect dev/test builds (replacingincludeschecks); export and test it (utils.test.ts).resolveEnvironmentindevelopment/webpack/utils/config.tsto derive MetaMask environment (testing, development, staging, release-candidate, pull-request, other) with explicit override via--targetEnvironment; wire intogetVariablesand dry-run output.--targetEnvironmentoption and enhancedgetDryRunMessagesignature/output; updatewebpack.config.tsto useresolveEnvironmentfor dry runs.development/webpack/README.md.Written by Cursor Bugbot for commit 0ac4bd5. This will update automatically on new commits. Configure here.