[Blazor] Source generator to create the discovery information at compile time.

[javiercn]

#46980

A sample of the generated output can be seen here

[SteveSandersonMS]

The runtime doesn't require component classes to be public (though I know the Razor compiler does). If this is only being used to generate a list of page components with routes then it's fine as-is, but if this is meant to be an exhaustive list of all component types then we should be sure to include nested types and nonpublic types.

[SteveSandersonMS]

Actually I realised that while we can refer to internal component types, we can't refer to nested private component types. So we have to avoid ever assuming we can generate an exhaustive list of component types. If this is only used for routable components then I guess that is fine.

[javiercn]

https://github.com/dotnet/razor/blob/69a87847a50299d3d98c5f70018afec9e49b4162/src/Compiler/Microsoft.CodeAnalysis.Razor/src/ComponentDetectionConventions.cs#L8-L46

The runtime doesn't enforce this constraint because the Razor compiler already does it. We had this discussion a few years ago with Ryan. We want to be in a position where you can define components in a way that is not discoverable or referenceable by the compiler and split the world between Razor syntax and non-Razor syntax.

We gain very little by supporting non-public/nested, etc. components, and we drastically constrain ourselves for the future.

[SteveSandersonMS]

The fact remains that we do already support it. The situation with the Razor compiler is that if you want your component to show up publicly you have to make it public, but for internal use it's just C# classes and you are free to use any C# language features.

[javiercn]

The fact remains that we do already support it. The situation with the Razor compiler is that if you want your component to show up publicly you have to make it public, but for internal use it's just C# classes and you are free to use any C# language features.

I don't think this is the case, I remember the discussion about this when we were preparing the initial release of Blazor and I believe we settled on the components being public. I don't think we added any check on the runtime because it had a runtime cost and we've always meant for the main way to author components to be Razor syntax and the compiler already enforced that behavior.

Even if we think this is not the case, I am ok for any code gen feature like this not working on those scenarios. The outcome is that either your page does not get automatically discovered or the render mode for your internal/private nested component is not taken into account.

If in the future, we want to support those scenarios we can if we choose too by emitting whatever reflection code is necessary to get a reference to those types.

That said, I think we are unnecessarily boxing ourselves here. There is little/none value from having a component being internal/private on apps and there are big drawbacks with regards to code generation.

[SteveSandersonMS]

The part I remember was specifically about parameter visibility, and even then, was about framework-supplied components not application-supplied components: #37840 (comment) Regardless, our conversations from 3 years ago are less important than what actually works in practice and hence what counts as backcompat.

Even if we think this is not the case, I am ok for any code gen feature like this not working on those scenarios. The outcome is that either your page does not get automatically discovered or the render mode for your internal/private nested component is not taken into account.

That is OK. We just have to remember not to design anything where it becomes problematic for people to do this.

That said, I think we are unnecessarily boxing ourselves here. There is little/none value from having a component being internal/private on apps and there are big drawbacks with regards to code generation.

I prefer to build a framework that naturally flows as part of the language. Components are like any other class. If there is value in making any class internal/private, the same rationale applies for components. It's very reasonable for an RCL to use components as internal implementation details without wanting them to be public.

[javiercn]

I prefer to build a framework that naturally flows as part of the language. Components are like any other class. If there is value in making any class internal/private, the same rationale applies for components. It's very reasonable for an RCL to use components as internal implementation details without wanting them to be public.

The challenge here is that we are splitting the world into people using Razor Syntax that require additional requirements and the "relaxed" version the framework enforces.

This makes it confusing for users when something doesn't work in Razor, and I don't think that it sets users up for success, but I also think that it's not critical either, as the vast majority of our users will use Razor syntax.

I would like to settle this discussion as follows:

• Non-public components won't be supported by any codegen/discovery feature for the time being.
  • This will be guidance we provide.
• If we receive feedback from customers that this is a problem, we'll re-evaluate alternative approaches:
  • Emit a warning when we detect cases where we can't source generate.
  • Update our source generation strategy to account for these cases via an alternative mechanism.

[SteveSandersonMS]

That sounds fine to me 😄

Keeping things open to pure C#-only components has been one of the strongest architectural moves we've pulled throughout Blazor and is partly what has stopped it from devolving into masses of custom template syntax as other UI frameworks and syntaxes have suffered in the past. It also keeps us open to major shifts in the future, such as entirely different code-based authoring models. So I am definitely keen not to compromise on that, even though it makes us think harder about framework design.

.razor is already a form of codegen, and doing further codegen on top of its conventions is perfectly fine and good. We just need to make sure we don't make non-.razor approaches impractical. We need to think of .razor as an extra layer within the system, not as the core.

For the specific features in this PR, I think the limitation of being public is fine. The reflection-based discovery within Router is already limited to public types so nothing weird is going to happen.

[SteveSandersonMS]

Is this only being used for routable page components? Just want to understand why we would be excluding our own framework components. If this is for build perf, another approach to consider would be filtering out assemblies that don't transitively reference M.A.Components, as that would eliminate everything from System.*.

[javiercn]

It was done for perf, but on second thought this looks like premature optimization on my part, so I am going to remove it.

Assembly references are not transitive. They are always a flat list, hence why I wanted to avoid the excessive scanning, but in retrospective, this will only happen once, so I think it's ok.

[SteveSandersonMS]

Approving as this looks great. I expect some updates may be needed to match updated APIs from #48283 but at least you'll be free to merge when ready.

[ghost]

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no breaking changes are introduced, please leave an /azp run comment here to rerun the CI pipeline and confirm success before merging the change.