Add an application part factory for an assembly that contains controllers and views.

[pranavkm]

Contributes to #29862.

MVC has infrastructure for providing where assets such as controllers or views come from by means of an ApplicationPartFactory. By default, MyApp.Views.dll is stamped with an CompiledRazorAssemblyApplicationPartFactory that can provided Razor Pages and views. This application part combines the behavior of the default part factory (so that it can provide things like controllers) and the views one (so that it can provide views) from the same assembly.

Assemblies in .net6 that are compiled with views and controllers in the same assembly will use this part factory.

[captainsafia]

Seems legit

[javiercn]

Spicy thought, instead of doing this, we could code-generate this class as part of the compiler work and this way we don't need another public type here.

[davidfowl]

I'm confused as to how this get wires up right now.

[captainsafia]

Spicy thought, instead of doing this, we could code-generate this class as part of the compiler work

Hmmm....what's the benefit of doing this?

I'm confused as to how this get wires up right now.

The ConsolidatedAssemblyApplicationPartFactory isn't used anywhere at the moment. Eventually, we'll be including the compiled views in the application assembly, which will be stamped stamped with a ConsolidatedAssembly attribute, and then using this API to look for RazorCompiledItems that are baked into the app assembly, in addition to it's own types.

@pranavkm Correct me if I got any details wrong.

[javiercn]

@captainsafia is spot on.

My suggestion is we don't need a public type, we can just codegen one of these puppies per assembly

[pranavkm]

👍🏽 on @captainsafia's analysis.

@javiercn I'm not sure what code-gening this gets us. Is it to avoid adding a public type?

[javiercn]

Yep

[javiercn]

Also because that way, changes in the mechanics of how it works are isolated to the compiler instead of being a concern shared by the runtime

[javiercn]

I signed-off, but just want you to think about it.

[pranavkm]

how it works are isolated to the compile

The downside is that the compiler would need to be aware of the app's tfm if we have to change the implementation. We're currently not planning to ship the source generator thru a runtime pack. Once we decide to use it, all of the code it generates is driven by RazorLanguageVersion, not by the app's tfm.

[pranavkm]

let me bring this up during API review. I'm kinda partial to just having the type in the framework, but I'll see how other folks feel about this.

[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.

[rynowak]

Spicy question - why do you need this boy VS adding this functionality to CompiledRazorAssemblyApplicationPartFactory? I can't think of a case today where someone has controllers in their existing razor code.

You could also simplify this other way by just building the Razor Page discovery into the default case.

Basically what I'm saying is that on reflection, it seems like this area is overdesigned a bit. Someone (wonder who's to blame) seemed to really be in love with this idea of combining codegen+metadata to configure runtime behaviour when they built this, and maybe it could just all be simpler.

[pranavkm]

Thanks @rynowak for the feedback. It feels like either of those options has slightly higher inherent breaking change risk or work to make work compared to just adding an additional API. Particularly given that this API is a bit tucked away and MVC is a wasteland of fun APIs, this didn't seem like overkill. We'll stick with the new type for now, we have time until 6.0 ship if we start feeling differently about it.