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

Bug: The simple injector DI container isn't practical #240

Open
glennawatson opened this issue Jan 22, 2019 · 6 comments
Open

Bug: The simple injector DI container isn't practical #240

glennawatson opened this issue Jan 22, 2019 · 6 comments
Labels

Comments

@glennawatson
Copy link
Contributor

As originally reported by @chuuddo, they have provided unit tests to help produce the exception here:

master...chuuddo:rxui_initialization_breaks_simpleinjector_resolver

Details of their explanation:
SimpleInjectorDependencyResolver not works in real apps.
I made a pull request #239 with changes to View and ViewModel classes to start initialization of RxApp and all tests fails.

After some investigations i found this issues:

  1. The container is locked after the first call to resolve but ReactiveUI invoke all registrations after RxApp.EnsureInitialized(). This can be fixed by not getting instace of MainWindow from container. If there are no other solutions, we should add this workaround to documentation.
    //var window = (MainWindow)container.GetInstance<IViewFor<MainViewModel>>();
    var window = new MainWindow();
    window.ViewModel = container.GetInstance<MainViewModel>();
    window.Show();
  1. SimpleInjector support multiple registrations for same interface only through Collection.Register. We can't register this lines using _container.Register() method.

_container.Register(serviceType, factory);

I don't have any workarounds for second issue. Can we use this DI container with ReactiveUI?

Originally posted by @chuuddo in #233 (comment)

@glennawatson
Copy link
Contributor Author

Again thanks for taking the time for letting us know about the issue @chuuddo

@RLittlesII RLittlesII added the Bug label Feb 22, 2019
@dpvreony
Copy link
Member

dpvreony commented Jul 16, 2019

@dpvreony
Copy link
Member

dpvreony commented Aug 3, 2019

doesn't seem to cater for contracts.

@RLittlesII
Copy link
Member

There was a way to do named instances in Simple Injector. I can't remember how off the top. The library considers the concern a bad practice, but it still allows you to do it. I'll see if I can pull an example. But your right it isn't idea.

@glennawatson
Copy link
Contributor Author

glennawatson commented Aug 3, 2019 via email

@schnerring
Copy link
Contributor

schnerring commented Jan 18, 2021

Since using ReactiveUI + Splat, I find myself using the follwing (recommended) pattern over and over again:

public OAuthSettings(IConfiguration configuration = null)
{
    configuration ??= Locator.Current.GetService<IConfiguration>() ??
                      throw new ArgumentNullException(nameof(configuration));
    // ...
}

This bugs me because it's because of error-prone code duplication and the fact that this Splat pattern "bleeds" into every class using DI. It reminds me so much of MEF's [ImportingConstructor] attributes everywhere. Using SimpleInjector makes above pattern obsolete.

Using Splat.SimpleInjector makes Sextant unusable because RegisterParameterStackService makes a registration after resolving a service. From Stack Overflow:

Simple Injector locks the container after first use, and it does not allow the container to be unlocked. This behaviour is very deliberate and it is explained in detail here why making registrations after having resolved is a bad idea. Making registrations after you resolve is an anti-pattern that is sometimes referred to as Register Resolve Register (not to confuse with Register Resolve Release).

I wonder, is this just an issue with Sextant (should I open an issue?) or can the register-resolve-register pattern also be found in other ReactiveUI components?

Also, I find it hard to understand how SimpleInjectorInitializer works. Intuitively I tried registering to the SimpleInjector container using Splat's API. It took me a while to find out that SimpleInjectorDependencyResolver fails silently:

public void Register(Func<object> factory, Type serviceType, string contract = null)
{
    // The function does nothing because there should be no registration called on this object.
    // Anyway, Locator.SetLocator performs some unnecessary registrations.
}

I tried looking into SetLocator but I can't find these unnecessary registrations. Was this changed? If this function "shouldn't be called", a NotImplementedException, telling the developer to use the initializer instead, would be very much appreciated. I did this because I use the UseSerilogFullLogger() extension method and tried calling it after container.UseSimpleInjectorDependencyResolver(initializer).

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

No branches or pull requests

4 participants