dotnet / dotnet/maui

[XSG] XamlHotReloadState static cache is brittle across multiple throwaway Roslyn compilations

Open
#36,937 0 comments 1 reaction 0 assignees View on GitHub
partner/hot-reload-xaml s/triaged xsg
Dominant language
C#
Stars
23.3k
Forks
2k
Avg merge
1d 10h
Merged PRs (30d)
297

Description

Follow-up to @tmat's review on #36833 (inline comment on `src/Controls/src/SourceGen/XamlHotReloadState.cs`).

### Concern
`XamlHotReloadState` holds XAML Incremental Hot Reload (XIHR) generation state in **mutable static state** and diffs the CURRENT generation against a CACHED PREVIOUS one to produce the incremental patch (the `UpdateComponent()` body).

Tomas asked (paraphrased): *"Sounds somewhat brittle. Would it work well if Roslyn created multiple compilations in the same process, ran source generators on them, and then threw them away?"*

### Why it's brittle
The cache is static (not tied to a specific `Compilation`) and survives across compilations in the same process — e.g. an IDE/design-time host, `dotnet watch`, or a test host that spins up several `Compilation`s. Interleaved or throwaway compilations can:
- **Miss a real change** — if the "previous" cached snapshot came from an unrelated/throwaway compilation, the diff against the current content can come out **empty**, so `UpdateComponent()` is emitted empty and Hot Reload silently drops a genuine XAML edit.
- **Report a spurious change** — conversely, a stale cached "previous" can make an unchanged file look changed.

This matters now that XIHR is on-by-default in Debug (#36833) and the diagnostics signal is *"empty `UpdateComponent()` body ⟺ no XAML change"*, classified pre-dispatch on `UpdateRequested.HandledTypes`. A poisoned cache corrupts that signal.

### Options
1. **Key the cache by `Compilation`** (e.g. `ConditionalWeakTable`) so throwaway compilations don't cross-contaminate and entries are collected with the compilation. Lowest-risk; keeps the empty-UC signal.
2. **Drop the cache entirely** and always full-reapply the `UpdateComponent()` body. Most robust, but then `UpdateComponent()` is *never* empty, which **breaks the empty-UC diagnostics signal** — would require moving change-classification to a **runtime `UpdateComponent()` IL diff** and re-aligning with the XamlTools team (@noiseonwires).
3. Move to incremental-generator-native caching (`IncrementalValueProvider`) instead of hand-rolled static state.

Option 1 is the likely near-term fix; option 2 is the "correct" long-term design but carries a cross-team contract implication.

### References
- Review comment: https://github.com/dotnet/maui/pull/36833#discussion_r3658710660
- Companion comment (dead `__version`/`StableContentHash`) addressed in #36912
- File: `src/Controls/src/SourceGen/XamlHotReloadState.cs`

cc @tmat

Contributor guide

Open the contributing guide

Research direction

Start with src/Controls/src/SourceGen/XamlHotReloadState.cs and the UpdateComponent() body, then read the review discussion on #36833. Exercise the generator with multiple throwaway or interleaved Roslyn compilations and verify that unrelated compilations do not affect the cached diff; done means genuine XAML changes are preserved without breaking the empty UpdateComponent() diagnostics signal.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.