Open mitar opened 3 years ago
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.
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).
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 theJsonService
code (again, great interface!) to use fetch (see https://github.com/IdentityModel/oidc-client-js/pull/1335 for PR because I think that should be default). The reason for fork is that it is not really easy to use a customJsonService
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:This:
Another approach could be to have
Global.JsonServiceCtor
. But that is more just a question where this config value is set.