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

(wip) Update custom-dependency-inversion.md #255

Closed
wants to merge 5 commits into from
Closed

Conversation

cabauman
Copy link
Contributor

  • Do not call InitializeSplat and InitializeReactiveUI because that will duplicate all Splat and ReactiveUI registrations.
  • Add sample implementation of using FuncDependencyResolver with NInject
  • Update implementation of IMutableDependencyResolver because two new interface members have been added.

I'm not familiar with Autofac, so I just put placeholders for the unregister methods. Can someone add implementations?

- Do not call InitializeSplat and InitializeReactiveUI because that will duplicate all Splat and ReactiveUI registrations.
- Add sample implementation of using FuncDependencyResolver with NInject
- Update implementation of IMutableDependencyResolver because two new interface members have been added.
@glennawatson
Copy link
Contributor

Autofac doesn't allow Unregister per say so it's fine to just leave the exception there. That was what I intended in containers where it wasn't supported.

@cabauman
Copy link
Contributor Author

Thanks for clarifying @glennawatson

@RLittlesII
Copy link
Member

RLittlesII commented Jan 19, 2019

I think this relates to #134
If our plan is to add an example per platform, I think we should have a main page that explains the concept and then individual pages where each container implementation is shown in isolation. We wouldn't want someone to copy and paste all of the code on the page, not realizing they are different implementations for different containers.

Locator.Current = resolver;
```

From this point on calls `Locator.Current` will go against your custom implementation!

## Bonus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of calling this a bonus, which implies that it is an extra added bit to the auto fac code above. We should either move this to another page, or explicitly call out using a header tag that this is a different container.

@worldbeater
Copy link
Contributor

Not sure, but maybe worth using the <details> tag to provide expand/collapse functionality for different ioc-container samples. In the beginning of the page we may describe what steps should a developer follow when creating a custom dependency resolver (implement IMutableDependencyResolver or use FuncDependencyResolver), and then show a list of collapsed buttons containing resolver implementations for most popular DI containers (Autofac, NInject, maybe SimpleInjector). The details-summary tags work like this, we use them in our getting-started guide:

<details>
  <summary>Expand-collapse title</summary>
  Collapsed content, can contain markup.
</details>

@RLittlesII
Copy link
Member

Yeah. The suggestion for multiple pages is based on the current site paradigm and that it would make it easy to select a particular container if we explicity put it in their face. I am less concerned about how it is separated (pages, details, expanding buttons, hidden iframes), and more concerned about it being clearly marked that a specific code sample belongs to a specific container.

@glennawatson
Copy link
Contributor

Once someone does reactiveui/splat#233 this documentation will change drastically, so could be worthwhile keeping it on one page.

@glennawatson
Copy link
Contributor

Going to close this PR, thanks for the work @cabauman -- given now that @RLittlesII has put most of the major DI engines into Splat I don't think this one is as needed anymore.

@glennawatson glennawatson deleted the cabauman-patch-1 branch March 2, 2019 21:42
@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.

4 participants