Crash when startUrl option is an array in config file

[dessant]

Is this a feature request or a bug?

bug

What is the current behavior?

web-ext-config.js

module.exports = {
  run: {
    startUrl: ['about:debugging', 'about:addons']
  }
};

$ web-ext run -s dist --verbose
[program.js][debug] Getting the version from package.json
[program.js][info] Version: 2.3.1
[program.js][debug] Getting the version from package.json
[program.js][debug] Discovering config files. Set --no-config-discovery to disable
[config.js][debug] Discovered config "/home/prc/.web-ext-config.js" does not exist or is not readable
[program.js][info] Applying config file: ./web-ext-config.js
[config.js][debug] Loading JS config file: "/media/s1/dev/pr/clear-browsing-data/web-ext-config.js" (resolved to "/media/s1/dev/pr/clear-browsing-data/web-ext-config.js")
[program.js][error] 
UsageError: The config file at /media/s1/dev/pr/clear-browsing-data/web-ext-config.js specified the type of "startUrl" incorrectly as "array" (expected type "string")
    at applyConfigToArgv (/media/s1/dev/pr/clear-browsing-data/node_modules/web-ext/dist/webpack:/src/config.js:75:13)
    at applyConfigToArgv (/media/s1/dev/pr/clear-browsing-data/node_modules/web-ext/dist/webpack:/src/config.js:45:17)
    at /media/s1/dev/pr/clear-browsing-data/node_modules/web-ext/dist/webpack:/src/program.js:195:24
    at Array.forEach (<anonymous>)
    at Program._callee$ (/media/s1/dev/pr/clear-browsing-data/node_modules/web-ext/dist/webpack:/src/program.js:193:19)
    at tryCatch (/media/s1/dev/pr/clear-browsing-data/node_modules/regenerator-runtime/runtime.js:62:40)
    at Generator.invoke [as _invoke] (/media/s1/dev/pr/clear-browsing-data/node_modules/regenerator-runtime/runtime.js:296:22)
    at Generator.prototype.(anonymous function) [as next] (/media/s1/dev/pr/clear-browsing-data/node_modules/regenerator-runtime/runtime.js:114:21)
    at step (/media/s1/dev/pr/clear-browsing-data/node_modules/web-ext/dist/webpack:/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:1)
    at /media/s1/dev/pr/clear-browsing-data/node_modules/web-ext/dist/webpack:/node_modules/babel-runtime/helpers/asyncToGenerator.js:28:1
    at <anonymous>

[program.js][debug] Command executed: run

What is the expected or desired behavior?

The browser should start and open the specified pages without errors.

Version information (for bug reports)

• Firefox version: 57.0.4
• Your OS and version: Arch Linux

$ node --version && npm --version && web-ext --version
v8.9.1
5.5.1
2.3.1

[kumar303]

Thanks for reporting this.

@rpl I guess we need to simulate what yargs does in this case and accept the array but validate each item in the array.

Looking at type, I think we can accept an array for boolean, number, or string. The count type is weird and we don't use it. It's probably not worth the effort.

[rpl]

@kumar303 yeah I agree.

I took a brief look at the related code and it seems that we need at least to change the following pieces:

• web-ext/src/config.js

  Lines 74 to 78 in 07d8211

  	if (optionType !== expectedType) {
  	throw new UsageError(`The config file at ${configFileName} specified ` +
  	`the type of "${option}" incorrectly as "${optionType}"` +
  	` (expected type "${expectedType}")`);
  	}

  
  here we should allow the config value also if:
  • it is an array of elements
  • the elements of the array are all of the expected type from the config
    (for boolen, number and string types)

• web-ext/src/config.js

  Lines 107 to 112 in 07d8211

  	const coerce = options[decamelizedOptName].coerce;
  	if (coerce) {
  	log.debug(
  	`Calling coerce() on configured value for ${option}`);
  	newArgv[option] = coerce(newArgv[option]);
  	}

  
  here we should probably apply coerce to the single elements of the array

[rpl]

@kumar303 I've been thinking about this and looked in all the types of the currently supported parameters and I'm not sure that we can arbitrarily accept a array for all the command of type string, number or boolean, the reason is that for most of the options that we have an array doesn't make sense and/or it would trigger even more issues because of the parameter unexpectantly becoming an array, just to make some example:

• what an array of boolean config values for the --pretty parameter for the "web-ext lint" should actually means?

• what an array of string config values for the --artifacts-dir should actually means? (we can use just one artifact dir for the generated assets, more than one just doesn't make sense and there is no point of supporting it)

• what an array of number values for --timeout parameter of the "web-ext sign" command should be used for?

And so, as an alternative approach, I think that we could on the contrary set as type 'array' only the parameters for which it makes sense to have an array of config values, because we also know that we are going to handle it correctly, e.g. I think that the following ones may fall in this category:

• pref (an array of preferences to set)
• start-url (an array of urls to open)
• target (an array of target where the extension should run, e.g. ['firefox-desktop', 'firefox'android'])

How that sounds to you?

[kumar303]

Fixed in #1221