TagHelperCollection Part 4: Rewrite Tag Helper Discovery(!) by DustinCampbell · Pull Request #12507 · dotnet/razor

DustinCampbell · 2025-11-18T22:10:49Z

Warning

This pull request contains breaking changes for the RazorSdk. Once this is merged and flows to the VMR, dotnet/dotnet@dc17a09 will need to be cherry-picked to resolve build breaks in src/sdk/src/RazorSdk.

Previously, tag helpers were discovered by ITagHelperDescriptorProvider. Each provider was responsible for walking a compilation's assemblies and producing TagHelperDescriptors from the types within. This change inverts the tag helper discovery process by introducing ITagHelperDiscoveryService and moving the tag helper construction logic into a set of TagHelperProducers. The ITagHelperDiscoveryService performs a single walk of the compilation or assembly and calls the producers as needed.

Importantly, the new process allows a more expansive cache to be maintained. There is now a per-assembly cache that holds onto TagHelperCollection instances. The old cache that was owned by providers via TagHelperCollector has been removed.

To complete this change, ITagHelperDescriptorProvider and related types have been deleted.

CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842196&view=results
Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842249&view=results

davidwengier

I hope that second last commit was mechanical, because I completely glossed over it. Last one wasn't much better :P

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/TagHelperDiscoveryService.cs

ToddGrun · 2025-11-20T00:30:23Z

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/TagHelperDiscoveryService.cs

+            watch.Stop();
+
+            timingsSpan[0] = (provider.GetType().Name, watch.Elapsed);
+            timingsSpan = timingsSpan[1..];


Definitely no problem with this, but it still feels like if we had something like a FixedSizeArrayBuilder, this would flow a bit more natural for me.

For sure! I've been thinking about bringing that type over too.

...osoft.CodeAnalysis.Razor.Compiler/src/Language/TagHelpers/Producers/BindTagHelperProducer.cs

ToddGrun · 2025-11-20T00:49:58Z

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/CompilerFeatures.cs


        if (builder.Configuration.LanguageVersion >= RazorLanguageVersion.Version_3_0)
        {
+            builder.Features.Add(new BindTagHelperProducer.Factory());


builder.Features.Add(new BindTagHelperProducer.Factory());

Is DefaultTagHelperProducer.Factory special in that it's not needed in here?

DefaultTagHelperProducer.Factory is less of a "default" and more of a "producer for legacy tag helpers". I left the name as "Default" because it was previously called DefaultTagHelperDescriptorProvider. It's the thing that makes ITagHelper derived tag helper descriptors. By default, the compiler doesn't use it. It gets added by the various RazorExtensions.Register(...) calls though.

Amusingly, the source generator added two instances of DefaultTagHelperDescriptorProvider. It added one specifically (which I removed below) and a second was added because it unconditionally calls RazorExtensions.Register(...).

These are the ITagHelperDescriptorProvider instances the Razor source generator uses for tag helper discovery on main. Note that there's two "DefaultTagHelperProviderDescriptor" instances in there.

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/CompilerFeatures.cs

ToddGrun · 2025-11-20T00:56:59Z

...Microsoft.CodeAnalysis.Razor.Compiler/src/Language/TagHelpers/Producers/TagHelperProducer.cs

+
+internal abstract class TagHelperProducer
+{
+    public abstract class FactoryBase : RazorEngineFeatureBase, IRazorEngineFeature, ITagHelperProducerFactory


FactoryBase

Would it make sense for this to be genericized to the specific TagHelperProducer type it returns?

I spent some time exploring that, but it didn't add value because everything ultimately calls through ITagHelperProducerFactory.

ToddGrun · 2025-11-20T01:12:55Z

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/TagHelperDiscoverer.cs

+        {
+            // First, let producers add any static tag helpers they might have.
+            // Also, capture any producers that need to analyze types.
+            using var _ = ArrayPool<TagHelperProducer>.Shared.GetPooledArraySpan(


ArrayPool

Is there an advantage to using this over a MemoryBuilder?

Not really. I just don't think to reach for MemoryBuilder when I don't need a builder that grows. When I need a fixed-sized buffer, I tend to just go right to ArrayPool<>.

ToddGrun · 2025-11-20T01:20:11Z

This looks great. I think I find this easier to understand now.

| Prelude | [Part 1](#12504) | [Part 2](#12505) | [Part 3](#12506) | [Part 4](#12507) | [Part 5](#12509) | This is a set of support utilities and helpers added as part of the `TagHelperCollection` changes: - Added `InterlockedOperations.Initialize(...)` overloads for lock free initialization with a factory method. (f3686aa) - Added two `LazyValue` structs that use the new `InterlockedOperations.Initialize(...)` overloads. These are useful for lazily initializing a value without extra allocations or locks. (81795f5) - Added `OverloadResolutionPriorityAttribute` polyfill type. (27ab174) - Refactored `TagHelperCache` to extract `CleanableWeakCache<TKey, TValue>`. This caches items as weak references. When the number of items added reaches a configurable threshold, the cache is cleaned up, and any dead weak references are removed. (673a9b5) ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842136&view=results Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842197&view=results

DustinCampbell · 2025-11-20T22:43:01Z

This looks great. I think I find this easier to understand now.

I'm glad you think so! I agree!

ToddGrun

| [Prelude](#12503) | Part 1 | [Part 2](#12505) | [Part 3](#12506) | [Part 4](#12507) | [Part 5](#12509) | This pull request represents all new code! It introduces a new immutable collection type, `TagHelperCollection`, that is designed to contain `TagHelperDescriptors`. It is built with the following principles: 1. Guarantee that collections never contain duplicate tag helpers. 2. Collections can be compared efficiently via checksums. 3. Multiple collections can be merged into a single collection efficiently, with minimal array copying. 4. Determining whether a collection contains a tag helper is efficient. > [!TIP] > Since this is all new code, I recommend reviewing commit-by-commit. Each commit represents a cohesive portion of new code. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842145&view=results Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842216&view=results

| [Prelude](#12503) | [Part 1](#12504) | Part 2 | [Part 3](#12506) | [Part 4](#12507) | [Part 5](#12509) | > [!WARNING] > This pull request contains breaking changes for the RazorSdk. Once this is merged and flows to the VMR, dotnet/dotnet@9a7e708 will need to be cherry-picked to resolve build breaks in `src/sdk/src/RazorSdk`. These commits represent the (mostly) mechanical changes needed to integrate `TagHelperCollection` across the Razor code base. Most references to `ImmutableArrary<TagHelperDescriptor>`, `IReadOnlyList<TagHelperDescriptor>`, `IEnumerable<TagHelperDescriptor>`, and `TagHelperDescriptor[]` have been replaced with `TagHelperCollection`. This is **by far** the largest of the `TagHelperCollection` pull requests. While most of the commits contain mechanical changes across the code base, there are few that include more substantial work that require a bit more scrutiny: - **Update `RenameService` to remove `ImmutableArray<TagHelperDescriptor>`** (fa3ad2b) This includes a fair amount of refactoring in `RenameService` to fix bugs found when moving to `TagHelperCollection`. - **Update `TagHelperFacts` to use `TagHelperCollection`** Extra work was done in `DirectiveAttributeComplationItemProvider` to clean up a bit following #12473. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842165&view=results Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842237&view=results

…c in source generator (#12506) | [Prelude](#12503) | [Part 1](#12504) | [Part 2](#12505) | Part 3 | [Part 4](#12507) | [Part 5](#12509) | This is a relatively small change to remove LINQ from the Razor source generator and use `TagHelperCollection` when checking for added/removed tag helpers. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842169&view=results Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842240&view=results

Add TagHelperDiscoveryService to the compiler that manages calling into ITagHelperDescriptorProviders. All existing code that previously used ITagHelperDecriptorProviders directly now uses TagHelperDiscoveryService. TagHelperDiscoveryService manages a per-assembly cache using the existing AssemblySymbolData infrastructure. Currently, this is essentially a copy-paste of the per-assembly cache in TagHelperCollector.

BindTagHelperDescriptorProvider and ComponentTagHelperDescriptorProvider are a bit unique in that there's a dependency between them. When BindTagHelperDescriptorProvider runs, it walks all of the previously added tag helpers and tries to add bind tag helpers for any components. This change breaks this dependency by introducing BindTagHelperProducer, which is used by both BindTagHelperDescriptorProvider and ComponentTagHelperDescriptorProvider. Now, ComponentTagHelperDescriptorProvider is responsible for adding bind tag helpers for the components any it adds.

- Move assembly-walking algorithm from TagHelperCollector to TagHelperDiscoveryService. - Introduce TagHelperDiscoverer to cache details derived from a compilation while walking assemblies. - Delete ITagHelperDescriptorProvider and all implementations - Use a ConcurrentDIctionary<int, TagHelperCollection> as the per-assembly cache and compute the key based on both options and the selected producers. - Add new RegisterDefaultTagHelperProducer API for RazorSdk to use rather than directly referencing DefaultTagHelperDescriptorProvider. - Update tests

…xt (#12509) | [Prelude](#12503) | [Part 1](#12504) | [Part 2](#12505) | [Part 3](#12506) | [Part 4](#12507) | Part 5 | This change introduces a weak cache for `TagHelperDocumentContext` keyed by the tag helper prefix string and `TagHelperCollection` checksum. This helps avoid creating new `TagHelperBinders` for the same set of tag helpers, since `TagHelperBinder` is expensive to create. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842228&view=results Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2842250&view=results

DustinCampbell requested a review from a team as a code owner November 18, 2025 22:10

davidwengier approved these changes Nov 19, 2025

View reviewed changes