Stryker: Deprecate support of module.export = function(config) {}

[kmdrGroch]

With our current support of JS and JSON files I think we don't need support for module.exports = function(config) {} in config files.
It only takes more space, and gives no benefits IMO. Just decreases maintainability and adds more, unnecessary functionalities.
It would allow us to refactor ConfigReader, which right now is a little bit messy...

[nicojs]

I agree with this, but we are losing functionality here. Implementing it as a function allows users to use the defaults to determine the values.

For example:

module.exports = function(config) {
  config.fileLogLevel = config.logLevel; // this would no longer work
}

I'm not sure if anyone uses this functionality.

I would like to deprecate it for 1 major version with a deprecation message like:

DEPRECATED: exporting a function from your stryker config file is deprecated and will be removed in the future. Please export an object or put your configuration in "stryker.conf.json" file. If you think this is a mistake, please report it here _LINK TO ISSUE_

[kmdrGroch]

So am I allowed to take this issue and deprecate it's usage? We could remove it it 4.1.0 or 4.0 release, since 4.0 will for sure means breaking changes :)

[nicojs]

No, we're planning to release 4.0 as soon as mutation switching is implemented (hopefully within a month or so (knock on wood 🪑🔨)). I would remove it in v5 or even later. We're not sure yet, so don't mention the specific version in the deprecation message, should be fine.

[Lakitna]

Is it currently possible to make this an async function?

module.exports = async function(config) {
  // ...
}

I see some (niche) use cases for this. If this is currently possible I'd consider removing it feature regression.

[kmdrGroch]

I think it is not possible, although you would have to try... in you search for config.set in source files you will see how it is made :)

[Lakitna]

It took me some time to find it, but the function is not awaited: https://github.com/stryker-mutator/stryker/blob/epic/mutation-switching/packages/core/src/config/ConfigReader.ts#L34

So no, the function can not be async.

[nicojs]

Yeah, currently not possible.

We could add support in the future by exporting a promise, or using top level await (supported in native esmodules from nodejs 14).

[Lakitna]

This issue is about removing support for having a function, so I suggest not adding support 😄

[nicojs]

hahaha yeah, I meant in a separate issue and PR. I should communicate more clearly, thank you! 😁