aspnet / Announcements

Subscribe to this repo to be notified about major changes in ASP.NET Core and Entity Framework Core
Other
1.66k stars 80 forks source link

MVC: Changes to [Activate] in beta-5 #28

Open rynowak opened 9 years ago

rynowak commented 9 years ago

Hi all, I wanted to share with you some of the changes that are coming for beta-5 of MVC and some of the rationale about why we're changing [Activate]

What's changing

We're removing [Activate] totally and simplifying some of the things it was possible to accomplish using it. Where [Activate] was used to access context-objects from the current request, it will be replaced by bespoke attributes to handle the supported cases. We've also trimming down the number of context-objects that are available so that it's clear what is supported. Where [Activate] was used to access items from the HttpContext.RequestServices (DI), use constructor injection in its place.

[FromServices] is still supported in controllers on action parameters, public controller properties and model properties. There are no changes to [FromServices] in beta-5.

There are no changes to @inject for beta-5.

Overall there should be very little impact on MVC code unless you're making very heavy use of POCO-style controllers and view components.

POCO Controller Sample

Before:

public class MyController
{
    [Activate]
    public ActionContext ActionContext { get; set; }

    [Activate]
    public HttpContext HttpContext { get; set; }
}

After:

public class MyController
{
    [ActionContext]
    public ActionContext ActionContext { get; set; }

    public HttpContext HttpContext => ActionContext.HttpContext
}

POCO ViewComponent Sample

Before:

public class MyViewComponent
{
    [Activate]
    public IRespository Repository { get; set; }

    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyViewComponent
{
    public MyViewComponent(IRepository repository)
    {
        Repository = repository;
    }

    public IRespository Repository { get; }

    [ViewComponentContext]
    public ViewComponentContext ViewComponentContext { get; set; }
}

TagHelper Sample

Before:

public class MyTagHelper : TagHelper
{
    [HtmlAttributeNotBound]
    [Activate]
    public IHtmlEncoder Encoder { get; set; }

    [HtmlAttributeNotBound]
    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyTagHelper : TagHelper
{
    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

    public IHtmlEncoder Encoder { get; }

    [HtmlAttributeNotBound]
    [ViewContext]
    public ViewContext ViewContext { get; set; }
}

About [Activate]

We originally created [Activate] to solve an implementation problem - how do you get ActionContext and other things into a controller without requiring the user to do this?

public class MyController : Controller
{
    public MyController(ActionContext context)
        : base(context)
    { }
}

We want writing controllers to have the minimum of boilerplate possible, so we created an attribute ([Activate]) that would tell the controller factory to inject certain known types. When we introduced new concepts (TagHelpers, ViewComponents), [Activate] came along for the ride to get the required context into those types.

Once we added the ability to inject arbitrary types from DI with [Activate], we realized that we'd built a secondary DI system that was specific to MVC and that only applied to a few concepts in the framework.

Our goal had always been to play nicely with 3rd party DI so that we could enable the use of the advanced concepts that dedicated DI systems can provide. Each piece of functionality we layer on top is another puzzle to solve for integration with a system like Ninject or Autofac (or your container of choice). We'd rather reset this and build towards a good integrated experience.

We've heard plenty of feedback on aspnet/Mvc#2151 asking us to embrace a merger between DI concepts and [Activate]. Our decision for now is that if there's a significant demand for more powerful DI features, then we should prioritize hooking up existing 3rd party DI solutions to fill the gap. We've done some enabling work for using DI to construct controllers, we might need to go further.

About [FromServices]

This is something we invented when revisiting model binding, and really grew independently of [Activate]. We've heard feedback in the context of the [Activate]/@inject discussion that [FromServices] is a bit awkward and oddball. We're still listening, but have chosen to do nothing for this release.

About @inject

We really feel like @inject is doing what we want it to from a usability standpoint. We heard the feedback loud and clear that everyone is happy with it, so it's staying put.

We don't feel like the many of the concerns we have about [Activate] can be applied to Razor code. Razor has a unique programming style and wouldn't support configuring many of the advanced DI features anyway (since you're not declaring constructors and properties).

The Future

We're still listening to feedback about this even though we're moving in a different direction. We want DI to be part of the Asp.Net platform as a whole, and don't want MVC to have a 'quirks mode' for certain framework pieces. Nothing here is set in stone, this is an iteration and it's still a beta release.

We'd love to see code samples of things you think are too hard or too verbose to do with these changes.

justinvp commented 9 years ago

Minor typo above:

    public MyTagHelper(IHtmlEncoder encoder)
    {
        Repository = repository;
    }

Should be:

    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }
rynowak commented 9 years ago

Fixed, thanks :grinning:

RickyLin commented 9 years ago

"The are no changes to @inject for beta-5" or "There are no changes to @inject for beta-5"?

rynowak commented 9 years ago

I clearly need a better proofreader.

There are no changes to @inject for beta-5

Correct

glen-84 commented 9 years ago

Like I said here, it reads as very repetitive – ViewContext written three times.

If the contexts are supplied from outside of the DI system, why not continue to use the single attribute, but limit its use to supplying framework contexts only? I don't think that the term Activate has any ties to DI, but if it does, you could use something else, like [Provision].

If it is technically possible to insert and retrieve these objects from the DI container, then this attribute is not required at all, and a single [Inject] attribute could be used everywhere (I assume it would be possible to replace this attribute with the attribute supplied with one's DI framework of choice?).

michaldudak commented 9 years ago

Is there any reason why every dependency couldn't just be injected into a constructor?

laurentkempe commented 9 years ago

@michaldudak I was about to ask exactly the same question! To me it shows clearly the dependencies.

kevinkuszyk commented 9 years ago

+1 for using just using constructor injection and dropping the attributes.

m0sa commented 9 years ago

+1 @kevinkuszyk

ghost commented 9 years ago

+1 @kevinkuszyk

ghost commented 9 years ago

+1 @kevinkuszyk

blobor commented 9 years ago

+1 @kevinkuszyk

marcodiniz commented 9 years ago

+1 for using just using constructor injection and dropping the attributes.[2]

or i just didin´t what is the advantage

2015-05-22 8:56 GMT-03:00 Kevin Kuszyk notifications@github.com:

+1 for using just using constructor injection and dropping the attributes.

— Reply to this email directly or view it on GitHub https://github.com/aspnet/Announcements/issues/28#issuecomment-104642798 .

henkmollema commented 9 years ago

Was wondering the same thing as @michaldudak. Especially this 'hybrid-mode' might look odd:

public class MyTagHelper : TagHelper
{
    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

    public IHtmlEncoder Encoder { get; }

    [HtmlAttributeNotBound]
    [ViewContext]
    public ViewContext ViewContext { get; set; }
}
glen-84 commented 9 years ago

Isn't it because the context(s) are not "primary" dependencies? In the controller example, you'd rather use constructor injection for your services and/or DbContexts, and get the framework stuff out of the way.

It's this:

public class MyController
{
    public ActionContext ActionContext { get; set; }

    public MyController(MyService service, ActionContext context) { /* ... */ }
}

Versus this:

public class MyController
{
    [ActionContext] or [Inject]
    public ActionContext ActionContext { get; set; }

    public MyController(MyService service) { /* ... */ }
}

Also, if you use the former, you'd expect both arguments to come from the DI container, but only one of them actually is (as far as I understand it).

This would be a lot simpler if the contexts could come from the DI container. That way, the developer can choose how he/she wants them injected.

CoreyKaylor commented 9 years ago

If it's a dependency necessary for the component to work it should come through the constructor in my opinion. This categorization of dependencies of ctor vs. property injection just adds to confusion down the road.

This is somewhat related to this issue regarding per request services. https://github.com/aspnet/DependencyInjection/issues/237

Eilon commented 9 years ago

The items that are provider by [ViewContext] are not services, and don't come from the DI container, so that's why they are a separate attribute. Your Ninject, Autofact, etc. has no idea what ViewData is. Part of the issue before was conflating "all injected things" as one big bucket, where in reality just because something "comes from somewhere else" doesn't mean it's DI.

terryt2000 commented 9 years ago

@Eilon, that's what I was thinking, just not able to articulate it...they are different concerns managed by different "containers"

michaldudak commented 9 years ago

IMO it would be great if the programmer won't have to think if a particular thing should be injected into a constructor or activated some other way. But I do understand this may be impossible/too hard to implement. If, however, we have to use attributes, I'll vote against specific ones like ActionContextAttribute as having the same identifier repeated three times (with three different meanings) just doesn't look right to me.

mdekrey commented 9 years ago

I think this is why many of the existing DI frameworks (Ninject, Unity, Autofac) support adding additional bindings to child scopes, rather than just registering them all up front. A developer shouldn't be concerned with where a value is coming from when setting up an object; isn't that one of the major pillars of DI?

Also, +1 for using constructor injection for everything, or adding attribute injection for if you really don't want child objects to have to worry about it.

Eilon commented 9 years ago

@michaldudak it's not quite the same identifier three times, though it certainly appears that way. (BTW I'm not saying it isn't confusing!) The attribute is really ActionContextAttribute. The type of the property is indeed ActionContext. And the property name is whatever you want, but logically it's normally called ActionContext as well. So while it does appear verbose, there's a fairly good reason for each of those.

@mdekrey the key difference here is that we really, really, really don't want to conflate DI with "other stuff." It's certainly possible for us to do that with some trickery, but that would make things so much harder to debug, so much harder to figure out what's going on, etc.

Please remember that most developers won't see this, and those that do, won't see it very often. These attributes are mostly needed when extending MVC 6, or when using POCO objects (e.g. a POCO Controller, POCO ViewComponent). I believe that relatively few developers will use these scenarios.

johncrn commented 9 years ago

+1 for DI all the things.

Things will get out of hand down the track if you stick with multiple dependency injection strategies. The lines will be blurred between self implemented and framework dependencies and the poor sod trying to put together their solution will be most confused.

@Eilon Could you please explain a little more the difference as you see it between DI and "other stuff"? From the examples shown above the only distinction I can see is that the DI'd services may pertain to a local solution and the attribute injected services are part of the framework implementation. On the face of it I can't see why you'd want a distinction between the two.

rajajhansi commented 9 years ago

If I only care about using Controllers to build Web APIs, not use Razor, ViewComponents and TagHelpers, do I need to worry about any of these attributes? If I only need access to HttpContext, can I not inject IHttpContextAccessor from Hosting? If someone uses POCO for a controller or ViewComponent, the idea is to keep them in a separate class library without any dependencies to frameworks like MVC so they can reuse them in other frameworks like Nancy etc. Adding an attribute pollutes that POCO anyways by adding dependency to a particular framework. +1 for just sticking to one attribute which should inject one context that provides access to other contexts as properties for those who want to write POCOs (for Controllers, ViewComponents, TagHelpers) yet need access to MVC framework related contexts. Adding one attribute for each context is not good. +1 for constructor injection only if these contexts can be mocked to run unit tests on controllers. Otherwise, they will grow up like the old ASP.NET HttpContext.

atrauzzi commented 9 years ago

tldr: :+1: to DI all the things. And more.

Few things I'm noticing that would be concerning for anyone who has to both grock and author ASP.NET code (perhaps not always in that order). Most importantly: Having any more than one annotation for any type of injection just seems plain confusing.

I'm going to pick up on @rynowak's handy examples though and refactor them to what I feel would make the most sense. I'll leave the original "befores", and substitute in my own "afters".

POCO Controller Sample

Before:

public class MyController
{
    [Activate]
    public ActionContext ActionContext { get; set; }

    [Activate]
    public HttpContext HttpContext { get; set; }
}

After:

public class MyController
{
    [Inject]
    public ActionContext ActionContext { get; set; }

    // Doing this would be down to taste, good either way.
    public HttpContext HttpContext => ActionContext.HttpContext
}

POCO ViewComponent Sample

Before:

public class MyViewComponent
{
    [Activate]
    public IRepository Repository { get; set; }

    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyViewComponent
{
    // Constructor injection is fine.
    public MyViewComponent(IRepository repository)
    {
        Repository = repository;
    }

    // Property injection is fine too, mix it up! 
    [Inject]
    public IRepository Repository { get; }

    [Inject]
    public ViewComponentContext ViewComponentContext { get; set; }
}

TagHelper Sample

Before:

public class MyTagHelper : TagHelper
{
    [HtmlAttributeNotBound]
    [Activate]
    public IHtmlEncoder Encoder { get; set; }

    [HtmlAttributeNotBound]
    [Activate]
    public ViewContext ViewContext { get; set; }
}

After:

public class MyTagHelper : TagHelper
{
    public MyTagHelper(IHtmlEncoder encoder)
    {
        Encoder = encoder;
    }

    // Again, like before, constructor or attribute, developer's choice!
    [Inject]
    public IHtmlEncoder Encoder { get; }

    [HtmlAttributeNotBound]
    [Inject]
    public ViewContext ViewContext { get; set; }
}

Lots of this obviously comes down to removing the pseudo-DI system, DRY, encapsulation and avoiding boilerplate. Based on my examples - especially when you only use constructor or property injection - you will see that they represent a minimum of code.

This is more than just code-golfing.

Improving the naming and forcing first-class mechanisms to apply throughout the framework are a matter of developer-joy. When 99% of questions can be answered with "just inject it", you create a more scrutable system and reduce the amount of magic people have to memorize. This is in contrast to "Did you annotate your ActionContext with [ActionContext]?" Perhaps obvious to the initiated, but the developer was set up to fail in that situation. The framework is forcing them to worry about internals instead of their own code.

For property injection, I think it's important this be available in vanilla ASP.NET and not left up to 3rd party containers. This actually has a much broader impact at a community level than might seem apparent at first. Also, Because ASP.NET is itself just a bunch of packages, the core classes become easier to use with a minimum of project-level code.
Again, not code-golfing so much as expecting developers to be fluent in your internal lifecycle: "Did you call the parent constructor?!" - things like this just turn people off and show poorly in books and demos.


The last thing that I would hate to see fall off the wagon is method injection. If we look at where other languages are heading, they are learning that controllers tend to just be procedural as opposed to object oriented. In Scala, they can even just be static objects. This convention may only need to apply when the framework is calling project code, but confers some serious savings in boilerplate and even performance by only initializing what's needed for a specific run on the system:

public class MyController
{
    public ActionResponse StoreUser(UserRegistrationService userRegistrationService)
    {
    }

    public ActionResponse ShowUser(UserDirectoryService userDirectoryService)
    {
    }
}

What stands out here is that when StoreUser is called, no instances of UserDirectoryService are obtained (and thus, instantiated) from the container. Contrast this to constructor or property injection which are a package deal. Controllers are just one of a handful of scenarios where method injection from the framework make sense. I certainly wouldn't require that it be available everywhere, but is really awesome to use.


Okay, there was a lot here, so I'm going to summarize:

It sounds like some of this would require that the default ASP.NET dependency injection system expand a bit, but I see very compelling reasons that will benefit all sides. The community will always have to code to the lowest common denominator, so that bar really ought to be set high out of the box.

atrauzzi commented 9 years ago

@Eilon - If it's okay, I wanted to sound off on some of your responses...

Your first to @michaldudak admits that it's both "confusing" and "with good reason". I'm interested in knowing a bit more as to why. Especially when compared to just a generic injection mechanic.

For your response to @mdekrey, the - living - message here is that we shouldn't have a concept of "DI" vs. "other stuff". By my own experience while learning ASP.NET MVC and when reading scaffold code: These one-off quasi-injections are way more confusing than if you told me "it's just getting injected" with constructor or attribute injection.

I wouldn't argue that most folks stick with defaults, but I also wouldn't rule out putting in strong fundamentals because of that. Up until now, the framework has relied on the base controller because there hasn't been a convenient injection system like what's being offered out of the box with ASP.NET 5. This story stands a chance of changing after release.

glen-84 commented 9 years ago

@atrauzzi,

I agree with almost everything that you said. However:

  1. @Eilon has mentioned that this would require some "trickery", which I read as "hacking". This means that there are three real options:

    • Hack the current code in such a way that everything is available via DI. (I don't think that anyone would like this)
    • Refactor the code entirely to support this. (It's quite possible that it is too late for such large changes)
    • Go with an interim solution that uses a single attribute to flag framework-injected dependencies.

    I think that most people would agree that the second option is ideal, but may not be possible at this stage. Option 3 might be a good compromise, until further work can be done.

  2. [Inject] can still be added for property injection. A decision will need to be made WRT how controller method dependencies are handled by default (i.e. without attributes). If I'm not mistaken, currently, the model binder will try to bind from all the various request properties by default. This will mean that you will need to use an attribute to do method injection as well ([Inject]), which seems fair to me.

@Eilon, I strongly disagree with the justification that "most developers won't see this". Usage of POCOs was made possible for a reason, and those who make use of them should have a pleasant experience as well.

davidfowl commented 9 years ago

It sounds like some of this would require that the default ASP.NET dependency injection system expand a bit, but I see very compelling reasons that will benefit all sides. The community will always have to code to the lowest common denominator, so that bar really ought to be set high out of the box.

People building middleware and libraries that depend on the DI abstractions need to code against the same base behaviors. The entire point of the library was to have a good enough implementation so that out of the box you don't have to pick a DI container. If you did choose one, it was intentional and because it had a wider feature set than the built in one. We don't want to support everything that Autofac/Ninject/StructureMap/Grace etc supports but we want to rely on features that most DI containers have (and I'm not sure property injection is one of those features we want to promote :smile: ).

Adding [Activate] only in MVC was even more confusing IMO because people started to ask for it in more places (like middleware), and it wasn't general purpose.

mrtaikandi commented 9 years ago

+1 @davidfowl

atrauzzi commented 9 years ago

@glen-84

Re 2: I'm not going to pretend to be completely fluent with the amount of work involved. But I do believe in never saying never :) I feel bad for not advocating for these changes sooner. But I think what we're talking about here would have a profound impact on how people view ASP.NET 5. Coming from other frameworks that get DI right and make it easy, what I see in controllers right now is unfortunate. I'm not here to flog decisions of the past, but this thread is about some of the most frequently used pieces of the framework - it has to be done right, or it will fail peoples' smell tests.

Oh, and I'm totally flexible if [Inject] has to be used for method injection. I understand what's going on there and it might even be nicer to have that "documentation" in place so that people can better separate what is coming from where.


@davidfowl - I think what you're saying supports my points! :D

"People building middleware and libraries that depend on the DI abstractions need to code against the same base behaviors."

Yes, this is exactly what I'm saying. What's I'm seeing right now is that ASP.NET will ship interfaces for DI, but only partially implement them in it's own. So if a package author wants the best reach, they will have to code to the most pervasive implementation of that interface. Which obviously would be the default ASP.NET one.

(As a small aside: I saw a library in nuget recently that had the gall to depend on a DI framework. That would be devastating to the package ecosystem if it became commonplace.)

I've offered up at least one good reason why property injection is desirable, if it's left out, ASP.NET will have cut too close to the bone.

For [Activate], thankfully ASP.NET 5 can change this and make available a much less surprising mechanic that can be used everywhere. One concept to rule them all!

glen-84 commented 9 years ago

@atrauzzi It's ASP.NET 5, and MVC 6. :smile:

atrauzzi commented 9 years ago

Agh, sorry. I think my brain is taking a break today.

mdekrey commented 9 years ago

After having implemented the child scoped-services behavior (see my commit that gets mentioned above), I'm torn. The branch does handle adding services to a child scope for the default container, but I did not completely extend it to Ninject and Autofac. (I wrote tests to show that those are failing, too.) It does not introduce any breaking changes for existing functionality that I could find from the tests, but would allow creating a child scope to, say, register an ActionContext for injection. I think this is a good start for what @glen-84 was making for his point 1 option 2.

However, I completely understand the point @davidfowl and @Eilon are making, and agree; as developers of our own stand-alone projects, when we are building a project that requires advanced features, we can break down that wall and use the DI component that best provides them.

On the other hand, the fact that ASP.NET 5 needs to implement their own property injection and fake a child container using [ActionContext] [ViewContext], etc., indicates to me that the feature set provided is too narrow for the DI abstraction as it stands. Package developers (such as the MVC team) have a different story they need to approach: make it easy for the consuming developers making stand-alone projects. There are many developers and companies who do just this; I would argue that while this isn't "most" developers, it is many developers who will be affected.

Instead of talking about what features to include or not to include in the DI container, I'd actually rather see hooks that we can tie into the DI build process to add these features. Want to add property injection? Add to a theoretical AfterBuild hook, or include this package that does it for you. (I see now that the MVC team built the FromServicesAttribute into model binding. That's pretty slick! I don't think it would go away necessarily.) Want to add child scoped-services? That'd probably be a bit more work, but there could still probably be an additional package depending on your DI implementation that does it for you. Right now, though, I don't see a way that we can do that without relying on a specific underlying system, which breaks the use case for package developers, who will likely need these features the most.


@atrauzzi - I agree with most of your points, too, but the hang-up for me on method injection is this: how would those get called? I fully believe that, if we could nail down the syntax, a package could be provided that supplies that functionality already and be DI container independent. Perhaps there's a different thread to discuss this?

atrauzzi commented 9 years ago

My thinking for method injection was that certain dispatch flows in the framework are container aware and would attempt to resolve the parameters prior to calling.

Not opposed to better ideas or improvements, but it's really a convenience to encapsulate at a method level rather than class.

glen-84 commented 9 years ago

I see now that the MVC team built the FromServicesAttribute into model binding. That's pretty slick!

See the last paragraph in my comment here.

... but the hang-up for me on method injection is this: how would those get called?

The framework controls the execution of controller methods, so it would just look for [Inject] attributes, and then request those types from the services container. I'm probably forgetting something though.

jnnwnk commented 9 years ago

I am getting errors in intellisens saying type or namespace "ActivateAttribute" is not available in Microsoft.AspNet.Mvc. I never used the attribute. The error refere to my all my layout files and they even appear without content in those files. This started after updating SDK to beta5. The app is running fine, but seems confusing. Has anybody an idea about that?

Bartmax commented 9 years ago

@janna173 I was about to ask the same. In my case is on _ViewImports.cshtml

developer1998 commented 9 years ago

@janna173 @Bartmax You can safely ignore them, I believe. It is the Visual Studio error not the framework. The rename did not synch with Visual Studio. You will notice that when you close that view and run run your project no errors and everything works as it should.

ielcoro commented 9 years ago

:+1: to constructor injection, resolving everything from the same container, and [Inject] attribute should be there for extremely special cases.

Having two different concepts for handling dependencies is clearly confusing.

brockallen commented 9 years ago

Related -- do we have a way to augment the DI system for per-request dependencies? IOW, something comparable to IHttpContextAccessor but for custom types?

Garciat commented 9 years ago

@brockallen Scoped services, maybe? They are (lazily) instantiated and disposed once per request.

brockallen commented 9 years ago

Right, but MVC itself needs to make the ActionContext in the middle of the request then make it available for injection. Scopes services aren't quite the same since it's configured 'statically'. I'd like a way to inject an instance at runtime for DI later in the request pipeline (like MVC needs to do).

mdekrey commented 9 years ago

I agree, @brockallen. This is called "Child Containers" and is available in most DI containers, but not available for Microsoft.Framework.DependencyInjection. I'm looking to see if there's a way to make an extension...

herecydev commented 9 years ago

@eashi You just linked this thread...

eashi commented 9 years ago

I deleted the comment to remove confusion (I hope I didn't make it worse :P) thanks @herecydev

NTaylorMullen commented 9 years ago

Note: If you're using the latest Mvc packages you may encounter the following errors: image

This is due to VS code generating [Activate] for some pieces of Razor code (activate no longer exists). If you run your application you'll see that the errors don't impact anything.

Workaround to avoid errors at design time:

Add the following class/namespace combo anywhere in your project:


namespace Microsoft.AspNet.Mvc
{
    public class ActivateAttribute : Attribute
    {
    }
}
developer1998 commented 9 years ago

You also have other issues, declarations in _ViewImports.cshtml are not picked up on design time. So my views are flagging many errors such as: The name 'someClass' does not exist in the current context The type or namespace name 'AppSettings' could not be found (are you missing a using directive or an assembly reference?)...

I know this will be resolved, just want to bring it up to attention...

Eilon commented 9 years ago

@developer1998 do you know if there's an issue on this logged in the Razor repo? (It might end up going to the Tooling repo but we could start out in the Razor repo.)

developer1998 commented 9 years ago

@Eilon No idea, but I can create it on the Razor repo.

Duke360 commented 9 years ago

@atrauzzi +1 plus i'm interested to understeand why certain things are not "service" enough to the ASP.NET team at the point that they require their own DI mechanism, that's not sarcastic, i'm really interested in understand

BrianVallelunga commented 9 years ago

Constructor injection makes the most logical sense from the developer's point of view, but I understand the complexity involved in injecting things that aren't DI services.

If you can't do constructor injection, here are three other options that I just thought up:

1) Rename [Activate] to something like [MvcContext] or [FromMvc] so it's clear that it is only for MVC . Having multiple different attributes for the same action of injecting things seems strange.

2) Use magic. If there are only a few types that this applies to, then just auto-inject them when the public properties exist.

3) Use interfaces. After DI does its thing with the constructor parameters, the framework can see if a class implements an interface and sets the properties as needed.

// Defined in framework
public interface IRequireViewComponentContext
{
    ViewComponentContext ViewComponentContext { get; set; }
}

public class MyViewComponent, IRequireViewComponentContext
{
    public MyViewComponent(IRepository repository)
    {
        Repository = repository;
    }

    public IRespository Repository { get; }

    public ViewComponentContext ViewComponentContext { get; set; }
}