Describe the bug
As the title describes, the clientConfigBuilder.build(wrapperCache, resolver) method should accept a Resolver OR WrapperCache, not both.
The reason for this is the method is opaque to outside developers, and it's unclear what it does with these two arguments.
Essentially, the build method will only use the wrapperCache parameter if a resolver parameter is not provided, otherwise it will use the resolver parameter or build a completely new Resolver for the CoreClientConfig.
Expected behavior
This, in my view, is a bug, and can be easily mitigated by having the build method accept a single argument of type:
type BuildOptions = { resolver: IUriResolver<unknown> } | { wrapperCache: IWrapperCache };
This would make sure that developers are explicit in their intent, and that the build method behaves in accordance with the developer's intent.
Having a BuildOptions type instead of something simple like an IUriResolver | IWrapperCache is a preference, but it will allow us to add additional options in the future (maybe even for both cases) while not breaking the method signature.
Additional context
Found while working on the #1480 refactor.
Describe the bug
As the title describes, the
clientConfigBuilder.build(wrapperCache, resolver)method should accept a Resolver OR WrapperCache, not both.The reason for this is the method is opaque to outside developers, and it's unclear what it does with these two arguments.
Essentially, the
buildmethod will only use thewrapperCacheparameter if aresolverparameter is not provided, otherwise it will use theresolverparameter or build a completely new Resolver for theCoreClientConfig.Expected behavior
This, in my view, is a bug, and can be easily mitigated by having the
buildmethod accept a single argument of type:This would make sure that developers are explicit in their intent, and that the
buildmethod behaves in accordance with the developer's intent.Having a
BuildOptionstype instead of something simple like anIUriResolver | IWrapperCacheis a preference, but it will allow us to add additional options in the future (maybe even for both cases) while not breaking the method signature.Additional context
Found while working on the #1480 refactor.