Keep a fallback component's derived tag helpers during discovery - #84818
Merged
Conversation
Slow discovery owns a fallback component's tag helpers and filters the discovered set down to the fallback type names. It matched each descriptor by its own type name, which is correct for the component descriptor but not for descriptors derived from it: a child content descriptor's type name is the component's suffixed with the property name (e.g. Card.Header), so it failed the filter and was dropped. A consumer then saw the component without its child content, warning RZ10012 for elements like <Header> and <ChildContent>. Filter on the owning component's name -- reconstructed from the descriptor's namespace and identifier, which point at the component for both the component descriptor and its derived descriptors -- so a fallback component and its child content (and other derived descriptors) are kept, or excluded from fast discovery, together. Fixes dotnet#84817 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Razor “Sonic” split-declaration discovery regression where fallback components were losing derived tag helper descriptors (notably child-content descriptors), which caused spurious RZ10012 warnings in consumers. The fix updates the ownership filtering logic so fallback components are kept/excluded together with their derived descriptors by filtering on the reconstructed owning component type name.
Changes:
- Add
GetOwningTypeName(TagHelperDescriptor)helper to reconstruct the owning component’s full type name fromTypeNamespace+TypeNameIdentifier. - Update both slow-path inclusion and fast-path exclusion filters to use the owning type name (instead of
descriptor.TypeName) when matching against fallback component type names. - Add a regression unit test validating that fallback component child content parameters (
Header/ChildContent) no longer produceRZ10012.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/RazorSourceGeneratorComponentTests.cs | Adds regression test covering fallback component child-content descriptors surviving discovery. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.Helpers.cs | Adds GetOwningTypeName helper used to identify the owning component for derived descriptors. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs | Switches fallback ownership filters to match on the owning type name to retain/exclude derived descriptors correctly. |
Member
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
davidwengier
approved these changes
Aug 8, 2026
JoeRobich
pushed a commit
that referenced
this pull request
Aug 11, 2026
Backs the `features/sonic` decl/impl split out of `main` to unblock insertion. Reverts the merge (#84778) and the commits that stacked on top of it and cannot stand without it. ## What reverts - **#84778** `Merge/sonic into main` -- the whole feature body (reverted with `-m 1`, keeping pre-sonic `main`). - **#84818** fallback child-content discovery fix -- patches sonic's fast/slow discovery, which no longer exists. - **#84813** restore `GetCSharpDocument()`/`GeneratedCode` -- these were re-added to shim sonic's API removal; reverting the merge restores the originals, so the shim would double-define them. - **#84795** always pass C# formatting options -- edits `CSharpFormattingOptionsHelper.cs`, a file that only exists because of sonic. ## What is kept The other post-merge commits are independent of sonic and stay: arcade bumps, SDL param removal, CS8802, #84758 (syntax-tree sharing), and the two Razor features that merely shared files -- **#84760** (unbound-attribute warnings) and **#84764** (empty-outer-tag warning). Preserving #84760 needed one manual resolution in `UnresolvedAttributeIntermediateNode.cs`: sonic added the `CloneNode()`/`Clone()` overrides and #84760 later added one property assignment inside `CloneNode`. The overrides are sonic infrastructure and go with the revert; #84760's `IsDirectiveAttributeCandidate` property and all its real usages are untouched. ## Validation - `Razor.slnf` builds clean (compiler + all tooling projects, including the ones #84795 touched). - `Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests`: 215 passed. - `Microsoft.AspNetCore.Razor.Language.UnitTests` (component / tag helper / bind / unbound-attribute): 1593 passed -- confirms #84760's kept feature still works. ## Note Reverting the merge means re-landing `features/sonic` later will need this revert itself reverted first -- expected for a back-out. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/84831)
chsienki
added a commit
that referenced
this pull request
Aug 21, 2026
…#84844) Re-lands the sonic decl/impl split (backed out in #84831 to unblock insertions) together with the slow-discovery perf work (#84808) and a new fix that removes the remaining cold-allocation regression on fallback components. ## Why this is safe to re-land now The split was reverted because a VS insertion showed a cold-allocation regression on fallback-heavy scenarios (Component Classification, cohosting completion) with no offsetting speed gain. This branch addresses the root cause rather than just re-applying the revert. ## What's here 1. **Un-revert** of #84831 -- restores the sonic merge and the fixes that landed on top of it (#84818 fallback child-content discovery, #84813 RZC public-API compat, #84795 formatting options). 2. **#84808** -- slow discovery resolves fallback types via the declaration table (`GetSymbolsWithName`) instead of a per-tree semantic model, and discovers only the fallback types rather than re-walking the whole augmented compilation. (Removed the +535 MB semantic-model regression.) 3. **Reuse the initial parse for fallback discovery** (new) -- the remaining regression. ## The new fix A fallback component (an `@inherits`/`@implements`/`@typeparam` header, or body markup the analysis can't route) can't be split, so its descriptor comes from discovery over an augmented compilation. Each fallback component was being processed **twice**: the generation engine's initial parse, then a freshly created declaration engine that **re-parsed the same source** just to produce a discoverable decl. For an app where nearly every component `@inherits` a base (MudBlazor, OrchardCore) every component is a fallback, so that second parse dominates cold discovery allocation. The split phase already builds the discoverable declaration surface for a *split* component from the nodes it just classified. This builds the same surface for a *fallback* component and stashes it on the document node; the generator lowers it in place -- reusing the initial parse -- instead of re-parsing through a separate engine. It's never emitted to pre-compilation (the bodiless type shell still fills that role for C# type resolution); only its syntax tree feeds slow discovery. The rare shapes with no render method or namespace keep the re-parse path. ## Measurements Cold source-generator allocation over 100 fallback components (in-memory A/B): | | Allocated | |---|---| | Pre-split baseline | 17.1 MB | | Sonic + #84808 (before this fix) | 19.6 MB | | **This branch** | **13.6 MB** | The fallback path is now **below** the pre-split baseline -- a genuine win, not break-even. ## Validation - `Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests`: 219 passed / 1 skipped (incl. #84818's fallback child-content test). - `Microsoft.AspNetCore.Razor.Language.UnitTests` (component / tag helper / split / bind / discovery / clone): 1672 passed. - Razor compiler builds clean. Draft pending the RPS + Speedometer insertion run. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/84844) --- ## Update: warm/interactive path fix (html-span completion) A matched-baseline Speedometer re-run showed the cold-discovery regressions gone but a smaller residual on **html-span completion in a component** (`CLR_BytesAllocated_devenv`) -- the interactive/warm path, a different signature than the cold source-generator work above. Root cause: the fallback decl incremental pipeline compared decls by **instance**, while the split-decl (`DeclSources`) fast path compares by **text**. A fallback component's decl is markup-free and checksum-suppressed, so a markup-only edit (every keystroke during completion) yields a byte-identical decl -- but the instance changed each run, invalidating the parse and, through the `Collect`, re-running slow discovery over *every* fallback component on each edit. Fix: compare fallback decls by text (`Text.ContentEquals`), mirroring the fast path. A markup-only edit now leaves the fallback decl `Unchanged`, keeping its parse and slow discovery cached. Measured (100 fallback components, warm re-run on a markup edit): | | warm alloc | |---|---| | Pre-split baseline | 0.41 MB | | Before this fix | 3.2 MB (~8x) | | **After this fix** | **0.42 MB** | Guarded by `IncrementalCompilation_WhenFallbackComponentMarkupChanges_SlowDiscoveryStaysCached` (verified to fail without the comparer). Full SG suite 220 passed / 1 skipped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #84817 -- a Sonic (split Razor declarations) regression where a fallback component's child content parameters were lost, causing spurious
RZ10012warnings (e.g.unexpected name 'Header'/'ChildContent') at every consumer.Root cause
Discovery splits into a fast path (components with a split declaration) and a slow path (fallback components with no split --
@inherits/@implements/@typeparamor unroutable body markup). The slow path owns a fallback component's tag helpers and filters the discovered set down to the fallback type names, and fast discovery excludes those same names to avoid duplicates.Both filters matched each descriptor by its own
TypeName. That is correct for the component descriptor itself, but a component's derived descriptors -- notably child content descriptors produced byComponentTagHelperProducer-- carry aTypeNameof the component suffixed with the property name (Card.Header,Card.ChildContent). Those names are not in the fallback type set, so the child content descriptors failed the filter and were silently dropped. The consumer then sawCardwithout its child content parameters and warnedRZ10012on<Header>/<ChildContent>.Fix
Filter on the owning component's name, reconstructed from the descriptor's
TypeNamespace+TypeNameIdentifier(both point at the owning component for a component descriptor and its derived descriptors). A newGetOwningTypeNamehelper does the reconstruction and both ownership predicates use it, so a fallback component and its derived descriptors are kept -- or excluded from fast discovery -- as a unit.Testing
Component_FallbackComponent_ChildContentParametersreproduces the regression (fallbackCardwithHeader/ChildContentRenderFragmentparams, consumed with child-content elements) and asserts noRZ10012.Microsoft Reviewers: Open in CodeFlow