Adding assets from node_modules is overly cumbersome

kuba-orlik commented 2 years ago

Is this a feature request or a bug?

Not a bug, but a request for improvement

What is the current behavior?

Making web-ext include a file from node_modules in the zip file with web-ext build is overly complex. Say, if I want to add a node_modules/react-dom/umd/react-dom.production.min.js, it is not enough to add --ignoreFiles '!**/node_modules/**/*/react-dom.production.min.js'. Due to how the matching is implemented, I have to add every directory on the path leading to that file, as well:

npx web-ext build --ignore-files '!**/node_modules' '!**/node_modules/**/react-dom' '!**/node_modules/**/react-dom/umd' '!**/node_modules/**/*/react-dom.production.min.js'

All that for a single file. When there's more files across different directories to add, this gets truly unwieldy.

What is the expected or desired behavior?

The expected behavior would be to --ignoreFiles !/node_modules/react-dom/umd/react-dom.production.min.js' be enough for the file to end up in the output zip

Version information (for bug reports)

Your OS and version: GNU\Linux, Arch
Paste the output of these commands:

node --version && npm --version && web-ext --version

v18.3.0
8.5.5
7.1.1

Rob--W commented 2 years ago

Are you sure that it doesn't work as expected?

Your sample pattern '!/node_modules/react-dom.production.min.js' does not match the sample path node_modules/react-dom/umd/react-dom.production.min.js. I would expect --ignoreFiles '![path here]' to work, e.g.--ignoreFiles !node_modules/react-dom/umd/react-dom.production.min.js'

Please provide the exact file paths and the exact command-line arguments if the logic is really not working as intended.

About the implementation:

The standard list of ignored files are listed at https://github.com/mozilla/web-ext/blob/416b12895109331170a5b4f3af64e83918f2488b/src/util/file-filter.js#L40-L47

and used at: https://github.com/mozilla/web-ext/blob/416b12895109331170a5b4f3af64e83918f2488b/src/util/file-filter.js#L57 https://github.com/mozilla/web-ext/blob/416b12895109331170a5b4f3af64e83918f2488b/src/util/file-filter.js#L89-L98

which in turn is passed to multimatch at https://github.com/mozilla/web-ext/blob/416b12895109331170a5b4f3af64e83918f2488b/src/util/file-filter.js#L112

To check the behavior, I ran the following code that only used the referenced library, at https://npm.runkit.com/multimatch/index.js

var multimatch = require("multimatch@5.0.0");
// ^ using 5.0.0 instead of 6.0.0. because RunKit doesn't support ESM yet.

multimatch(
  "/path/to/node_modules/react-dom/umd/react-dom.production.min.js",
  [
    "**/node_modules",
    "**/node_modules/**/*",
    "!/path/to/node_modules/react-dom/umd/react-dom.production.min.js",
    // or even: "!**/react-dom.production.min.js",
]);

// result: [] (empty array, i.e. input file should not be ignored by web-ext)

kuba-orlik commented 2 years ago

I've reproduced the issue here: https://github.com/kuba-orlik/web-ext-repro

In my original post I did in fact enter the wrong path, but the typo was not the cause of my issue. I've corrected the original issue text to make it more clear. The logic is really not working as intended.

I've prepared a script that lets you test it at a glance. Its output is:

File node_modules/react-dom/umd/react-dom.production.min.js exists on disk

>>> Trying with the simple glob: !/node_modules/react-dom/umd/react-dom.production.min.js...

Applying config file: ./package.json
Building web extension from /home/kuba/projects/midline/web-ext-repro
Destination exists, overwriting: /home/kuba/projects/midline/web-ext-repro/web-ext-artifacts/hello-0.1.8.zip
Your web extension is ready: /home/kuba/projects/midline/web-ext-repro/web-ext-artifacts/hello-0.1.8.zip

>>> Target file is NOT in the zip file

=================== 

>>> Now trying with the needlessly complex glob: !**/node_modules !**/node_modules/**/react-dom !**/node_modules/**/react-dom/umd !**/node_modules/**/*/react-dom.production.min.js...

Applying config file: ./package.json
Building web extension from /home/kuba/projects/midline/web-ext-repro
Destination exists, overwriting: /home/kuba/projects/midline/web-ext-repro/web-ext-artifacts/hello-0.1.8.zip
Your web extension is ready: /home/kuba/projects/midline/web-ext-repro/web-ext-artifacts/hello-0.1.8.zip

>>> Target file IS in the zip file

mozilla / web-ext