Moving IViewEngine to MvcOptions #726
Conversation
|
First draft to vet design |
| /// <param name="descriptors">A list of ViewEngineDescriptors</param> | ||
| /// <param name="viewEngineType">Type representing an <see cref="IViewEngine"/>.</param> | ||
| /// <returns>ViewEngineDescriptor representing the added instance.</returns> | ||
| public static ViewEngineDescriptor Add([NotNull] this IList<ViewEngineDescriptor> descriptors, |
There was a problem hiding this comment.
Should we have a generic overload here?
There was a problem hiding this comment.
What do you mean? Nevermind figured it out. It just seems like more API surface for a very limited use API.
| _viewEngines = _viewEnginesProvider.ViewEngines; | ||
| } | ||
|
|
||
| return _viewEngines; |
There was a problem hiding this comment.
What does lazy-init buy us here? This service is only instantiated when we're actually going to execute a view right?
There was a problem hiding this comment.
We settled on this pattern with ModelBinders. I thought it made sense to keep it consistent.
But yes, in this case we don't benefit in any particular way.
|
Rebased and updated PR. |
|
|
once yishai is happy| namespace Microsoft.AspNet.Mvc.Rendering | ||
| { | ||
| public class CompositeViewEngineTest | ||
| { |
There was a problem hiding this comment.
Missing a few test cases:
- When there is a single view engine
- When the first view engine returns found
|
|
|
Fixed in #770 |
No description provided.