Skip to content

Commit

Permalink
fix(mv3): fix file paths passed to app-init.js (#24059)
Browse files Browse the repository at this point in the history
## **Description**

Prior to this PR, the extension is stuck in a loading state when create
an mv3 dev build and attempt to load it. This error can be seen in the
console:

![Screenshot from 2024-04-16
12-23-35](https://github.com/MetaMask/metamask-extension/assets/7499938/0bb7fb5d-fdf4-4604-b045-a41011d6966b)


The problem was introduced in
#23672

That PR has the following in the description:

> ...The manifest file sometimes references files using a path relative
to both the repo and the bundled extension (background.html and
popup.html) and sometimes it doesn't (inpage.js lives in the scripts
folder).
> This PR makes the paths relative to the repo (and the bundled
extension).

The problem is with the file paths passed to `importScripts` in
`app-init.js`. The docs for for `importScripts` say that the path
parameter is `A string value representing the URL of the script to be
imported. The URL may be absolute or relative. If the URL is relative,
it is relative to the HTML document's base URL.`

Prior to this PR, the file paths were not "relative to the HTML
document's base URL". This PR corrects that.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24059?quickstart=1)

## **Related issues**

Fixes MetaMask/MetaMask-planning#2374. 

## **Manual testing steps**

1. `yarn start:mv3`
2. Install the extension
3. You should be able to open and use the extension UI

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
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.
  • Loading branch information
danjm authored Apr 17, 2024
1 parent 1134d5f commit 387216c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 13 deletions.
23 changes: 11 additions & 12 deletions app/scripts/app-init.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ let scriptsLoadInitiated = false;
const testMode = process.env.IN_TEST;

const loadTimeLogs = [];

// eslint-disable-next-line import/unambiguous
function tryImport(...fileNames) {
try {
Expand Down Expand Up @@ -57,27 +56,27 @@ function importAllScripts() {
throw new Error('Missing APPLY_LAVAMOAT environment variable');
}

loadFile('./scripts/sentry-install.js');
loadFile('../scripts/sentry-install.js');

// eslint-disable-next-line no-undef
const isWorker = !self.document;
if (!isWorker) {
loadFile('./scripts/snow.js');
loadFile('../scripts/snow.js');
}

loadFile('./scripts/use-snow.js');
loadFile('../scripts/use-snow.js');

// Always apply LavaMoat in e2e test builds, so that we can capture initialization stats
if (testMode || applyLavaMoat) {
loadFile('./scripts/runtime-lavamoat.js');
loadFile('./scripts/lockdown-more.js');
loadFile('./scripts/policy-load.js');
loadFile('../scripts/runtime-lavamoat.js');
loadFile('../scripts/lockdown-more.js');
loadFile('../scripts/policy-load.js');
} else {
loadFile('./scripts/init-globals.js');
loadFile('./scripts/lockdown-install.js');
loadFile('./scripts/lockdown-run.js');
loadFile('./scripts/lockdown-more.js');
loadFile('./scripts/runtime-cjs.js');
loadFile('../scripts/init-globals.js');
loadFile('../scripts/lockdown-install.js');
loadFile('../scripts/lockdown-run.js');
loadFile('../scripts/lockdown-more.js');
loadFile('../scripts/runtime-cjs.js');
}

// This environment variable is set to a string of comma-separated relative file paths.
Expand Down
2 changes: 1 addition & 1 deletion development/build/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ function createFactoredBuild({
const jsBundles = [
...commonSet.values(),
...groupSet.values(),
].map((label) => `./${label}.js`);
].map((label) => `../${label}.js`);
await createManifestV3AppInitializationBundle({
applyLavaMoat,
browserPlatforms,
Expand Down

0 comments on commit 387216c

Please sign in to comment.