[XAML] Incremental XAML Hot Reload (source-generated patch chains)#34338
Open
StephaneDelcroix wants to merge 7 commits into
Open
[XAML] Incremental XAML Hot Reload (source-generated patch chains)#34338StephaneDelcroix wants to merge 7 commits into
StephaneDelcroix wants to merge 7 commits into
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34338Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34338" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds incremental XAML Hot Reload support to the XAML Source Generator pipeline by diffing XAML trees across generator runs and emitting versioned UpdateComponent_vNtoM() methods plus a per-page [MetadataUpdateHandler] dispatcher. Runtime support is introduced via a component registry to map live instances to stable node IDs.
Changes:
- Introduces a XAML tree diff engine + stable node-id assignment and uses them to generate
UpdateComponent_vNtoM()source on property-only edits. - Adds runtime
XamlComponentRegistry(with PublicAPI entries) used by generated code and the metadata update handler. - Adds/updates unit and integration tests and extends the source-gen test driver to plumb the MSBuild opt-in flag.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/src/SourceGen/XamlGenerator.cs | Wires incremental HR into generator output; emits UC + handler sources; updates in-proc state cache. |
| src/Controls/src/SourceGen/InitializeComponentCodeWriter.cs | Emits __version field + registry registration; adds helpers for root type resolution and UC generation. |
| src/Controls/src/SourceGen/UpdateComponentCodeWriter.cs | New UC code generator producing UpdateComponent_vNtoM() methods from diffs. |
| src/Controls/src/SourceGen/MetadataUpdateHandlerCodeWriter.cs | New generator emitting [MetadataUpdateHandler] glue to apply updates to live instances. |
| src/Controls/src/SourceGen/XamlNodeDiff.cs | New property-only diff engine for parsed XAML trees (structural changes => null). |
| src/Controls/src/SourceGen/NodeIdHelper.cs | New helper assigning stable {Type}_{depth}_{index} node IDs. |
| src/Controls/src/SourceGen/XamlHotReloadState.cs | New in-process cache for previous XAML + version per (assembly, relative path). |
| src/Controls/src/Xaml/XamlComponentRegistry.cs | New runtime registry (weakly keyed by instance) for nodeId → component and instance enumeration. |
| src/Controls/src/Xaml/PublicAPI/*/PublicAPI.Unshipped.txt | Adds new public API entries for XamlComponentRegistry across TFMs. |
| src/Controls/tests/SourceGen.UnitTests/SourceGeneratorDriver.cs | Extends test driver options to include EnableMauiIncrementalHotReload. |
| src/Controls/tests/SourceGen.UnitTests/InitializeComponent/SourceGenXamlInitializeComponentTests.cs | Updates hint-name filtering and adds IHR opt-in parameter plumbing. |
| src/Controls/tests/SourceGen.UnitTests/InitializeComponent/IncrementalHotReloadICTests.cs | New tests validating IC emits __version + registry registrations when IHR is enabled. |
| src/Controls/tests/SourceGen.UnitTests/XamlNodeDiffTests.cs | New unit tests for diff behavior (property diffs vs structural null). |
| src/Controls/tests/SourceGen.UnitTests/NodeIdHelperTests.cs | New tests verifying stable node ID assignment. |
| src/Controls/tests/SourceGen.UnitTests/UpdateComponentCodeWriterTests.cs | New tests for UC generation output shape and content. |
| src/Controls/tests/SourceGen.UnitTests/XamlIncrementalHotReloadPipelineTests.cs | New “two-run” integration tests covering generator replay behavior and UC/handler emission. |
| src/Controls/tests/SourceGen.UnitTests/SourceGen.UnitTests.csproj | Enables MetadataUpdaterSupport for tests involving hot reload infra. |
| src/Controls/tests/Core.UnitTests/XamlComponentRegistryTests.cs | New unit tests validating registry behavior (register/tryget/unregister/getinstances). |
Comment on lines
+112
to
+118
| // Extracts generated sources from a run result by hint-name suffix | ||
| static ImmutableArray<GeneratedSourceResult> GetSources(GeneratorDriverRunResult result) | ||
| => result.GeneratedTrees | ||
| .Select(t => t) | ||
| .Select(_ => default(GeneratedSourceResult)) | ||
| .ToImmutableArray(); | ||
|
|
There was a problem hiding this comment.
GetSources currently returns an array of default GeneratedSourceResult values derived from GeneratedTrees, but it doesn’t actually extract any generated sources and appears unused. This looks like leftover scaffolding; consider removing it to reduce confusion, or implement it properly and use it in assertions.
Suggested change
| // Extracts generated sources from a run result by hint-name suffix | |
| static ImmutableArray<GeneratedSourceResult> GetSources(GeneratorDriverRunResult result) | |
| => result.GeneratedTrees | |
| .Select(t => t) | |
| .Select(_ => default(GeneratedSourceResult)) | |
| .ToImmutableArray(); |
Contributor
Author
There was a problem hiding this comment.
Stale comment — GetSources is no longer present in this file (the test helper was reworked into ThreeRuns in a later commit). No action needed.
Comment on lines
+123
to
+126
| { | ||
| // Type unknown — use dynamic dispatch (slower but safe) | ||
| castPrefix = $"({varName} as global::Microsoft.Maui.Controls.BindableObject)?."; | ||
| } |
There was a problem hiding this comment.
The fallback branch for an unresolved node type emits ({varName} as BindableObject)?.{Property} = ..., which will not compile: (1) conditional access (?.) is not allowed on the left-hand side of an assignment, and (2) most properties (e.g. Text) don’t exist on BindableObject at compile time. Instead, if the node type can’t be resolved, the UC generator should conservatively fall back (e.g., emit goto fallback; for that node / return null to skip UC generation), or use a truly dynamic/reflection-based assignment that still compiles.
Contributor
Author
There was a problem hiding this comment.
Good catch — this was a real bug. The fallback emitted (x as BindableObject)?.Prop = value which is invalid C# (conditional access on the LHS of an assignment). Fixed in b260645: when NodeXmlType can't be resolved, skip emission and write a // skipped: node XAML type could not be resolved. breadcrumb instead of attempting an unsafe cast. The runtime fallback via BindableObject was also unreliable because most properties (e.g. Text) aren't defined there.
Comment on lines
+262
to
+272
| if (fqName is "int" or "global::System.Int32" or "long" or "global::System.Int64" | ||
| or "short" or "global::System.Int16" or "byte" or "global::System.Byte") | ||
| return rawXamlValue; // emit as-is (numeric literal) | ||
|
|
||
| // Float/double | ||
| if (fqName is "float" or "global::System.Single") | ||
| return $"{rawXamlValue}f"; | ||
| if (fqName is "double" or "global::System.Double") | ||
| return rawXamlValue; | ||
| if (fqName is "decimal" or "global::System.Decimal") | ||
| return $"{rawXamlValue}m"; |
There was a problem hiding this comment.
Numeric property literals are emitted directly from the raw XAML string (e.g., for int/double/decimal). If the XAML value isn’t a valid numeric literal (or uses a format that doesn’t compile), this will produce uncompilable generated code rather than falling back. Consider validating with invariant-culture TryParse for each numeric kind and returning null (goto fallback) when parsing fails.
Suggested change
| if (fqName is "int" or "global::System.Int32" or "long" or "global::System.Int64" | |
| or "short" or "global::System.Int16" or "byte" or "global::System.Byte") | |
| return rawXamlValue; // emit as-is (numeric literal) | |
| // Float/double | |
| if (fqName is "float" or "global::System.Single") | |
| return $"{rawXamlValue}f"; | |
| if (fqName is "double" or "global::System.Double") | |
| return rawXamlValue; | |
| if (fqName is "decimal" or "global::System.Decimal") | |
| return $"{rawXamlValue}m"; | |
| if (fqName is "int" or "global::System.Int32" | |
| or "long" or "global::System.Int64" | |
| or "short" or "global::System.Int16" | |
| or "byte" or "global::System.Byte") | |
| { | |
| if (fqName is "int" or "global::System.Int32") | |
| { | |
| if (int.TryParse(rawXamlValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out _)) | |
| return rawXamlValue; // emit as-is (numeric literal) | |
| return null; | |
| } | |
| if (fqName is "long" or "global::System.Int64") | |
| { | |
| if (long.TryParse(rawXamlValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out _)) | |
| return rawXamlValue; | |
| return null; | |
| } | |
| if (fqName is "short" or "global::System.Int16") | |
| { | |
| if (short.TryParse(rawXamlValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out _)) | |
| return rawXamlValue; | |
| return null; | |
| } | |
| if (fqName is "byte" or "global::System.Byte") | |
| { | |
| if (byte.TryParse(rawXamlValue, NumberStyles.Integer, CultureInfo.InvariantCulture, out _)) | |
| return rawXamlValue; | |
| return null; | |
| } | |
| } | |
| // Float/double | |
| if (fqName is "float" or "global::System.Single") | |
| { | |
| if (float.TryParse(rawXamlValue, NumberStyles.Float | NumberStyles.AllowThousands, CultureInfo.InvariantCulture, out _)) | |
| return $"{rawXamlValue}f"; | |
| return null; | |
| } | |
| if (fqName is "double" or "global::System.Double") | |
| { | |
| if (double.TryParse(rawXamlValue, NumberStyles.Float | NumberStyles.AllowThousands, CultureInfo.InvariantCulture, out _)) | |
| return rawXamlValue; | |
| return null; | |
| } | |
| if (fqName is "decimal" or "global::System.Decimal") | |
| { | |
| if (decimal.TryParse(rawXamlValue, NumberStyles.Number, CultureInfo.InvariantCulture, out _)) | |
| return $"{rawXamlValue}m"; | |
| return null; | |
| } |
Contributor
Author
There was a problem hiding this comment.
Not actually the case — numeric values aren't emitted from the raw XAML string. They flow through NodeSGExtensions.ValueForLanguagePrimitive (Int16/Int32/Int64/UInt16/UInt32/UInt64/Single/Double/Decimal/Byte/SByte branches in NodeSGExtensions.cs:402-540), each using TryParse(value, NumberStyles.Number, CultureInfo.InvariantCulture, …) with a ConversionFailed diagnostic on failure, and output via SymbolDisplay.FormatPrimitive which always produces a valid C# literal. No action needed.
Comment on lines
+155
to
+169
| // Emit Register calls and __version bump for incremental hot reload | ||
| if (nodeIds != null) | ||
| { | ||
| codeWriter.WriteLine(); | ||
| foreach (var kvp in sgcontext.Variables) | ||
| { | ||
| if (kvp.Key is ElementNode en | ||
| && nodeIds.TryGetValue(en, out var nodeId) | ||
| && !string.IsNullOrEmpty(nodeId)) | ||
| { | ||
| codeWriter.WriteLine($"global::Microsoft.Maui.Controls.Xaml.XamlComponentRegistry.Register(this, \"{nodeId}\", {kvp.Value.ValueAccessor});"); | ||
| } | ||
| } | ||
| codeWriter.WriteLine("__version = 1;"); | ||
| } |
There was a problem hiding this comment.
When EnableDiagnostics hot-reload fallback triggers (InitializeComponentRuntime(); return;), the new incremental-hot-reload bookkeeping (XamlComponentRegistry.Register(...) + __version = 1;) is skipped entirely. That leaves instances unregistered and/or with stale __version, so subsequent incremental updates will consistently fail/fallback. Consider explicitly opting the instance out before returning (e.g., unregister + reset __version), or ensure the runtime-inflation path also sets up registry/version so IHR can resume after a runtime reload.
Contributor
Author
There was a problem hiding this comment.
Good observation. The legacy ResourceProvider2 fallback (Xamarin-era pure-XAML hot reload) is a coexistence path: when it returns content, InitializeComponentRuntime re-parses the XAML and creates fresh children that the static IC code knows nothing about. We emit Unregister(this) + __version = 0 so the page's stale XIHR registrations are dropped, but we deliberately don't re-Register the new children (the legacy path doesn't know our node IDs). This makes the page a silent no-op for XIHR until the next normal Initialize — which is the right outcome: incremental hot reload is opt-in and meant to replace, not augment, the legacy ResourceProvider2 path. Happy to add an explicit code comment documenting this if you think it's worth it.
| readonly struct NodeDiff(string nodeId, IReadOnlyList<PropertyDiff> propertyChanges, XmlType? nodeXmlType = null) | ||
| { | ||
| /// <summary> | ||
| /// Stable path from the root, e.g. <c>""</c> for root, <c>"Label[0]"</c>, <c>"VerticalStackLayout[0]/Label[0]"</c>. |
There was a problem hiding this comment.
The NodeId XML-doc/comment describes an index-path format like Label[0] / VerticalStackLayout[0]/Label[0], but the implementation generates IDs in the {TypeName}_{depth}_{index} format (e.g., Label_1_0). Please update the comment to match the actual ID format to avoid misleading future maintainers.
Suggested change
| /// Stable path from the root, e.g. <c>""</c> for root, <c>"Label[0]"</c>, <c>"VerticalStackLayout[0]/Label[0]"</c>. | |
| /// Stable path from the root, composed of segments in the <c>{TypeName}_{depth}_{index}</c> format | |
| /// (e.g. <c>""</c> for root, <c>"Label_1_0"</c>, <c>"VerticalStackLayout_0_0/Label_1_0"</c>). |
Contributor
Author
There was a problem hiding this comment.
Stale comment — NodeId doc-comment now accurately describes the actual format ("" for root, "0", "1" etc., assigned depth-first by NodeIdHelper). See XamlNodeDiff.cs:58-61. The {TypeName}_{depth}_{index} format mentioned was from an earlier prototype.
StephaneDelcroix
force-pushed
the
feature/xaml-incremental-hotreload
branch
from
8f1a3cc to
459e08c
Compare
StephaneDelcroix
force-pushed
the
feature/xaml-incremental-hotreload
branch
2 times, most recently
from
1e2557d to
9c34d31
Compare
Contributor
|
/review -b feature/refactor-copilot-yml |
Contributor
AI code review for net11.0 targetVerdict: Needs discussion (draft/WIP; non-approval automated review, no human approval implied) Large feature PR (XAML Incremental Hot Reload via the source generator) by the XAML/SourceGen area owner. ~45 files including a new diff/codegen pipeline, runtime Observations:
CI: required pipelines red, expected for a WIP draft; not assessed as merge-ready. Confidence: medium and intentionally high-level — main pre-merge items are the registry lifetime/leak question and final API review. |
StephaneDelcroix
force-pushed
the
feature/xaml-incremental-hotreload
branch
from
bc39b90 to
4e24d57
Compare
Add the foundational components for XAML Incremental Hot Reload (XIHR): - XamlNodeDiff: source-generator-side diff engine that compares old vs. new SGRootNode trees and produces a descriptive diff (property changes, child add/remove, structural changes) without any codegen concerns. - XamlComponentRegistry: runtime registry mapping component types to live instances and node IDs, enabling the MetadataUpdateHandler to locate the right instances when XAML changes. - NodeIdHelper: assigns stable IDs to XAML nodes. - InitializeComponent codegen: emits __version field and registers the component instance with XamlComponentRegistry. - UpdateComponentCodeWriter: generates an UpdateComponent(diff, version) partial method that applies a diff to a live instance. - Pipeline wiring (XamlGenerator/XamlHotReloadState), MetadataUpdateHandler glue codegen, and the static XamlIncrementalHotReloadHandler SDK class. - Comprehensive unit and pipeline tests covering diff/IC/UC. Documents the trim/AOT limitation on the generated handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend XamlNodeDiff and UpdateComponent codegen to handle structural changes in the XAML tree: - Same-parent child reordering. - Child add and remove operations. - Same-type sibling matching for stable identity across reorders. - Refactor node IDs from position-based paths to stable unique integers so renames/reorders don't invalidate identity. - Single UpdateComponent() per component (per spec), replacing the earlier per-node versioning scheme. - Cache parsed SGRootNode to avoid re-parsing old XAML against new compilation each generation. - Make the diff engine purely descriptive (no codegen-driven fallback). - Fix DiffProperties for Value ↔ Binding ↔ StaticResource ↔ DynamicResource transitions. - Add ToDebugString diff verification and property transition matrix tests. Includes multiple rounds of review fixes (UnregisterSubtree, Layout checks, prefix collisions, UC fallback alignment, ID test type safety). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a sandbox app that exercises the XAML Incremental Hot Reload pipeline end-to-end. Adds the EnableMauiIncrementalHotReload CompilerVisibleProperty so the source generator can opt-in based on project metadata. Fixes a duplicate GeneratedCodeAttribute on the generated UpdateComponent partial class. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add full support for markup-driven properties in UpdateComponent:
- MarkupNode handling: {Binding}, {StaticResource}, {DynamicResource},
C# expressions.
- {Binding Path, Mode=…, StringFormat=…} parameter support.
- Reuse IC's ExpandMarkupsVisitor and compile-time type converters in
UC rather than maintaining a parallel parser/converter path.
- Reuse IC's markup-extension pipeline in UC, removing hand-coded
expression handlers.
- Compiled TypedBinding emission in UC when x:DataType is known.
- Incremental handling of x:DataType changes (no structural fallback).
- Root-level child changes and content-container child handling.
- Attached bindable property support (set/clear via attached owner type).
- Handle MarkupNode properties on newly emitted elements.
- Smart same-type sibling matching reused across markup paths.
Includes structural child-change test coverage (remove, reorder, nested,
replace, multi-add) and attached-bindable-property tests; fixes IC
pipeline namespace handling for attached properties.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wire the MetadataUpdateHandler into the MAUI SDK: - Enable MetadataUpdateHandler registration in the SDK targets. - Gate XIHR behind RuntimeFeature.IsIncrementalHotReloadEnabled so the runtime path is fully opt-in. - Marshal UpdateComponent invocations to the main thread. - Invalidate measure after UpdateComponent so visual refresh occurs. - Rewrite the handler to track live instances via WeakReference, avoiding leaks of XAML components after hot reload. - Move the MUH into the SDK and emit Track() calls from IC so each component registers itself with the runtime tracker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Support hot reload of ResourceDictionary entries in UpdateComponent: - Diff and codegen for resource add / remove / change. - Replace blanket Clear() with targeted resource key tracking so only the resources that actually changed are updated. - Track only emittable keys in UC (skip converters/custom types in the remove path; emit fresh instances for custom types on add). - IC registers initial resource keys so removals are correctly diffed. - Fix converter swap crash by emitting a runtime StaticResource lookup in UC for converter references. - Fix converter resource removal: register custom types in IC. - Fixes for CS0414 (__version), CS1061 (resource Contains), and CS0162 (unreachable code) in generated resource UC patch bodies. - E2E tests for resource hot reload scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
StephaneDelcroix
force-pushed
the
feature/xaml-incremental-hotreload
branch
from
09607a6 to
f276fc7
Compare
Contributor
AI code review refresh for net11.0 targetHead reviewed: Prior-review reconciliation (round-15 → now)The new commit
CI status (build 1457900)Mixed. Classification:
Blast radius
Findings summary
Confidence: medium-high on the test-failure diagnosis (read from actual Helix results) and on the registry resolution (read at head); medium on the AOT/trim reds (not deep-dived). Automated non-approval review. No human approval is implied or given; this does not gate merge. |
StephaneDelcroix
force-pushed
the
feature/xaml-incremental-hotreload
branch
3 times, most recently
from
2d9c193 to
7f0c8ee
Compare
StephaneDelcroix
changed the title
Contributor
AI code review refresh for net11.0 targetHead reviewed:
Prior‑review reconciliation (round‑16 → now)
New since round‑16 (this commit) — spot review
CI status
Blast radius
Findings summary
Confidence: high on the Automated non‑approval review. No human approval is implied or given; this comment does not gate merge and uses neither approve nor request‑changes. |
…iew feedback
- Optimize MetadataUpdateHandler with a 2-level Type→instances table
for fast lookup; add Style/Trigger/VSM/Behavior coverage.
- Skip the ICRuntime structural fallback when XIHR is enabled — the
incremental path now covers the previously fallback-only cases.
- Binding cleanup, attached-property clear on remove, expression-binding
reuse, event handler unsubscribe to avoid zombie bindings/handlers.
- Code dedup and factory-context plumbing.
- Resource safety/validation fixes and null-forgiving guards.
Address multi-model review feedback (B1-B5, M1-M13):
Blockers:
- B1 (partial): include TargetFramework in XamlHotReloadState cache key
so per-TFM builds of the same assembly do not contaminate each other.
(True Roslyn IncrementalGenerator pure-function compliance is tracked
as a follow-up — requires IncrementalValueProvider re-architecture.)
- B2: align all 7 PublicAPI.Unshipped.txt files. Every TFM (except
netstandard) now ships the handler entries; every TFM ships the
resource-key API and the new FindStaticResource helper.
- B3: rewrite XamlGenerator's IHR entry so first-run seeding only fires
when the cache has no entry (hadPreviousEntry==false). Previously an
unrelated .cs edit could silently reset __version to 0 while patches
were retained, mis-aligning runtime and generated state.
- B4: revert the literal "x" namespace comparisons. XamlParser
.ParsePropertyName normalises every x: directive via XmlName.xName /
xKey / xClass / xFieldModifier / xTypeArguments / xDataType — all
constructed as new XmlName("x", "..."). The original code was
correct; documented inline to prevent the same false-positive.
- B5: refactor UpdateComponentCodeWriter so each TryGet probe wraps its
work in 'if (TryGet) { ... }' instead of 'if (!TryGet) return;'. A
missing node now skips a single change and __version still advances;
previously the whole UpdateComponent() short-circuited and the page
was stranded at the old version, replaying the same failure forever.
Majors:
- M1: tag RuntimeFeature.IsIncrementalHotReloadEnabled with
[FeatureGuard(RequiresUnreferencedCode/RequiresDynamicCode)] so the
IL linker can prove the gated paths unreachable when XIHR is off.
- M2: emit "\n" (not RuntimeInformation-conditional) and sort
HashSet enumerations before codegen so generator output is bit-
deterministic across OS hosts (preserves Roslyn output caching).
- M3: tag XamlComponentRegistry [EditorBrowsable(Never)].
- M4: use SymbolDisplay.FormatLiteral for resource-key emission to
prevent injection via crafted keys.
- M6: resolve the synthetic "_Content" diff key to the type's
[ContentProperty] target before emission (root + child sites).
- M7: detect x:DataType change on the node itself (not only ancestors)
so bindings on the same node refresh when their type context shifts.
- M8: emit XamlComponentRegistry.FindStaticResource(this, key) for
StaticResource converter swaps. Walks the parent chain → Application
→ system resources, matching StaticResourceExtension.ProvideValue
semantics. Previously this.Resources[key] only checked the page-
level dictionary.
- M9: batch UpdateApplication's main-thread dispatches into a single
BeginInvokeOnMainThread call per metadata update.
- M10: drop the handler's parallel weak-reference list.
UpdateApplication now queries XamlComponentRegistry.GetInstances(type)
directly, eliminating the 'register here / register there' double-
bookkeeping. Track() is kept as a no-op for backward compatibility
with already-compiled IC bodies.
- M11: ArgumentNullException guard on Track(null).
- M12: serialize RegisterResourceKeys' Remove+Add against the
ConditionalWeakTable so concurrent registrations on the same page
cannot race (netstandard2.0 has no AddOrUpdate).
- M13: optimise pure reorders to RemoveAt + Insert (preserves handler /
animation / focus state of retained children). Falls back to Clear +
re-Add when the change involves any Add or Remove.
All 413 SourceGen unit tests pass. 26/26 XamlComponentRegistry +
XamlIncrementalHotReload Core.UnitTests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Second-round re-review fixes:
- M8 (correctness): emit FindStaticResource(parentAccessor, key) — the
target element of the markup-extension property assignment — instead
of FindStaticResource(this, ...). Walking from `this` (the page)
skipped any ResourceDictionary scoped on intermediate parents
(e.g., a ContentView's own <Resources>), causing valid StaticResource
references to silently miss during hot reload.
- M4 (completion): replace UpdateComponentCodeWriter.EscapeString's
hand-rolled C-escape table with Microsoft.CodeAnalysis.CSharp
.SymbolDisplay.FormatLiteral. The previous implementation missed
C0 control chars outside the named set (\u0001-\u0006, \u000e-\u001f,
etc.), which would have produced invalid C# string literals for
XAML values containing them.
- NEW (AOT): remove `((dynamic)parent).Content = ...` from
EmitContentPropertyChange. The synthesised dynamic call requires
Microsoft.CSharp and is incompatible with NativeAOT / full trim.
Pass the parent type symbol and emit a typed cast; fall back to
dynamic only if type resolution failed (preserved behaviour).
StephaneDelcroix
force-pushed
the
feature/xaml-incremental-hotreload
branch
from
7f0c8ee to
b260645
Compare
Contributor
AI code review refresh for net11.0 targetHead reviewed: Verdict: Needs changes — the new commit ( Prior-review reconciliation (round 17 → now)
New since round 17 (spot review of
|
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.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
This PR adds XAML Incremental Hot Reload (XIHR) — a Roslyn source generator + runtime feature that lets live XAML edits update existing instances without rebuilding the app. When the developer edits a XAML file, the generator emits a per-version patch method (
UpdateComponent) and the[MetadataUpdateHandler]runtime applies it to every live instance of the affected page, advancing each instance through its accumulated patch chain.The feature is opt-in via project property (
<EnableIncrementalHotReload>) and gated by aRuntimeFeatureswitch (IsIncrementalHotReloadEnabled) so trimmed/AOT production builds pay zero cost.What changed
Source generator (
src/Controls/src/SourceGen/):XamlGeneratororchestrates per-file IC + UC emission with versioning.InitializeComponentCodeWriteremits the initial__versionfield andRegister(...)calls.UpdateComponentCodeWriteremits per-version patch bodies (if (__version == N) { …; __version = N+1; }), supporting property changes, child add/remove, structural reorders, attached properties, markup extensions, bindings, and ResourceDictionary updates.XamlNodeDiffcomputes the semantic diff used to decide patch vs. structural reset vs. empty (no-op) diff.XamlHotReloadStatekeeps the per-file(prev XAML, parsed tree, node IDs, version, patch bodies)cache across generator invocations.Runtime (
src/Controls/src/Xaml/):XamlComponentRegistrytracks live instances + named components per page viaConditionalWeakTableand weak references.XamlIncrementalHotReloadHandler([assembly: MetadataUpdateHandler]) snapshots the registry on a metadata update, then dispatchesUpdateComponent()calls on the UI thread.Feature switch:
RuntimeFeature.IsIncrementalHotReloadEnabledwith[FeatureSwitchDefinition]+[FeatureGuard]so the trimmer can dead-strip the runtime when disabled.Sample:
Maui.Controls.Sample.Sandboxdemonstrates the developer scenario.Tests
XamlComponentRegistry(registration lifecycle, weak-ref cleanup, prefix rename).NoOpEdit_BetweenPatches_PreservesVersionChain— semantic no-op edits (e.g., adding a comment) must NOT reset version or clear accumulated patches.Review history
This branch went through 5 rounds of multi-model code review (Claude Sonnet 4.6, Claude Opus 4.7, GPT-5.5 in parallel). 7 round-1 blockers + 13 majors + 3 round-2 findings + 7 round-3 findings + 4 round-4 findings were all addressed. Round-5 verdict: LGTM from all 3 reviewers, high confidence.
Known follow-ups (not blockers for this PR)
XamlHotReloadState) keyed by(assembly, TFM, relativePath). A future refactor to pureIncrementalValueProviderpipelines would reduce coupling and improve build-server cacheability.XamlHotReloadStateaccumulates one entry per XAML file ever seen by the generator host. For multi-hour IDE sessions with file renames or deletes, entries are never reclaimed until generator-host shutdown. Adding pruning from the currentAdditionalTextssnapshot is a small follow-up.Targeting
Base branch:
net11.0(this is a new feature, not a bug fix).