#14583 broke the development build scripts (e.g. `yarn start`) by adding a positional argument to a package script (`build:dev`) that is used and passed positional arguments in the build script itself. This PR removes the positional argument from the `build:dev` script and `yarn start` now works again. In addition, the `--apply-lavamoat` flag is properly forwarded to child processes, which was not the case in the original implementation.
To test, `yarn start` should work and LavaMoat should _not_ be applied, in distinction to `yarn build:dev dev --apply-lavamoat=true`. Whether LavaMoat is applied can be determined by checking whether `Object.isFrozen(Object.prototype)` is `true` (with LavaMoat) or `false` (without LavaMoat).
Adds a new flag, `--apply-lavamoat`, to the main build script. The flag controls whether LavaMoat is actually applied to the output of the build process. The flag defaults to `true`, but we explicitly set it to `false` in the `start` package script. Meanwhile, the `start:lavamoat` script is modified such that it applies LavaMoat to the build output in development mode, but it no longer runs the build process itself under LavaMoat as there aren't very compelling reasons to do so.
This change is motivated by the fact that development builds do not have their own dedicated LavaMoat policies, which causes development builds to fail since #14537. The downside of this change is that LavaMoat-related failures will not be detected when running `yarn start`. @kumavis has plans for fixing this problem in a future major version of the `@lavamoat` suite.
* lavamoat - apply lavamoat protections to popup and notification
* build - enable lavamoat for home
* lavamoat - add missing ui overrides for react family
* deps/patches - patch zxcvbn for ses compat
Certain build steps accidentally omitted the `version` variable. It has
now been restored to all steps, ensuring that all environment variables
are correctly injected into all bundles.
A check has been added to the Sentry setup module to ensure the release
is not omitted in the future.
Certain build steps accidentally omitted the `version` variable. It has
now been restored to all steps, ensuring that all environment variables
are correctly injected into all bundles.
A check has been added to the Sentry setup module to ensure the release
is not omitted in the future.
This commit modifies the build system so that TypeScript files can be
transpiled into ES5 just like JavaScript files.
Note that this commit does NOT change the build system to run TypeScript
files through the TypeScript compiler. In other words, no files will be
type-checked at the build stage, as we expect type-checking to be
handled elsewhere (live, via your editor integration with `tsserver`,
and before a PR is merged, via `yarn lint`). Rather, we merely instruct
Babel to strip TypeScript-specific syntax from any files that have it,
as if those files had been written using JavaScript syntax alone.
Why take this approach? Because it prevents the build process from being
negatively impacted with respect to performance (as TypeScript takes a
significant amount of time to run).
It's worth noting the downside of this approach: because we aren't
running files through TypeScript, but relying on Babel's [TypeScript
transform][1] to identify TypeScript syntax, this transform has to keep
up with any syntax changes that TypeScript adds in the future. In fact
there are a few syntactical forms that Babel already does not recognize.
These forms are rare or are deprecated by TypeScript, so I don't
consider them to be a blocker, but it's worth noting just in case it
comes up later. Also, any settings we place in `tsconfig.json` will be
completely ignored by Babel. Again, this isn't a blocker because there
are some analogs for the most important settings reflected in the
options we can pass to the transform. These and other caveats are
detailed in the [documentation for the transform][2].
[1]: https://babeljs.io/docs/en/babel-plugin-transform-typescript
[2]: https://babeljs.io/docs/en/babel-plugin-transform-typescript#caveats
The version of a build is now derived from both the `version` field in
`package.json` and the requested build type and version. The build type
and version are added onto the manifest version as a suffix, according
to the SemVer prerelease format.
We already have support in the extension for versions of this format,
but to apply a Flask or Beta version required manual updates to
`package.json`. Now it can be done just with build arguments.
A `get-version` module was created to make it easier to generate the
version in the various places we do that during the build. It was
created in the `development/lib` directory because it will be used by
other non-build development scripts in a future PR.
The `BuildType` constant was extracted to its own module as well, and
moved to the `development/lib` directory. This was to make it clear
that it's used by various different development scripts, not just the
build.
This commit allows developers to write TypeScript files and lint them
(either via a language server in their editor of choice or through the
`yarn lint` command).
The new TypeScript configuration as well as the updated ESLint
configuration not only includes support for parsing TypeScript files,
but also provides some compatibility between JavaScript and TypeScript.
That is, it makes it possible for a TypeScript file that imports a
JavaScript file or a JavaScript file that imports a TypeScript file to
be linted.
Note that this commit does not integrate TypeScript into the build
system yet, so we cannot start converting files to TypeScript and
pushing them to the repo until that final step is complete.
The version of a build is now derived from both the `version` field in
`package.json` and the requested build type and version. The build type
and version are added onto the manifest version as a suffix, according
to the SemVer prerelease format.
We already have support in the extension for versions of this format,
but to apply a Flask or Beta version required manual updates to
`package.json`. Now it can be done just with build arguments.
A `get-version` module was created to make it easier to generate the
version in the various places we do that during the build. It was
created in the `development/lib` directory because it will be used by
other non-build development scripts in a future PR.
The `BuildType` constant was extracted to its own module as well, and
moved to the `development/lib` directory. This was to make it clear
that it's used by various different development scripts, not just the
build.
We would like to insert TypeScript into the ESLint configuration, and
because of the way that the current config is organized, that is not
easy to do.
Most files are assumed to be files that are suited for running in a
browser context. This isn't correct, as we should expect most files to
work in a Node context instead. This is because all browser-based files
will be run through a transpiler that is able to make use of
Node-specific variables anyway.
There are a couple of important ways we can categories files which our
ESLint config should be capable of handling well:
* Is the file a script or a module? In other words, does the file run
procedurally or is the file intended to be brought into an existing
file?
* If the file is a module, does it use the CommonJS syntax (`require()`)
or does it use the ES syntax (`import`/`export`)?
When we introduce TypeScript, this set of questions will become:
* Is the file a script or a module?
* If the file is a module, is it a JavaScript module or a TypeScript
module?
* If the file is a JavaScript module, does it use the CommonJS syntax
(`require()`) or does it use the ES syntax (`import`/`export`)?
To represent these divisions, this commit removes global rules — so now
all of the rules are kept in `overrides` for explicitness — and sets up
rules for CommonJS- and ES-module-compatible files that intentionally do
not overlap with each other. This way TypeScript (which has its own set
of rules independent from JavaScript and therefore shouldn't overlap
with the other rules either) can be easily added later.
Finally, this commit splits up the ESLint config into separate files and
adds documentation to each section. This way sets of rules which are
connected to a particular plugin (`jsdoc`, `@babel`, etc.) can be easily
understood instead of being obscured.
If an error occurs while running Browserify, the stream that Browserify
creates will emit an `error` event. However, this event is not being
handled, so Node will catch it instead. But the error message it
produces is very nebulous, as it merely spits out the stream object and
completely ignores the actual error that occurred. So this commit
listens for the `error` event and outputs the error.
One note here is that when we are outputting the error, we must get
around a bug that exists in Endo where if you pass an Error object to
`console.{log,error,info,debug}` then you will just see `{}` on-screen.
We get around this by printing `err.stack`.
This PR adds `snaps` under Flask build flags to the extension. This branch is mostly equivalent to the current production version of Flask, excepting some bug fixes and tweaks.
Closes#11626
* added fix for snaps devx issue
* reordered lines
* updated comment
* added test that ensures removeFencedCode detects a file with sourceMap inclusion
* fixed test
* Update development/build/transforms/remove-fenced-code.test.js
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
ESLint rules have been added to enforce our JSDoc conventions. These
rules were introduced by updating `@metamask/eslint-config` to v9.
Some of the rules have been disabled because the effort to fix all lint
errors was too high. It might be easiest to enable these rules one
directory at a time, or one rule at a time.
Most of the changes in this PR were a result of running
`yarn lint:fix`. There were a handful of manual changes that seemed
obvious and simple to make. Anything beyond that and the rule was left
disabled.
The ESLint config for the extension explicitly includes support for
Prettier. However, this is already being provided by our global ESLint
config (`@metamask/eslint-config`). Therefore there is no need to
include it here. In fact, this is causing weird issues where the `curly`
option is getting overridden somehow. After this change, these syntaxes
are invalid:
``` javascript
if (foo) return;
```
``` javascript
if (foo) return 'bar';
```
* Update support links for Flask
* Disable 'prefer-const' in code fence linting
* Add bespoke home footer for Flask and update logic
* fixup! Add bespoke home footer for Flask and update logic
* Fix code fence lint failure
* Fix support request link in account menu
* Fix unit test failure
The Firefox extension version format does not support the version
format we use (SemVer), so we have to specially format the extension
version to be compatible. The format we chose was
`[major].[minor].[patch].[buildType][buildVersion]`. But when we tried
to submit a build with a version in that format, it was rejected as
invalid for unknown reasons.
The Firefox extension format has been updated to
`[major].[minor].[patch][buildType][buildVersion]`. This seems to pass
validation.
The `version_name` manifest field was being used on Chrome to store the
build type. However, Chrome intended this field to be a full
representation of the version, for display purposes. This was evident
when uploading this version to the Chrome Web Store, because it used
`flask` as the entire version.
Instead the `version_name` field now includes the full SemVer version
string. The version parsing code within the build script and in the
wallet itself have been updated accordingly.
The build script only allowed prerelease versions for the "beta" build
type (e.g. `X.Y.Z-beta.0`). Now it allows Flask prerelease versions as
well.
This is required for the Flask release, where the prerelease version
helps distinguish the Flask error reports and metrics.
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 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.
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 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.
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.
Erik Marks
kumavis
Mark Stacey
Elliot Winkler
David Walsh
Dan Finlay
Niranjana Binoy
Jyoti Puri
Hassan Malik
ryanml