Adding EntryAssembly to AddRazorPages/AddControllers*

[brunolins16]

Background and Motivation

Currently, the default behavior in MVC to load the default ApplicationParts from the entryassembly is rely on the IWebHostEnvironment, however, as reported in the #10611, there are scenarios where the developer wants to just create a new ServiceCollection and configure as needed. This scenario is not supported today because some providers will not be loaded, eg. RazorCompiledItemProvider required in Razor Pages.

The proposal is to add a new overload to all Add* methods available to include Assembly entryAssembly parameter that will be used to load the default ApplicationParts from.

Related:

• AddRazorPages() on new ServiceCollection > ApplicationPartManager missing parts #10611

Proposed API

namespace Microsoft.Extensions.DependencyInjection
{
   public static class MvcServiceCollectionExtensions
   {
+      public static IMvcBuilder AddControllers(this IServiceCollection services, Assembly entryAssembly) 
+      {}

+      public static IMvcBuilder AddControllers(this IServiceCollection services, Assembly entryAssembly, Action<MvcOptions>? configure)
+      {}

+      public static IMvcBuilder AddControllersWithViews(this IServiceCollection services, Assembly entryAssembly) 
+      {}

+      public static IMvcBuilder AddControllersWithViews(this IServiceCollection services, Assembly entryAssembly, Action<MvcOptions>? configure)
+      {}

+      public static IMvcBuilder AddRazorPages(this IServiceCollection services, Assembly entryAssembly) 
+      {}

+      public static IMvcBuilder AddRazorPages(this IServiceCollection services, Assembly entryAssembly, Action<RazorPagesOptions>? configure)
+      {}

   }
}

namespace Microsoft.Extensions.DependencyInjection
{
   public static class MvcCoreServiceCollectionExtensions
   {
+      public static IMvcCoreBuilder AddMvcCore(this IServiceCollection services, Assembly entryAssembly)
+      {}
   }
}

Usage Examples

RazorPages

services.AddRazorPages(typeof(Startup).Assembly);

Controllers

services.AddControllers(typeof(Startup).Assembly);

Controllers with views

services.AddControllersWithViews(typeof(Startup).Assembly);

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[pranavkm]

• Let's make the configure non-nullable in the API (without null-checking it so users can pass in a null if they really want to). e.g.

+ public static IMvcBuilder AddControllers(this IServiceCollection services, Assembly entryAssembly) 
{}

+ public static IMvcBuilder AddControllers(this IServiceCollection services, Assembly entryAssembly, Action<MvcOptions> configure)
+      {}

Given that the API touches the startup code, and the plan is to update the templates we should run it by @davidfowl / @DamianEdwards

[DamianEdwards]

I admit I'm not quite following the full plan here. In addition to the new overloads, are we planning to update the templates to call them? What is passed in for the entryAssembly parameter? If so, why do we think the default/idiom needs changing? Will the existing overload produce a warning or be documented as legacy?

[pranavkm]

are we planning to update the templates to call them

Yup.

What is passed in for the entryAssembly parameter?

We want to pass in the assembly that is the root for scanning controllers / components etc. The actual value to be passed is a bit something to be figured out. We can't quite do GetType().Assembly and the generated top-level statement type is currently unmentionable. There isn't another type in app root namespace to do typeof(WellKnownType).Assembly so 🤷🏽

Will the existing overload produce a warning or be documented as legacy?

We hadn't discussed that, but we could consider doing so. It does mean it'll produce a lot of noise in a whole slew of apps as they are updated.

[DamianEdwards]

I really don't think we should change the default/idiom here (including the template). Exposing that concept so early and for so many seems like a bad fit. For the vast majority of cases in an app, don't we just want this to be the app assembly itself, which is the entry assembly already?

[brunolins16]

I really don't think we should change the default/idiom here (including the template). Exposing that concept so early and for so many seems like a bad fit. For the vast majority of cases in an app, don't we just want this to be the app assembly itself, which is the entry assembly already?

In my opinion, the way MVC detects the entry assembly is obscure and hard to find what is going on when errors similar to the reported in the issue happens, however, I think this behavior is there for so many years (i am not sure if the users complained or not before) that I agree with @DamianEdwards that is better no change the default and instead just add the new overload and we maybe we can include in the docs scenarios where they are a better option.

[dazinator]

I really don't think we should change the default/idiom here (including the template). Exposing that concept so early and for so many seems like a bad fit. For the vast majority of cases in an app, don't we just want this to be the app assembly itself, which is the entry assembly already?

In my opinion, the way MVC detects the entry assembly is obscure and hard to find what is going on when errors similar to the reported in the issue happens, however, I think this behavior is there for so many years (i am not sure if the users complained or not before) that I agree with @DamianEdwards that is better no change the default and instead just add the new overload and we maybe we can include in the docs scenarios where they are a

Makes sense - just wanted to mention that if previous method remains, then it may be prudent to throw an exception when that method cannot locate the entry assembly due to missing prerequisite services, perhaps with the error indicating that you can call the new overload to provide the entry assembly as a param explicitly - that should catch cases like the issue linked?

[brunolins16]

@dazinator After some discussion we decided that this API is not totally correct, and we should not add it, right now. What I will add instead, for now, is just a log message informing that no actions were detected and could be caused by a misconfiguration. Also point to this documentation Share controllers, views, Razor Pages and more with Application Parts that explains the usage of AddApplicationPart and ConfigureApplicationPartManager