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

Repeated file changes doesn't properly compile manifest #628

Open
vwheezy opened this issue Mar 26, 2024 · 12 comments
Open

Repeated file changes doesn't properly compile manifest #628

vwheezy opened this issue Mar 26, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@vwheezy
Copy link
Contributor

vwheezy commented Mar 26, 2024

Using a mostly standard setup, the first dev compile outputs the correct manifest.json but every reload after uses the non-compiled version.

E.g. first compilation outputs:

manifest.json
{
  "name": "__MSG_appName__",
  "description": "__MSG_appDescription__",
  "version": "1.0.0",
  "short_name": "__MSG_appShortName__",
  "manifest_version": 3,
  "default_locale": "en",
  "background": {
    "scripts": [
      "serviceWorker.js",
      "webextension-toolbox/background_page.js"
    ]
  },
  "content_scripts": [
    {
      "matches": [
        "<all_urls>"
      ],
      "js": [
        "contentScript.js"
      ]
    }
  ],
  "permissions": [
    "storage",
    "scripting"
  ],
  "options_ui": {
    "page": "pages/options.html",
    "open_in_tab": true
  },
  "action": {
    "default_title": "__MSG_browserActionTitle__",
    "default_popup": "pages/popup.html",
    "default_icon": {
      "16": "images/icon-16x.png",
      "32": "images/icon-32x.png",
      "48": "images/icon-48x.png",
      "128": "images/icon-128x.png"
    }
  },
  "icons": {
    "16": "images/icon-16x.png",
    "32": "images/images-32x.png",
    "48": "images/icon-48x.png",
    "128": "images/icon-128x.png"
  },
  "browser_specific_settings": {
    "gecko": {
      "id": "addon@example.com"
    }
  }
}

Second compilation:

manifest.json
{
  "name": "__MSG_appName__",
  "short_name": "__MSG_appShortName__",
  "description": "__MSG_appDescription__",
  "manifest_version": 3,
  "default_locale": "en",
  "background": {
    "scripts": ["serviceWorker.js"]
  },
  "content_scripts": [
    {
      "matches": ["<all_urls>"],
      "js": ["contentScript.js"]
    }
  ],
  "permissions": ["storage", "scripting"],
  "options_ui": {
    "page": "pages/options.html",
    "open_in_tab": true
  },
  "action": {
    "default_title": "__MSG_browserActionTitle__",
    "default_popup": "pages/popup.html",
    "default_icon": {
      "16": "images/icon-16x.png",
      "32": "images/icon-32x.png",
      "48": "images/icon-48x.png",
      "128": "images/icon-128x.png"
    }
  },
  "icons": {
    "16": "images/icon-16x.png",
    "32": "images/images-32x.png",
    "48": "images/icon-48x.png",
    "128": "images/icon-128x.png"
  },
  "__firefox|safari__browser_specific_settings": {
    "gecko": {
      "id": "addon@example.com"
    }
  }
}

First compile output shows:

asset manifest.json 1.08 KiB [emitted]

Second compile output shows:

asset manifest.json 997 bytes [emitted] [from: manifest.json] [copied]

I've outputted the changedFiles variables and it's everything in the source tree excluding dist, but including manifest.json for some reason.

Tried using --copy-ignore but couldn't figure out how to inject "packages" with globs.

If this is a code issue, I could help submit a PR. If this is a setup issue, any help would be greatly appreciated!

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 26, 2024

I tried building the codebase, but I was getting a lot of errors without making any modifications. Some instructions would be appreciated and I could submit a CONTRIBUTING.md to help the next developers.

@tm1000
Copy link
Member

tm1000 commented Mar 26, 2024

@vwheezy have you looked at https://github.com/webextension-toolbox/webextension-toolbox which uses this plugin?

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 26, 2024

@tm1000 Yes, that's what I'm using currently. I thought the issue was with that codebase, but I'm fairly certain I tracked the issue down to here.

My --copy-ignore comment was actually in reference to webextension-toolbox (I was very tired writing that issue 😅)

@tm1000 tm1000 self-assigned this Mar 26, 2024
@tm1000 tm1000 added the bug Something isn't working label Mar 26, 2024
@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 27, 2024

How do you build the project? I have a good amount of time to try and fix this, plus I'll write some documentation for the future.

I'm new to developing npm packages, but trying to rebuild the codebase using npm run build-plugin & npm run build-client from inside node_modules does not work. However, it does when I clone the repo separately

@tm1000
Copy link
Member

tm1000 commented Mar 27, 2024

@vwheezy I believe I fixed those issues so please git pull and run npm upgrade.

If you still have errors then please post them here. Thanks

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 27, 2024

@tm1000 Ah so frustrating, I had gotten it working (without knowing how) and then I changed something and it's not working again (without knowing how) 🙂.

I don't think your changes fixed it. I tried it using both npm link and adding the path of my local copy of the repo in package.json.

Something I noticed: before I don't think I had the webextension-toolbox directory being emitted to dist. It's being emitted now, but the manifest is still incorrect. Additionally, when it first started working dist wasn't separating into different vendor folders. Not sure what that was about.

Thanks so much for the help. Let me know if there's anything I can do, I'm just banging my head against a wall trying to recreate the conditions where it was working.

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 27, 2024

Completely forgot to mention this a long time ago: any change in manifest.json correctly rebuilds manifest.json, any other modification noticed by webpack, including changes in messages.json and changes in dependencies of manifest.json, does not recompile the manifest correctly.

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 28, 2024

I got it to work by forcing the manifestChanged boolean to be true, but I'm sure that's not intended behavior.

I changed detectManifestModification to this:

  detectManifestModification(compiler: Compiler) {
    if (compiler.modifiedFiles && compiler.options.context) {
      console.log("!!! compiler modified files !!!", compiler.modifiedFiles);
      const manifestFile = path.join(
        compiler.options.context,
        this.manifestNameDefault
      );
      // this.manifestChanged = compiler.modifiedFiles.has(manifestFile);
      this.manifestChanged = true;
    }
  }

@tm1000
Copy link
Member

tm1000 commented Mar 28, 2024

What you could do is see the value of manifestFile and compare it to the list in compiler.modifiedFiles I'm thinking they are not matching after the first go-around

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 28, 2024

I'm not sure if this function is the issue. It correctly detects if manifestChanged. In my application, I'm not modifying the manifest, but when I do it forces a rebuild of the manifest.

I see in addManifest that it only rebuilds the manifest if it was changed, which also makes sense.

In keepFiles, I'm not sure how the CleanPlugin is used/called in the cycle of the plugin. I printed out the asset that would be matched against the function and only background_page.js shows up, not manifest.json. Is this the issue?

Also, changedFiles in reloadExtensions shows all of the files on every compile except for the first one, where it only shows the package output (xpi, zip, etc), background_page.js, and manifest.json. AFAICU, reloadExtensions doesn't actually affect the compilation of the files though, it just reloads the client side.

@tm1000
Copy link
Member

tm1000 commented Mar 29, 2024

Actually this appears to be an issue in https://github.com/webextension-toolbox/webextension-toolbox where the copy plugin is copying manifest.json back over. The fix is listed below for webextension-toolbox, havent applied it yet because I am cleaning up this plugin to make more sense.

diff --git a/src/common/webpack.ts b/src/common/webpack.ts
index c5ed136..ef569c8 100644
--- a/src/common/webpack.ts
+++ b/src/common/webpack.ts
@@ -137,8 +137,10 @@ export default async function webpackConfig({
     path: resolvedTarget,
     filename: "[name].js",
     chunkFilename: "[id].chunk.js",
+    // https://github.com/webpack/webpack-dev-middleware/issues/861
     clean: true,
   };
+
   /** *************************** */
   /*    WEBPACK.OPTIMIZATION    */
   /** *************************** */
@@ -241,6 +243,9 @@ export default async function webpackConfig({
     ignore: compileIgnoredArray,
   });
 
+  // Add manifest to compiled files list
+  compiledFiles.push(resolve(src, "manifest.json"));
+
   // Copy non compiled files
   config.plugins.push(
     new CopyPlugin({
@@ -251,7 +256,7 @@ export default async function webpackConfig({
           context: resolve(src),
           from: resolve(src, "**/*").replace(/\\/g, "/"),
           globOptions: {
-            ignore: [...copyIgnore, "manifest.json", ...compiledFiles],
+            ignore: [...copyIgnore, ...compiledFiles],
           },
           to: resolvedTarget,
         },

@vwheezy
Copy link
Contributor Author

vwheezy commented Mar 29, 2024

Briefly tested and it seems to work, thanks so much for the work!

I'll submit an issue and PR to the project and link it to this issue.

Not sure if this is the right place to ask this, but I've familiarized myself with this codebase and wanted to help contribute to this project more. Any issues, features, or documentation you need done?

@vwheezy vwheezy closed this as completed Mar 29, 2024
@vwheezy vwheezy reopened this Mar 29, 2024
@stale stale bot added the wontfix This will not be worked on label May 31, 2024
@tm1000 tm1000 removed the wontfix This will not be worked on label Jun 1, 2024
@webextension-toolbox webextension-toolbox deleted a comment from stale bot Jun 1, 2024
@stale stale bot added the wontfix This will not be worked on label Aug 1, 2024
@tm1000 tm1000 removed the wontfix This will not be worked on label Aug 7, 2024
@webextension-toolbox webextension-toolbox deleted a comment from stale bot Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants