Runtime: [Proposal]: .NET DI council

[davidfowl]

The idea here is to add some process to how we govern features that go into the DI abstraction by having the authors of dependency injection containers be the gate for new features. The original goal of the abstraction was to support a minimal set of features that most container authors could implement directly or via a small bridge (the initial feature set was designed looking at existing container implementations). Over the last couple of releases, we've had a couple of stalled feature requests because of this:

• https://github.com/aspnet/Extensions/pull/2475
• https://github.com/aspnet/Extensions/pull/536

Currently our stance is that we will not add any more features unless containers that already support the current abstraction continue to work. I'd like to formalize that process a bit more and have container authors that support the MEDIA (Microsoft.Extensons.DependenyInjection.Abstraciton) chime in on what their thoughts are.

Here's my proposal for the initial set of people:

Unity - @ENikS
Autofac - @tillig, @alexmg
LightInject - @seesharper
Lamar, StructureMap - @jeremydmiller
DryIoc - @dadhi
StashBox - @z4kn4fein
Grace - @ipjohnson

Please let me know if I've left somebody out the list here is based on my history of working on this project and having these contributors involved (this it the OG list 😄). This is an open discussion so I don't want to dictate what the process should be (though I have some ideas). Lets try to come up with something concrete based on the following example:

There's a discussion about Func<T>. There's a pull request open that seems to pass all the tests. Lets come up with how we would evaluate and accept/reject features like this.

[tillig]

I'm curious if this may extend beyond containers that implement the conforming container interface. For example, if there is a hook for a non-conforming container to inject things into an ASP.NET Core controller; and the abstraction now declares Func<T> support is valid so ASP.NET Core framework base controller classes somehow start requiring it; then the non-conforming container also needs to support that.

[davidfowl]

This doesn't affect non-conforming containers at all. Non-conforming containers work alongside the default implementation of the DI container.

[seesharper]

@tillig The main difference between an adapter and a non-conforming container is that when using an adapter, everything is resolved through the adapter. That includes all the AspNet.Core infrastructure in addition to "user defined" services. With a non-confirming container, the AspNet Core infrastructure is resolved using the default container (MS.DI) while "your code" is resolved through the non-confirming container. So for instance, if the AspNet Core team starts to rely on new functionality in the abstraction for use in the AspNet Core infrastructure, every adapter needs to handle that.

[tillig]

OK, so if...

• A non-conforming container registers an IControllerActivator that resolves controllers and controller dependencies from that non-conforming container AND
• Base controllers or dependencies provided by the ASP.NET Core framework start requiring Func<T> or start assuming that the weird generic "filtering" thing is going on from dotnet/extensions#536 AND
• The underlying non-conforming container doesn't support that but the default container implementation does, so it's assumed everything does

...then that's not the stuff we'd we're talking about here. Fair enough.

I just remember being somewhat bitten by "we assume registration of services will yield an IEnumerable<T> in the exact same order during resolve time" and that seems like it would have affected both conforming and non-conforming containers. However, if we're not talking about that sort of thing here, then good to go.

[seesharper]

I just remember being somewhat bitten by "we assume registration of services will yield an IEnumerable in the exact same order during resolve time" and that seems like it would have affected both conforming and non-conforming containers. However, if we're not talking about that sort of thing here, then good to go.

yeah, my guess is that AspNet Core infrastructure relies on this behaviour and that is probably why it is part of the specification.
You could say that it will affect non-confirming containers as well if you start out using only MS.DI and then later on switch to a non-confirming container for "your services". Then there is a chance that some behaviour has changed and it might break.
Also be aware that it is not only the AspNet Core infrastructure that is affected by a change/addition to the abstraction. Every library/code that builds upon IServiceCollection will also be affected.

Correct me if I'm wrong (@davidfowl), but I don't think Microsoft is trying to support all possible features, but rather add a few more basic ones that every container supports anyway. Think of it as the netstandardfor containers :)

[tillig]

Sure. I don't mean to ride ASP.NET Core, it's just the most convenient example I can think of at the moment. It seems like from a "required supported features" standpoint it goes both ways:

• From the composition root out through the leaves by way of IServiceProvider resolving IControllerActivator (or other hooks for non-conforming containers).
• From the leaves, testing against the "default services" (based on the default container) and assuming incorrectly that "however the injection happens, it'll just work."

If we're defining a set of common features that all "sources of injection" (be they conforming or non conforming containers) need to support - that netstandard of containers - then it seems like it does affect both conforming and non-conforming containers.

Again, latching onto ASP.NET Core, if a framework-provided base class in a controller class hierarchy requires Func<T>, even the non-conforming container implementing IControllerActivator then needs to support that. That's perhaps a contrived example but the idea is there.

c# public class MyController : HttpClientControllerBase { public MyController(Func<HttpClient> clientFactory) : base(clientFactory){} } public abstract class HttpClientControllerBase { public HttpClientControllerBase(Func<HttpClient> clientFactory) { // Some value-added controller base class from the framework, but which lives // in the class hierarchy of application code. This will come from the non-conforming // container via IControllerActivator, not the IServiceProvider. } }

From a blinders-on standpoint, saying "containers and lifetime scopes should support Func<T>" isn't a huge deal by any means. But it does imply that it isn't just containers that implement IServiceProvider or IServiceScope that will be affected by the decision. It's anything in the system that is responsible for resolving dependencies.

MEDIA.IServiceProvider => MEDIA.IServiceScope => IControllerActivator => need to resolve Func<T>.

[seesharper]

That being said, I welcome this council initiative so that we can discuss the abstraction going forward. At the same time I'd like to point out that it is important that we don't raise the bar too high.

[ipjohnson]

I concur with @seesharper The council should help foster new functionality while still keeping compatibility with existing adapters. Func<T> and constrained generics seem like two worth while features.

@davidfowl is there any president to follow or is this type of thing new ground?

[Tornhoof]

I suggest SimpleInjector's: @dotnetjunkie
He was very involved in the original discussion wrt to conforming containers.

[davidfowl]

@Tornhoof that's fine but I guess I should be more clear what this is. I wanted to make the current process a bit more formal but I didn't intend for this to be a design by committee thing. I want to get the opinions of the DI authors to see if we should allow new features into the abstraction since we currently reject every new feature. I haven't thought through how that would work other than tagging people on issues or PRs.

[Tornhoof]

@davidfowl You asked

Please let me know if I've left somebody out the list here is based on my history of working on this project and having these contributors involved (this it the OG list 😄)

I suggested Steven, that's it. No hidden agenda here.

[ENikS]

I welcome the initiative. The abstraction should evolve and new features should not be stopped.

Unity already implements both of the features in question, so it would be desirable to keep it compatible with existing implementation, otherwise you have my vote.

[davidfowl]

I want to clear up one thing, Microsoft is still going to ultimately decide on what features to include/exclude into the container abstraction. The goal of this is to formalize how we go about evaluating new features and we need to not only consider new container authors that want to implement the abstraction. That's why we should be extremely conservative when looking at new features. For example, Func<T> does not work in the stashbox container (https://github.com/aspnet/Extensions/pull/2475/files#diff-38ee0d32417bbdf7da2b4438bf87deceR8).

[davidfowl]

So an extremely low overhead way to start this. I'm going to make a team on github (if the powers that be allow it) with the people in this group. When new issues or pull requests come up, I'll tag the group to have a discussion about it and we can take it from there.

[danielcweber]

For example, Func does not work in the stashbox container

@davidfowl Not entirely true, it just doesn't pass the assertion that you get a Func<IService> from the container. Instead, you get a Func<ActualService> when ActualService is the registration. Let me see what I can do to make them pass.

[danielcweber]

@davidfowl Changed the skipped tests, they don't have to be skipped any more and StashBox fully supports Func<T>.

[seesharper]

If we take Func<T> as a "simple" example, we need a few more tests to prove that we have a consistent behaviour across all containers. Once we start to talk about deferred service resolution like Func<T>, there are questions like ..What if the service is registered as a scoped service? LightInject closes the Func<T> around the scope from which the graph was requested to ensure that we don't need ambient context such as storing the "current" scope in an AsyncLocal<T>.

[tillig]

We might be getting in the weeds a bit if we dive too deep into _specific features_ in _this particular issue_ since it seems like this was more about the formation of the council using those noted items as examples. If there's a GitHub team, maybe we can have some chats there?

[davidfowl]

Ok great let’s feel this out with the discussion being on Func\

[seesharper]

Just wanted to emphasise that there might be more than meets the eye even when it comes to adding the simplest feature 😊.

[z4kn4fein]

@davidfowl @danielcweber Yeah, I also noticed those broken tests and thanks for pointing out that not the entire functionality is broken, but Stashbox behaves a bit different. However, that was a glitch in the implementation which I already fixed. I'll release the fix with the next version soon, but first I want to finish some other ongoing developments on it. Thanks for adjusting the tests to make Stashbox pass, this fix won't break the current state of them, just fixed the previous.

I also fixed that part recently which caused the open generic related tests to fail, so with the next version, all the skipped tests should pass.

[davidfowl]

I haven't forgotten about this 😄

[jbogard]

Does...does this mean constrained generics might actually make it in? 😍

[eerhardt]

Moving this issue to Future, since this is tracking a Proposal, and nothing needs to be fixed for 5.0.

[ENikS]

@davidfowl

With so many changes to layout of the repositories, could you point me to a suggested way of building and testing latest DI?
Is there easier way of doing it than building entire runtime libraries?

[davidfowl]

@maryamariyan can you assist?

[maryamariyan]

With so many changes to layout of the repositories, could you point me to a suggested way of building and testing latest DI?
Is there easier way of doing it than building entire runtime libraries?

Here's the steps you would need to work on runtime repo:

1) Check build requirements.
2) Check the quick start on how to build the runtime repo libraries.
3) Then you could follow instructions for running tests, or just create a sample console/web application project and add a reference to the Extensions library dll you built, in your case DI and check behavior locally.

HTH.

[ENikS]

Thank you

[ENikS]

@maryamariyan

You were so helpful last time, perhaps you could help again?
I am looking for a way to run included tests inside Visual Studio, is there a guide on doing something like that?

[eerhardt]

@ENikS - check out this document:

https://github.com/dotnet/runtime/blob/master/docs/workflow/testing/visualstudio.md

If that doesn't work or is unclear - can you open an issue so we can update the docs?