Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using MS.Extensions.DI directly, removing all unneeded code, pulling in the MediatR package directly #802

Merged
merged 3 commits into from
Nov 20, 2022

Conversation

jbogard
Copy link
Owner

@jbogard jbogard commented Nov 20, 2022

Brrrrrrrr

@jbogard jbogard merged commit 963df38 into master Nov 20, 2022
@jbogard jbogard deleted the ms-ext-di branch November 21, 2022 02:40
@davidroth
Copy link

davidroth commented Nov 22, 2022

@jbogard Just wanted to ask if you are aware of the consequences for simple injector (and maybe other containers).
Simple Injector is a "non-conforming" DI Container, meaning that it doesn't replace the built-in DI Container, but instead lives side-by-side the built-in container. Where the built-in container is used to resolve framework and third-party components, Simple Injector is used by the application developer to resolve their application components.

For general discussion regarding conforming containers see from former discussions such as dotnet/aspnetcore#30115, dotnet/aspnetcore#29194, dotnet/aspnetcore#28957, dotnet/aspnetcore#14585, dotnet/aspnetcore#8886 and aspnet/DependencyInjection#334.

So this change would break support for SimpleInjector. Realistically, there would be no way back (rather than forking).
Of course, its your project and anyone is free to fork it😅

@jbogard
Copy link
Owner Author

jbogard commented Nov 22, 2022

How would this break exactly? Doesn't the Container object implement IServiceProvider?

This doesn't force people to use the ServiceCollection stuff, it just bundles it together. The breaking change to the Mediator class is to change the dependency from my custom delegate type of ServiceFactory to the System.ComponentModel type IServiceProvider.

@davidroth
Copy link

This doesn't force people to use the ServiceCollection stuff, it just bundles it together.

Ah, I see. Thanks for the clarification. I originally thought it requires using the frameworks DI Container (IServiceCollection).
But that is not the case as you said. By using the following registration we can still control the registration in the composition root:

Container.RegisterInstance(typeof(IMediator), new Mediator(Container.GetInstance));

Doesn't the Container object implement IServiceProvider?

It does currently, yes. May be removed in the future though. But then we can still write a simple adapter:

Container.RegisterInstance(typeof(IMediator), new Mediator(new ServiceProviderAdapter { Container = container }));

public class ServiceProviderAdapter : IServiceProvider
{
    public Container Container {get;set;}
    public object GetService(Type type) => this.Container.GetInstanceOrNull(type);
}

So everything seems to be good :-)

@jbogard
Copy link
Owner Author

jbogard commented Nov 22, 2022

Yep, you should be able to write a simple adapter. MediatR used to use CommonServiceLocator years and years ago so this is a bit of return to form. I don't plan on altering anything with MediaR internals to force using its registration. In fact, I'm looking at adding factory objects to make it a bit more explicit about instantiating things like behaviors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants