Cysharp
[Feature Request] Adding the Commands Class to the service Container to enable access to it from other services via DI?
Is is correct that the following construct:
ConsoleApp.ServiceProvider = services.BuildServiceProvider();
var app = ConsoleApp.Create();
app.Add<MyCommands>();
and specifically app.Add<MyCommands>() is just a code signature the code generator runs on as the implementation of the resultant ConsoleApp.Add<T>() is empty:
public void Add<T>() { }
In the case that one is using DI, is it not unrealistic that a registered service might want to interact with the command class via an interface that could be injected into it?
Today is seem the code just new's up the Commands class (getting services from the DI container) and calls the requisite public function.
Could and alternative approach be to register the Command Class as say a Singleton (say), and any interfaces on it that one wants to leverage from other services.
services.AddSingleton<IMyService, MyService>();
services.AddSingleton<MyCommandClass>();
services.AddSingleton<IMyInterface>(provider => provider.GetRequiredService<MyCommandClass>());
And then I the generated use the ConsoleApp.ServiceProvider to instantiate the the Command Class (potentially updating these lines in the Emitter.cs - assuming the parser has the requisite information):
...
var instance = ServiceProvider!.GetRequiredService<global::MyCommandClass>();
...
The DI container will the inject the required services as needed based the constructor, and invoke the Command Function as before.
Also a potential advantage on registering and getting the Command Class back from the DI Container, is that is will (from Net8.0) handle Async/Disposal in the container - not that the library has not already solved for that otherwise - in a non DI way.
Add<T> is probably necessary to pass type information to the Source Generator.
It may be possible to make a change that prioritizes the Service Provider in instantiation.
var instance = provider.GetService<MyCommands>() ?? new MyCommands(); /* when T is concrete class, fallback */
If this satisfies your needs, I will consider implementing it.
Hola @neuecc - been distracted, thanks for coming back on this!
So the assumption will be that both the concrete class as well as its interfaces (per my example above) that are registered in the DI container. This could work, but does have a slight smell in that other services "could" grab (DI) the entire command. If this is what you can do that that would be great!
Another approach dependent on the desired degree of coupling between the library and the DI container, could be:
That there be a number of Add<TC> , and say Add<TC, TI1>, Add<TC, TI1, TI2>... and the Command <TC> is new'd up as it is today and that the ConsoleApp owns the Service Container (or creates it, if it needed per the overrides), and each Add registers each of the interfaces <TI1> etc. Some of the .Net generics seem to support up to 8x variants, see Action for example.
The Services Container would be accessible as an additional property, similar to the ServiceProvider such that other services can be registered.
I then guess the Run /RunAsync would then build the container, and expose that on the ServiceProvider property.
As above - I'd be happy to work with the more basic approach.
At some point there might be a request to support different DI containers, sure sure how that would then play in either way.
Hmm, by the way, are you actually struggling with this in a real-world case? These kinds of things often involve design trade-offs, so it's not advisable to unnecessarily add complexity just because it seems theoretically correct.
@neuecc
Hmm, by the way, are you actually struggling with this in a real-world case? These kinds of things often involve design trade-offs, so it's not advisable to unnecessarily add complexity just because it seems theoretically correct.
yes, I have a specific case for this. It's kinda of proprietary so it difficult to share the specifics, but do have a case when the Command launches other services that are DI'd, and there is status monitoring between then and the "state" of completeness of the cli command.
I'd suggest this is not an uncommon pattern?
I've a workaround that solves this for now by using an adapter service also registered in the DI container that implements IMyInterface the command injects this in and in turn registers it's IMyInterface interface with (this as IMyInterface) and the adapter delegates the interface calls though to the command. Whilst it works it an extra call on the stack, and it a tad kludgy?!?
If this satisfies your needs, I will consider implementing it.
Per you reply above what are you consideration timelines, and other change consideration per other issues - as would tie to a release? Do you have a roadmap? Is there something like a contribution I can do to help support that?
I'd also suggest this issue is also similarly valuable to the current use case - https://github.com/Cysharp/ConsoleAppFramework/issues/142
To be honest, I don't understand more than half of what's being said. As a result, I don't think any of the priorities are particularly high.