mozilla / web-ext

A command line tool to help build, run, and test web extensions
Mozilla Public License 2.0
2.7k stars 338 forks source link

Execution of `web-ext run` with `chromium` defined as target in config file does not launch Chromium #1862

Open sineway opened 4 years ago

sineway commented 4 years ago

Is this a feature request or a bug?

Bug

What is the current behavior?

Execution of web-ext run with chromium defined as target in config file does not launch Chromium, while web-ext run --target chromium works as expected.

What is the expected or desired behavior?

Launch Chromium

Version information

$ node --version
v13.11.0

$ npm --version
6.14.2

$ web-ext --version
4.0.0
Rob--W commented 4 years ago

Could you share the config file that you've used for your test? And did you confirm that it was loaded (that will be shown in the command output)?

sineway commented 4 years ago

It is simple web-ext-config.js in the project root

module.exports = {
    run: {
        target: [
            "firefox-desktop",
            "chromium"
        ]
    },
    build: {
        overwriteDest: true
    }
};

Yes, it is shown in the command output

$ web-ext run
Applying config file: ./web-ext-config.js
rpl commented 4 years ago

I suspected that the config was being overridden by the CLI parameters, and running web-ext in verbose mode does confirm that:

...
[program.js][info] Applying config file: ./web-ext-config.js
[config.js][debug] Loading JS config file: "/.../web-ext-config.js" (resolved to "/.../web-ext-config.js")
[config.js][debug] Favoring CLI: target=firefox-desktop over configuration: target=chromium,firefox-desktop

The "Favoring CLI" log is being logged by src/config.js: https://github.com/mozilla/web-ext/blob/de9211a4b5f62939ea360c36ac7554a8179d566a/src/config.js#L100-L106

But the value from cli is actually the default value for that option: https://github.com/mozilla/web-ext/blob/de9211a4b5f62939ea360c36ac7554a8179d566a/src/program.js#L494-L502

I verified that "removing the default value from the target cli option configuration" (linked above) does prevent this issue (well, to be precise it does "workaround the issue"), but the root cause is basically the same one behind #1327: the config file is being loaded after yargs has parsed the command line options and applied its own defaults.

And so I think that a more proper way to fix this issue (as well as #1327) would be to make sure that:

motin commented 4 years ago

For the record, #1327 is fixed in recently released 4.3.0 but this issue remains unresolved.

Rob--W commented 4 years ago

1327 was fixed by hooking on the validation logic, to defer the validation of required parameters until after reading from the config file.

If we want to fix this bug in a similar way, then we would have to figure out if it is possible to delay the logic that sets the defaults.

rpl commented 3 years ago

As a workaround for this issue (until we have been able to handle it with a more proper fix), --target=chromium can be passed as an explicit argument in an npm script, eg. something like:

package.json:

{
   ...
   "scripts": {
     "web-ext-run-chrome": "web-ext run --target=chromium"
     ...
   },
   ...
}

and all the rest of the cli options passed from the config file.