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

Dependency Injection in Views #946

Merged
merged 1 commit into from
Feb 9, 2016
Merged

Dependency Injection in Views #946

merged 1 commit into from
Feb 9, 2016

Conversation

ardalis
Copy link
Contributor

@ardalis ardalis commented Feb 2, 2016

Fixes #128


A Simple Example
----------------
With the above warning in mind, you can inject a service into a view using the ``@inject`` directive, as shown below:
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see some more descriptive text on the syntax of @inject. It's effectively a property definition, where the first parameter is the type of the property and the second parameter is the property name. @using statements apply when specifying the property type. The property will then get populated from DI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure it's worth calling out the warning here. If you emphasize it enough in the intro it'll stand on its own.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "With the above warning in mind,"

Copy link
Contributor

Choose a reason for hiding this comment

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

The code below (from the http://docs.asp.net/projects/mvc/en/latest/views/view-components.html sample), shows injecting a service into a view using the @Inject directive:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to include syntax and remove "with the above warning".

@danroth27
Copy link
Member

@NTaylorMullen Please review.


`View sample files <https://github.com/aspnet/Docs/tree/1.0.0-rc1/mvc/views/dependency-injection/sample>`_

.. warning:: When applying this functionality, ensure you maintain `separation of concerns <http://deviq.com/separation-of-concerns>`_ between your controllers and views. Most of the data your views display should be passed in from the controller. This technique is recommended only for view-specific data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ehh, not sure this warrants a warning. Weave this into the above introduction on how dependency injection can be good and bad if not used correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd write it as
We recommend you maintain SoC between .. Generally the data displayed in views should be passed ...
what is This technique? Write that as
Injecting a service into a view is recommended for view-specific data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworded; removed warning block.

@NTaylorMullen
Copy link
Contributor

Hmm, after looking over the format of this article I'm not sure the format's right. It dives into the linked samples pretty extensively but I was under the assumption this would be more of a reference doc (@danroth27 please verify)? If so I'd envision the layout being:

  • Intro
  • Inject a service into a view
  • Override a previously injected service into a view (HtmlHelper for example).

Each showcasing how to do inject the service and the outcome but not diving too much into detail on what's going on in the sample.


public List<State> ListStates()
{
// the abbreviated states of america
Copy link
Contributor

Choose a reason for hiding this comment

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

Some abbreviated states in the USA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@danroth27
Copy link
Member

@NTaylorMullen This topic wasn't intended to be an @inject reference. It was intended to be scenario based, so I think the amount of example code is ok.

@ardalis Please do add a section about being able to override a previously injected service. For example you can override @Html to provide your own HTML helpers

@ardalis
Copy link
Contributor Author

ardalis commented Feb 5, 2016

Are you sure overriding works? I have this:
@Inject MyHtmlHelper Html
in my sample and it says:
"Member with the same name is already declared"
@NTaylorMullen @danroth27

@NTaylorMullen
Copy link
Contributor

Hey @ardalis what bits are you using? I tried this locally with rc1-update1 bits and all seems well:

public class FooHelper
{
    public int Value { get; set; }
}

image

@ardalis
Copy link
Contributor Author

ardalis commented Feb 7, 2016

image

I'm using rc1-update1 bits as well. Let me know your email and I can send you the project - it's tiny.

@NTaylorMullen
Copy link
Contributor

@ardalis Taylor dot Mullen at microsoft.com

@NTaylorMullen
Copy link
Contributor

Hey @ardalis. Everything works on my box (verified runtime too). My bet is that your tooling is semi-corrupted. /cc @ToddGrun

image

@ardalis
Copy link
Contributor Author

ardalis commented Feb 8, 2016

@NTaylorMullen Thanks. Not sure how to resolve that immediately. I'll write the code I think should work and check it in and you can see if it looks good and wrap up this PR, ok? Thanks!

@NTaylorMullen
Copy link
Contributor

@ardalis Does it work at runtime for you?

@ardalis
Copy link
Contributor Author

ardalis commented Feb 9, 2016

@NTaylorMullen it does now that I remembered to add the helper to DI... Should be all good now. Shipit?


By `Steve Smith`_

ASP.NET MVC 6 supports `dependency injection <https://docs.asp.net/en/latest/fundamentals/dependency-injection.html>`_ into views. This can be useful for view-specific services, such as localization or data required only for populating view elements. You should try to maintain `separation of concerns <http://deviq.com/separation-of-concerns>`_ between your controllers and views. Most of the data your views display should be passed in from the controller; injecting services into views is recommended only for view-specific data.
Copy link
Contributor

Choose a reason for hiding this comment

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

@danroth27 should we be changing names to core (instead of MVC 6)?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Nuke ; injecting services into views is recommended only for view-specific data.. You already mention this in the previous bit.

@Rick-Anderson
Copy link
Contributor

@NTaylorMullen Shouldn't we be using the latest RC2 bits at this late stage. Why have planned obsolescence?

@Rick-Anderson
Copy link
Contributor

@ardalis Just spin up a clean VM with the latest bits, it's the lowest friction approach to a clean environment. I do my development on a VM and have a scheduled task robocopy the code to my publishing VM every minute.

@NTaylorMullen
Copy link
Contributor

@Rick-Anderson RC2 bits have the core renames. If @danroth27 wants to hold off on those then RC1 bits should be fine.

@NTaylorMullen
Copy link
Contributor

:shipit:

@ardalis ardalis merged commit 67e6c6a into master Feb 9, 2016
@ardalis
Copy link
Contributor Author

ardalis commented Feb 9, 2016

Merged.

@danroth27 danroth27 deleted the ardalis/di-views branch February 18, 2016 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants