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

Update View and ViewModel mocks for SimpleInjector tests. #239

Conversation

chuuddo
Copy link

@chuuddo chuuddo commented Jan 22, 2019

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This change add one binding to mocks, that start RxApp initialization.

What is the current behavior? (You can also link to an open issue here)

#233

What is the new behavior (if this is a feature change)?

What might this PR break?

Breaks all current tests in Splat.SimpleInjector.Tests project.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

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)

@dnfclas
Copy link

dnfclas commented Jan 22, 2019

CLA assistant check
All CLA requirements met.

@glennawatson
Copy link
Contributor

Closing. #240 covers the bug issue.

Pull Requests are normally for functionality or bug fixes you want to add to a project, issues are for bugs or problems. So just moving to the issue referencing your code changes.

@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants