-
Notifications
You must be signed in to change notification settings - Fork 2k
[XAML] XIHR: deterministic content-hash versioning + always-emit UpdateComponent #36833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cb8c0a2
52e83f3
c370447
f50acd1
f96c372
b81c6c0
9626ba6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,35 @@ public static string EscapeIdentifier(string identifier) | |
| : $"@{identifier}"; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// A stable, deterministic 32-bit content hash (FNV-1a) of the XAML text, used as the | ||
| /// <c>__version</c> content identity for XAML Incremental Hot Reload. Unlike a monotonically | ||
| /// increasing counter (which depends on edit history held in mutable static state and makes the | ||
| /// generator non-deterministic), this value is a pure function of the current XAML content, so | ||
| /// identical content always yields the same identity — and a revert to earlier content restores | ||
| /// the earlier identity. Unlike <see cref="string.GetHashCode()"/> it is not randomized per | ||
| /// process, so it is reproducible across builds/hosts. Returned as a non-negative int. | ||
| /// </summary> | ||
| public static int StableContentHash(string? content) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can probably use the System.IO.Hashing NuGet for this, works even on .NET framework. We use it on dotnet/android.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @tmat. Rather than switching FNV-1a → |
||
| { | ||
| unchecked | ||
| { | ||
| const uint fnvOffset = 2166136261; | ||
| const uint fnvPrime = 16777619; | ||
| uint hash = fnvOffset; | ||
| if (content != null) | ||
| { | ||
| foreach (char c in content) | ||
| { | ||
| hash = (hash ^ (byte)(c & 0xFF)) * fnvPrime; | ||
| hash = (hash ^ (byte)((c >> 8) & 0xFF)) * fnvPrime; | ||
| } | ||
| } | ||
| // Fold to a non-negative int so it renders as a plain integer literal. | ||
| return (int)(hash & 0x7FFFFFFF); | ||
| } | ||
| } | ||
|
Comment on lines
+45
to
+66
|
||
|
|
||
| public static ProjectItem? ComputeProjectItem((AdditionalText additionalText, AnalyzerConfigOptionsProvider optionsProvider) tuple, CancellationToken cancellationToken) | ||
| { | ||
| if (cancellationToken.IsCancellationRequested) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,21 +14,28 @@ | |
| namespace Microsoft.Maui.Controls.SourceGen; | ||
|
|
||
| /// <summary> | ||
| /// Generates a single <c>UpdateComponent()</c> partial method containing accumulated | ||
| /// <c>if (__version == N)</c> patch blocks from successive XAML Hot Reload edits. | ||
| /// Generates a single <c>UpdateComponent()</c> partial method that applies one absolute | ||
| /// previous→current patch for XAML Incremental Hot Reload. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// Per the spec, the generated method chains sequential <c>if</c> blocks (not <c>else if</c>): | ||
| /// The method is emitted UNCONDITIONALLY (no <c>if (__version == N)</c> version-chain guard) and is | ||
| /// present on EVERY generation — even the first compile and no-op edits — so a XIHR type never gains | ||
| /// or loses the method across generations (that member churn is what crashes Roslyn's EnC delta | ||
| /// tracking). Its body applies the current patch and then stamps <c>__version</c> with a deterministic | ||
| /// content hash of the current XAML: | ||
| /// <code> | ||
| /// internal void UpdateComponent() | ||
| /// { | ||
| /// if (__version == 0) { /* v0→v1 patch */ __version = 1; } | ||
| /// if (__version == 1) { /* v1→v2 patch */ __version = 2; } | ||
| /// /* absolute previous→current patch (property sets assign target values) */ | ||
| /// __version = 123456789; // stable content hash of the current XAML | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Documentation — This example documents behavior the shipped generator does not emit, and it contradicts the classifier that depends on it.
This isn't cosmetic. The empty-UC IL-length classifier only works because an empty body is truly Suggested fix: drop the Flagged by: 2/3 reviewers |
||
| /// return; | ||
| /// } | ||
| /// </code> | ||
| /// A v0 instance chains through ALL patches; a fresh instance (whose <c>InitializeComponent</c> | ||
| /// sets <c>__version</c> to the latest) skips them all. | ||
| /// Because patch property-sets are absolute, a single patch brings any live instance to the current | ||
| /// state regardless of which edit it was last updated to, and a revert to earlier content collapses | ||
| /// to the earlier patch/identity — deterministic, revert-stable output for identical XAML (no | ||
| /// accumulated chain, no stale intermediate values). See the XIHR versioning determinism fix. | ||
| /// </para> | ||
| /// <para> | ||
| /// Property value encoding strategy (in priority order): | ||
|
|
@@ -48,9 +55,14 @@ static class UpdateComponentCodeWriter | |
| const string NewLine = "\n"; | ||
|
|
||
| /// <summary> | ||
| /// Generates the code for a single <c>if (__version == fromVersion) { ... __version = toVersion; }</c> block. | ||
| /// Returns <see langword="null"/> when <paramref name="diff"/> contains no changes. | ||
| /// Generates the statements that apply a single previous→current patch (property sets, child-list | ||
| /// changes, etc.), WITHOUT any <c>if (__version == N)</c> guard or <c>__version</c> assignment — the | ||
| /// caller (<see cref="GenerateUpdateComponent(INamedTypeSymbol, string, string?, int)"/>) wraps this | ||
| /// body and stamps the content-hash identity. Returns <see langword="null"/> when | ||
| /// <paramref name="diff"/> contains no changes. | ||
| /// </summary> | ||
| /// <param name="fromVersion">Vestigial: the monotonic version no longer drives dispatch (kept for the state/bookkeeping call chain and test signatures).</param> | ||
| /// <param name="toVersion">Vestigial: see <paramref name="fromVersion"/>.</param> | ||
| /// <param name="newIds">ID dictionary for added nodes (from the new tree). May be null for tests.</param> | ||
| public static string? GeneratePatchBody( | ||
| XamlTreeDiff diff, | ||
|
|
@@ -130,11 +142,6 @@ static class UpdateComponentCodeWriter | |
| codeWriter.WriteLine(); | ||
| } | ||
|
|
||
| // Always bump __version, even when individual TryGet probes missed. Skipping the bump | ||
| // would strand the instance at fromVersion and force the same (already-failed) patch | ||
| // to re-run on every subsequent UpdateComponent() invocation, never making progress. | ||
| codeWriter.WriteLine($"__version = {toVersion};"); | ||
|
|
||
| codeWriter.Flush(); | ||
| return codeWriter.InnerWriter.ToString(); | ||
| } | ||
|
|
@@ -143,20 +150,31 @@ static class UpdateComponentCodeWriter | |
| /// Assembles a complete <c>UpdateComponent()</c> source file from accumulated patch bodies. | ||
| /// Each patch body becomes an <c>if (__version == N) { ... }</c> block inside the single method. | ||
| /// </summary> | ||
| public static string? GenerateUpdateComponent( | ||
| /// <summary> | ||
| /// Generates the <c>UpdateComponent()</c> method body from a single baseline→current patch. | ||
| /// The patch is <em>always</em> emitted (even when <paramref name="patchBody"/> is null/empty), so the | ||
| /// method never appears→disappears across generations — that member churn is what crashes Roslyn's | ||
| /// EnC delta tracking (dotnet/maui XIHR versioning fix). Because patch property-sets are absolute | ||
| /// (they assign target values, not relative deltas), a single baseline→current patch correctly brings | ||
| /// any live instance to the current state regardless of which edit it was last updated to, and a revert | ||
| /// to the baseline collapses to an empty patch — no accumulated version chain, no stale intermediate | ||
| /// values, deterministic output for identical XAML. | ||
| /// </summary> | ||
| public static string GenerateUpdateComponent( | ||
| INamedTypeSymbol rootType, | ||
| string accessModifier, | ||
| List<string> allPatchBodies, | ||
| int startVersion = 0) | ||
| string? patchBody) | ||
| { | ||
| if (allPatchBodies.Count == 0) | ||
| return null; | ||
|
|
||
| using var codeWriter = new IndentedTextWriter(new StringWriter(CultureInfo.InvariantCulture), "\t") { NewLine = NewLine }; | ||
|
|
||
| codeWriter.WriteLine(GeneratorHelpers.AutoGeneratedHeaderText); | ||
| codeWriter.WriteLine("#nullable enable"); | ||
| codeWriter.WriteLine("#pragma warning disable CS0219 // Variable is assigned but its value is never used"); | ||
| // A XAML class that derives from another XAML class emits its own UpdateComponent(), which | ||
| // intentionally hides the base's (each level patches its own XAML tree). Because UpdateComponent | ||
| // is now emitted on EVERY generation — not just on edit — this hiding surfaces at build time for | ||
| // inherited XAML; suppress the member-hiding warning in this generated file. | ||
| codeWriter.WriteLine("#pragma warning disable CS0108 // Member hides inherited member; missing new keyword"); | ||
| codeWriter.WriteLine(); | ||
| codeWriter.WriteLine($"namespace {rootType.ContainingNamespace};"); | ||
| codeWriter.WriteLine(); | ||
|
|
@@ -167,21 +185,17 @@ static class UpdateComponentCodeWriter | |
| codeWriter.WriteLine($"[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]"); | ||
| codeWriter.WriteLine($"internal void UpdateComponent()"); | ||
|
|
||
| // The method is ALWAYS emitted (member stability / no EnC churn), but its BODY is empty when | ||
| // this generation carries no XAML change (first compile, empty/reverted diff, structural | ||
| // reset). A non-empty body means the XAML actually changed. The SDK's MetadataUpdateHandler | ||
| // classifies a delta as a XAML change by inspecting whether this method's body is non-empty | ||
| // (its compiled IL is more than a trivial return) — an empty UpdateComponent() therefore reads | ||
| // as "not a XAML change", which is what keeps pure C#/code-behind edits out of the XAML-change | ||
| // signal while still guaranteeing the method never appears/disappears across generations. | ||
| using (PrePost.NewBlock(codeWriter)) | ||
| { | ||
| // Emit each patch as a sequential if (__version == N) block | ||
| for (int i = 0; i < allPatchBodies.Count; i++) | ||
| { | ||
| var body = allPatchBodies[i]; | ||
| var version = startVersion + i; | ||
| codeWriter.WriteLine($"if (__version == {version})"); | ||
| using (PrePost.NewBlock(codeWriter)) | ||
| { | ||
| WriteIndentedBody(codeWriter, body); | ||
| } | ||
| } | ||
|
|
||
| codeWriter.WriteLine("return;"); | ||
| if (!string.IsNullOrWhiteSpace(patchBody)) | ||
| WriteIndentedBody(codeWriter, patchBody!); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -207,7 +221,7 @@ static class UpdateComponentCodeWriter | |
| if (patchBody == null) | ||
| return null; | ||
|
|
||
| return GenerateUpdateComponent(rootType, accessModifier, new List<string> { patchBody }, startVersion: fromVersion); | ||
| return GenerateUpdateComponent(rootType, accessModifier, patchBody); | ||
| } | ||
|
|
||
| static void WriteIndentedBody(IndentedTextWriter codeWriter, string body) | ||
|
|
@@ -312,7 +326,8 @@ static void EmitChildListChange( | |
| bool hasAdded = false; | ||
| for (int i = 0; i < change.NewChildren.Count; i++) | ||
| { | ||
| if (change.NewChildren[i].Kind == ChildChangeKind.Added) { hasAdded = true; break; } | ||
| if (change.NewChildren[i].Kind == ChildChangeKind.Added) | ||
| { hasAdded = true; break; } | ||
| } | ||
| bool pureReorder = !hasAdded && change.RemovedNodeIds.Count == 0; | ||
|
|
||
|
|
@@ -1163,7 +1178,8 @@ static void TryEmitAttachedPropertyChange( | |
| break; | ||
| } | ||
| } | ||
| if (getter != null) break; | ||
| if (getter != null) | ||
| break; | ||
| current = current.BaseType!; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Config Impact / Regression — This line is an undisclosed revert of merged PR #36832, bundled into a PR described as source-generator-only.
git logon this file confirms HEAD contains the commitb81c6c064c Revert "Turn XAML Incremental Hot Reload (XIHR) off by default (#36832)". #36832 was merged intonet11.0on 2026-07-28 with the explicit rationale: "XIHR was enabled by default for Debug builds on the net11.0 branch, which is causing issues. This PR makes XIHR opt-in instead of on-by-default." This line re-flips that default back on for every Debug build.The PR description states "This is a source-generator-only change (
src/Controls/src/SourceGen); no public API changes" — which isn't accurate for this file. There is no mention of the revert, no link to #36832, and no statement that the issues motivating it are resolved.Scenario: every
net11.0MAUI app that builds Debug without explicitly settingEnableMauiIncrementalHotReload=falsesilently re-acquires whatever #36832 was disabling. Because the flip is buried in a PR titled "deterministic versioning," bisecting a resulting regression points at the wrong change. This also compounds the two ❌ findings below, both of which are only reachable by default because of this line.Two things worth noting: the repo's own review rules require that a PR reverting a labeled fix name that PR and get explicit author acknowledgment; and the test comment added at
MSBuildTests.cs:311-313already says "on by default in Debug" — so the flip is known to the change, just not to the description.Suggested fix: split the default flip out of this PR and land the determinism/always-emit work first; or keep it and explicitly document the revert, link #36832, confirm its original issues are fixed, and get sign-off from whoever merged it.
Flagged by: 3/3 reviewers