Fix backwards compatibility break for reporterOptions

[holm]

Description of the Change

In #4004 there was a change to use the documented reporterOption in favour of the setting that had always been used in practice called reporterOptions. This broke a lot of configurations that used the reporterOptions, which was the only way it every worked AFAIK. This is also clear in that all the built-in reporters use this name.

This changes the documentation to specify reporterOptions instead and ensure that any one that has switched to reporterOption after upgrade, still works.

Alternate Designs

reporterOption could have been chosen as the new preferred way, but objects with options are generally referred to in their plural form in JS. I think the root cause of the confusion comes from the command line option reporter-option. I think it makes sense that this is singular, since it is a single option, but it is compiled into a reporterOptions object.

Why should this be in core?

To fix related issues like michaelleeallen/mocha-junit-reporter#106 (comment)

Benefits

Make it easier to upgrade to Mocha 7

Possible Drawbacks

Slight overhead of supporting two aliases

Applicable issues

[jsf-clabot]

All committers have signed the CLA.

[coveralls]

Coverage increased (+0.07%) to 92.925% when pulling 17adf4a on peakon:issue/4142 into a24683f on mochajs:master.

[juergba]

I'm against using reporterOptions in the Mocha constructor.
We are using options in differnt context - CLI, Mocha constructor, browser environment - and there must be consistent rules for the option naming. Otherwise it's just a mess, for the user and for maintainance.

• CLI: use whatever canonical, alias name or its camel-cased version
• constructor: canonical or its camel-cased version
• browser: canonical or its camel-cased version

Version 7 is a major release and the best moment to implement this change.

Edit: some of the problems you report, must have a different cause, since they use Mocha v6.2.0

[holm]

I'm unsure what change you are requesting here. I'm not sure why the change I am proposing is inconsistent with anything.

Surely you do not want to require everyone to update their settings and all reporters to release new versions of their packages, which then in turn have to manage backwards compatibility with older versions of Mocha?

[juergba]

reporterOptions is not a canonical name, but an alias.

I'm planning to reduce the options passed into Mocha constructor (in our case: reporter-option, reporter-options, reporterOption, reporterOptions, O). So I don't agree to the change from reporterOption to reporterOptions as expected option name.

For backwards compability (L105) we could add reporterOptions as fallback, but reporterOption should remain the option to be.

[holm]

The Mocha constructor is never called with reporter-option or reporter-options AFAIK. That is only done via the CLI.

Searching the code base, up until your change, reporterOption was never used outside the documentation mistake. If anyone had passed it in, it would simply be ignored. From what I can tell you want to change it to use reporter-option on the command line and reporterOption in the rest of the code. I don't really see why that is better than my proposal of reporter-option in the CLI and reporterOptions in the rest of the code. They are not the same, one is a single option, the other is a set of options.

Since reporterOptions have always been used by everyone, rather than try to force this change on everyone, wouldn't it be better to just use my suggestion?

[juergba]

No, I don't want to change anything for use in CLI, everything remains as is.
In CLI you can use comma separators, so --reporter-option is not only a single option.

For the constructor, I need this rule for all options: "canonical or its camel-cased version".
"[...] have always been used [...]" is not a good argument, especially in a major release. Furthermore if we keep backwards compability with the mentioned fallback.

[juergba]

I added some detailed comments to avoid further misunderstandings. Please also adapt the test accordingly.

[juergba]

@holm thank you, just two small comments.

[juergba]

@holm thank you