-
-
Notifications
You must be signed in to change notification settings - Fork 126
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
Added custom-dependency-inversion.md (#161) #161
Conversation
public abstract class MyDependencyResolver : IMutableDependencyResolver | ||
{ | ||
public abstract object GetService(Type serviceType, string contract = null); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm.. may want to provide a reference implementation to having a Dictionary even of items etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe we should provide a production-ready custom dependency resolver implementation for a trendy DI container, like SimpleInjector or DryIoc that are loved by mobile developers for performance 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically for the GetService
method or for the entire implementation? I struggled with this primarily because I don't want to affect peoples design decisions too much. I could technically just rip the guts from ModernDependencyResolver that way if they copy/paste from the website, they will at least be baseline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@worldbeater I actually wrote this last night while I was implementing this for Autofac. I could provide that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RLittlesII it'd be great! Autofac is an extremely popular DI container so we may attract more people by saying RxUI supports it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to an implementation of an AutofacDependencyResolver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to an implementation of an AutofacDependencyResolver
Could we have a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest we put one popular DI engine in for the moment, then expand as people add more. Eg SimpleInjector etc.
protected IMyContainer Container; | ||
|
||
public MyDepedencyResolver(IMyContainer container) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially you might want to talk about InitializeReactiveUI which will re-register all the IPlatformsRegister stuff into the new container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding something here is fine. There needs to be some verbiage on the site about those methods and what they do, which I think is beyond the scope of this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I am going to go into a full run down how the internals of that work.
@glennawatson @worldbeater Updated with an Autofac implementation. |
Made requested changes. Please review again.
|
||
```csharp | ||
public class AutofacDependencyResolver : IMutableDependencyResolver | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth fixing spacing issues :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@worldbeater fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't find the "Approve" button, but I approve. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve button is under the top right in the "Changes" screen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under Review Changes
```csharp | ||
var resolver = new AutofacDependencyResolver(container); | ||
resolver.InitializeSplat(); | ||
resolver.InitializeReactiveUI(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment something like
// These Initialize methods will add to your new container dependencies required by ReactiveUI
// and these must be present if you override with your own Locator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
Resolves #134