Should default constructor arguments be moved to config?

[mitar]

First, I think this library is really amazing. You have great internal interfaces which allow one to nicely implement alternative implementations. E.g., I am using this library inside a Chrome extension so I implemented a storage class to use extension storage and specialized extension popup.

But I also want to run this library in a service worker of the extension. And things became complicated there: service workers do not have XMLHttpRequest, but only fetch. So I had to fork and change the JsonService code (again, great interface!) to use fetch (see #1335 for PR because I think that should be default). The reason for fork is that it is not really easy to use a custom JsonService class. Currently, JsonService is passed as a default value for a parameter to a constructor in multiple places, but it is hard to change that default value or pass some other value to those parameters.

I would suggest that all those constructor parameters with default values are also configured through config parameter. For example, in MetadataService, the initialization could then be instead of:

    constructor(settings, JsonServiceCtor = JsonService) {
        ...
        this._jsonService = new JsonServiceCtor(['application/jwk-set+json']);
    }

This:

    constructor(settings, JsonServiceCtor = JsonService) {
        ...
        this._jsonService = new (settings.JsonServiceCtor || JsonServiceCtor)(['application/jwk-set+json']);
    }

Another approach could be to have Global.JsonServiceCtor. But that is more just a question where this config value is set.

[brockallen]

I would suggest that all those constructor parameters with default values are also configured through config parameter.

Yep, this is a realization I came to a while back as well. I did build the first version of this thing like 7 years ago now (give or take?). Of course this would be a breaking change, so it's something on my TODO list if/when I ever get the cycles for a v2.

[mitar]

I do not think it is a breaking change if done like I described above (check if value is set in config, if not, use the value from the constructor).