The ESLint config has been updated to v8. The breaking changes are:
* The Prettier rule `quoteProps` has been changed from `consistent` to
`as-needed`, meaning that if one key requires quoting, only that key is
quoted rather than all keys.
* The ESLint rule `no-shadow` has been made more strict. It now
prevents globals from being shadowed as well.
Most of these changes were applied with `yarn lint:fix`. Only the
shadowing changes required manual fixing (shadowing variable names were
either replaced with destructuring or renamed).
The dependency `globalThis` was added to the list of dynamic
dependencies in the build system, where it should have been already.
This was causing `depcheck` to fail because the new lint rules required
removing the one place where `globalThis` had been erroneously imported
previously.
A rule requiring a newline between multiline blocks and expressions has
been disabled temporarily to make this PR smaller and to avoid
introducing conflicts with other PRs.
`remote-redux-devtools` is now explicitly excluded and disabled in non-
dev builds, and in the `testDev` build. This was causing console errors
in the `testDev` build during e2e tests, which would cause certain
tests to fail.
This was already only supposed to be enabled for development builds,
but this library used the `NODE_ENV` environment variable to make that
determination. This gives us more control over when it's disabled.
The React dev tools can result in console errors if dev tools is not
open during the test. Some of our e2e tests fail if there are any
console errors, so these errors break those tests.
`react-devtools` has been completely disabled for `testDev` builds to
make debugging e2e tests easier. The React dev tools can still be used
from development builds.
A propType error was showing up during e2e tests with a `testDev`
build. It was caused by `process.env.IN_TEST` being treated as a
boolean, when in fact it is either the string `'true'` or a boolean.
`IN_TEST` has been updated to always be a boolean. `loose-envify` has
no trouble injecting boolean values, so there's no reason to treat this
as a string.
The LavaMoat policy generation script would sporadically fail because
it ran the build concurrently three times, and the build includes
steps that delete the `dist` directory and write to it. So if one build
process tried to write to the directory after another deleted it, it
would fail.
This was solved by adding a new `--policy-only` flag to the build
script, and a new `scripts:prod` task. The `scripts:prod` task only
runs the script tasks for prod, rather than the entire build process.
The `--policy-only` flag stops the script tasks once the policy has
been written, and stops any other files from being written to disk.
This prevents the three concurrent build processes from getting in each
others way, and it dramatically speeds up the process.
The environment variables used for test builds was wrong for certain
bundles because the `testing` flag wasn't passed through to the
function that determines which environment variables to inject.
Effectively this means that test builds on `master` were going to the
production `metamask` Sentry project rather than the `test-metamask`
project. This has been the case since #11080.
The `testing` flag is now included for all bundles, and test builds now
use the `test-metamask` Sentry project in all cases.
This PR improves the error handling of the code fence removal transform stream by catching errors thrown by the `removeFencedCode` function and passing them to the `end` callback. This appears to resolve a problem where watched builds would blow up whenever a file with fences was reloaded.
This PR adds one LavaMoat background script policy or each build type. It also renames the build system policy directory from `node` to `build-system` to make its purpose more clear. Each build type has the original `policy-override.json` for `main` builds. The `.prettierignore` file has been updated to match the locations of the new auto-generated policy files.
We need to maintain separate policies for each build type because each type will produce different bundles with different internal and external modules.
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
The build system now supports platform-specific modifications to the
manifest for each build type. The need to customize the `id` on Firefox
motivated this change.
To support this, a new directory was made in each build type directory
for manifest changes. The images currently in this directory were moved
into an `images` subdirectory.
This new `manifest` directory can include each manifest file currently
in `app/manifest`. The `_base.json` file is assumed to exist, but the
platform manifest modifications are optional.
This PR enables the exclusion of JavaScript and JSON source by `buildType`, and enables the running of `eslint` under LavaMoat. 80-90% of the changes in this PR are `.patch` files and LavaMoat policy additions.
The file exclusion is designed to work in conjunction with our code fencing. If you forget to fence an import statement of an excluded file, the application will now error on boot. **This PR commits us to a particular naming convention for files intended only for certain builds.** Continue reading for details.
### Code Fencing and ESLint
When a file is modified by the code fencing transform, we run ESLint on it to ensure that we fail early for syntax-related issues. This PR adds the first code fences that will be actually be removed in production builds. As a consequence, this was also the first time we attempted to run ESLint under LavaMoat. Making that work required a lot of manual labor because of ESLint's use of dynamic imports, but the manual changes necessary were ultimately quite minor.
### File Exclusion
For all builds, any file in `app/`, `shared/` or `ui/` in a sub-directory matching `**/${otherBuildType}/**` (where `otherBuildType` is any build type except `main`) will be added to the list of excluded files, regardless of its file extension. For example, if we want to add one or more pages to the UI settings in Flask, we'd create the folder `ui/pages/settings/flask`, add any necessary files or sub-folders there, and fence the import statements for anything in that folder. If we wanted the same thing for Beta, we would name the directory `ui/pages/settings/beta`.
As it happens, we already organize some of our source files in this way, namely the logo JSON for Beta and Flask builds. See `ui/helpers/utils/build-types.js` to see how this works in practice.
Because the list of ignored filed is only passed to `browserify.exclude()`, any files not bundled by `browserify` will be ignored. For our purposes, this is mostly relevant for `.scss`. Since we don't have anything like code fencing for SCSS, we'll have to consider how to handle our styles separately.
The code fence transform was including contents after the final END directive twice. That was not covered by the tests, because none of the examples contained any content after the final END directive, and concatenating the empty string twice does not produce an observable difference in the test results.
This bug was due to an off-by-one error in the loop of the multiSplice function. The error has been fixed, and more test cases have been added.
Static files have been added for the Flask build. This includes logos
of each size and variety that we use, and it includes the 3D model JSON
file.
Closes#12427
The production build was accidentally broken in #12440 because of a
merge conflict with a #12441 that wasn't initially noticed. The
conflict was the renaming of the `BuildTypes` variable to `BuildType`.
This variable is used to check the current build type, but only for
production builds. `BuildTypes` is `undefined`, so this would result in
a crash when that enum was used.
The build script has been updated to embed the correct Infura project
ID and Segment write key for beta and Flask builds. These are set via
environment variable or config file. They have already been added in CI
as environment variables.
The Segment production write key has also been moved into the set of
environment variables that can be set in the configuration file. This
was to make the way we reference it more consistent.
The new project IDs and keys are only used in the "production"
environment, which right now is the merge step into the `master`
branch. This is appropriate for Flask, but it doesn't match our plan
for how the beta release would get created. In a future PR, when the
beta release automation work is completed, the conditions for when
the beta secrets are used should be updated to ensure they're used only
for the beta builds.
Closes#11896
Recently validation was added for our build configuration as part of
the PR #12438. This had the unintended consequence of making all builds
from forks fail because they don't get secrets injected. Specifically
it was the missing `INFURA_PROJECT_ID` that made the builds fail.
The Infura project ID is no longer required for building. In practice
it's still required for doing anything with a build but running e2e
tests, but that's all we need to do in CI anyway.
The build type (i.e. the distribution) is now included in the Sentry
environment during setup, for all builds except the "main" build. This
will allow us to track Flask and beta errors separately from other
errors.
A constant was created for the build types. The equivalent constant in
our build scripts was updated to match it more closely, for
consistency. We can't use the same constant in both places because our
shared constants are in modules that use ES6 exports, and our build
script does not yet support ES6 exports.
The singular `BuildType` was used rather than `BuildTypes` to match our
naming conventions elsewhere for enums. We name them like classes or
types, rather than like a collection.
Relates to #11896
We now use two separate Infura project IDs for production builds, and
for all other builds. Previously all CI builds used the production
Infura project ID. Separating them will make our Infura dashboard
metrics more representative of real production usage.
The new environment variable for production has been setup in CI
already, but the old environment variable will remain set to the
production project ID until this commit is included in a release.
We can't switch the old environment variable out until we're confident
that it won't get used for a production build.
We now use constants for the various different build environments. This
was done to improve the JSDoc types of the `getInfuraProjectId` helper
method.
The `getConfigValue` function was added to make it easier to validate
that required config values are set. This should ensure builds fail
early with an informative error message when they are missing the
necessary configuration.
The main `version` field in `package.json` will now include the beta
version (if present) rather than it being passed in via the CLI when
building. The `version` field is now a fully SemVer-compatible version,
with the added restriction that any prerelease portion of the version
must match the format `<build type>.<build version>`.
This brings the build in-line with the future release process we will
be using for the beta version. The plan is for each future release to
enter a "beta phase" where the version would get updated to reflect
that it's a beta, and we would increment this beta version over time as
we update the beta. The manifest gives us a place to store this beta
version. It was also important to replace the automatic minor bump
logic that was being used previously, because the version in beta might
not be a minor bump.
Additionally, the filename logic used for beta builds was updated to
be generic across all build types rather than beta-specific. This will
be useful for Flask builds in the future.
* lavamoat - add lavamoat to webapp background
* test:e2e - add delay to resolve failure
* test:e2e - add delay to resolve failure
* build - add a switch for applying lavamoat, currently off for all
* test/e2e - remove delays added for lavamoat
* Revert "test/e2e - remove delays added for lavamoat"
This reverts commit 79c3479f15c072ed362ba1d4f1af41ea11a17d63.
* lockdown - breakout making globalThis properties non-writable into lockdown-more.js
* Update app/scripts/lockdown-more.js
Co-authored-by: David Walsh <davidwalsh83@gmail.com>
* Update app/scripts/lockdown-more.js
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
Co-authored-by: David Walsh <davidwalsh83@gmail.com>
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
The MetaMask logo used for beta development builds was wrong. The lock
screen (and any other place using the `@metamask/logo` logo) showed the
correct logo, but all of our static assets used the "regular" logo.
Now the beta logo should be used everywhere for beta development
builds.
This is a refactor to replace the `isBeta` boolean with `buildType`
throughout the build system. This will allow us to modify the behaviour
of each step of the build process for Flask as well.
This should result in no functional changes.
This PR adds build-time code exclusion by means of code fencing. For details, please see the README in `./development/build/transforms`. Note that linting of transformed files as a form of validation is added in a follow-up, #12075.
Hopefully exhaustive tests are added to ensure that the transform works according to its specification. Since these tests are Node-only, they required their own Jest config. The recommended way to work with multiple Jest configs is using the `projects` field in the Jest config, however [that feature breaks coverage collection](https://github.com/facebook/jest/issues/9628). That being the case, I had to set up two separate Jest configs. In order to get both test suites to run in parallel, Jest is now invoked via a script, `./test/run-jest.sh`.
By way of example, this build system feature allows us to add fences like this:
```javascript
this.store.updateStructure({
...,
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
///: BEGIN:ONLY_INCLUDE_IN(beta)
PluginController: this.pluginController,
///: END:ONLY_INCLUDE_IN
});
```
Which at build time are transformed to the following if the build type is not `beta`:
```javascript
this.store.updateStructure({
...,
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
});
```
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
This rationalizes how arguments are passed to and parsed by the build system. To accomplish this, everything that isn't an environment variable from `.metamaskrc` or our CI environment is now passed as an argument on the command line.
Of such arguments, the `entryTask` is still expected as a positional argument in the first position (i.e. `process.argv[2]`), but everything else must be passed as a named argument. We use `minimist` to parse the arguments, and set defaults to preserve existing behavior.
Arguments are parsed in a new function, `parseArgv`, in `development/build/index.js`. They are assigned to environment variables where convenient, and otherwise returned from `parseArgv` to be passed to other functions invoked in the same file.
This change is motivated by our previous inconsistent handling of arguments to the build system, which will grow increasingly problematic as the build system grows in complexity. (Which it will very shortly, as we introduce Flask builds.)
Miscellaneous changes:
- Adds a build system readme at `development/build/README.md`
- Removes the `beta` package script. Now, we can instead call: `yarn dist --build-type beta`
- Fixes the casing of some log messages and reorders some parameters in the build system
This adds an `--omit-lockdown` flag to our build script, which will cause SES `lockdown` to be omitted from the resulting bundle. Useful for development when we don't want the environment to be locked down.
Thanks to @kumavis for the suggestion.
This PR makes ~all named intrinsics in all of our JavaScript processes non-modifiable. A named intrinsic is any property specified by the ECMAScript specification that exists on `globalThis` when the JavaScript process starts. We say that a property is non-modifiable if it is non-configurable and non-writable. We make exceptions for properties that meet any of the following criteria:
1. Properties that are non-configurable by the time `lockdown-run.js` is executed are not modified, because they can't be.
2. Properties that have accessor properties (`get` or `set`) are made non-configurable, but their writability cannot be modified, and is therefore left unchanged. It's unclear how many of the named intrinsics this applies to, if any, but it's good defensive programming, regardless.
The Sentry `release` was not being configured correctly. It was being
left blank. This is because the location of the extension version was
moved in #11029. The build script was correctly updated in that PR, but
that work was accidentally undone in a merge error that was included
in #11080.
Fixing up tests and add back old custom gas modal for non-eip1559 compliant networks
Remove unnecessary props from send-gas-row.component
fix breaking test
Fix primary and secondary title overrides
fix rebase issue
Fix rebase conflict
Co-authored-by: David Walsh <davidwalsh83@gmail.com>
Sentry is now configured with environment variables, rather than with
hard-coded values. This makes it easier to test Sentry functionality
using a different Sentry account, as we did recently during QA of
v9.5.1.
The only change for the normal build process is the introduction of the
`SENTRY_DSN_DEV` variable, which can be set via `.metamaskrc` or via an
environment variable. This determines where error reports are sent. It
still defaults to our team Sentry account's `metamask-testing` project.
The `sentry:publish` script now requires SENTRY_ORG and SENTRY_PROJECT
to be set in order to publish release artifacts. The CircleCI
configuration has been updated with these values, so it should act the
same as it did before. Previously we had used a CLI flag to specify the
organization and project, but Sentry already natively supports these
environment variables [1].
[1]: https://docs.sentry.io/product/cli/configuration/#configuration-values
The version field is now stored in the main `package.json` file rather
than in the base manifest. It is built into the final manifest during
the build script.
This makes it easier to communicate what the current version should be
to our `auto-changelog` script. It's also generally a more conventional
place to keep track of the version, even considering that we're not
publishing to npm.
