web-ext

web-ext copied to clipboard

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

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)?

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

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:

• the config file is discovered and loaded before yargs has parsed the command line option (which would prevent that yargs would fail for missing mandatory parameters missing from the command line because are configured in the config file, which is the underlying reason for #1327), but we would need to also take into account that as part of the CLI options the user may have also specified --config or --no-config-discovery options
• and be sure that values coming from config are overriding yargs default values for the options not explicitly part of the options on the command file (which is the underlying reason for this issue as described above)

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

#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.

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.