From 65f686a987580821d4b714c2d7f37afd03223871 Mon Sep 17 00:00:00 2001 From: Copilot App <223556219+Copilot@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:50:58 -0700 Subject: [PATCH 1/2] Build Reactor's DataTemplates from code instead of parsing markup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reactor builds two DataTemplates at runtime by handing XamlReader.Load a markup string: the ContentControl shell every items control mounts (ListView/GridView/FlipView), and the legacy text-node TreeView template. Both exist only because DataTemplate could not be constructed from code. microsoft-ui-xaml adds `new DataTemplate(DataTemplateElementFactory)`, which takes a callback returning the subtree. This ports both templates onto it. Beyond removing a runtime XAML parse, the TreeView template loses its `{Binding Content.Content}`. A classic Binding resolves that path by string through CsWinRT's ICustomPropertyProvider — reflection, which NativeAOT trims unless the source type is annotated (that annotation is what #1108 added), and which costs a reflective lookup per realized row even when it works. Reading the same chain in a DataContextChanged handler is strongly typed: no annotation needed, no lookup, and the event re-fires on recycling so reused rows retext correctly. The `[WinRT.GeneratedBindableCustomProperty]` attribute on TreeViewNodeData is deliberately left in place. Reactor no longer needs it, but it is public API surface that consumers binding to TreeViewNodeData themselves still rely on. DO NOT MERGE YET. The constructor is gated behind `[feature(Feature_ExperimentalApi)]` in microsoft-ui-xaml and has not shipped in a public Windows App SDK, so CI cannot restore this branch: the versions in Directory.Build.props are an internal experimental build and do not resolve from public feeds. When the API ships, update those two versions and drop the metapackage opt-out in the selftest host; no other change should be needed. Validated against Microsoft.WindowsAppSDK.WinUI 3.0.0-experimental.260818.21 (the PR-built native core, Microsoft.ui.xaml.dll 3.3.0.2608): full selftest suite 1447 planned / 0 failures, with the text-node TreeView fixture green on both the mount and update arms. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17bce8bb-d57a-4f76-a55d-27c9029fc9a3 --- Directory.Build.props | 21 ++++++-- src/Reactor/Core/Reconciler.cs | 50 ++++++++++++++----- .../Reactor.AppTests.Host.csproj | 17 +++++++ 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 71781d5b5..03a61cdea 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -17,15 +17,28 @@ before any bootstrapper has run. --> - 2.1.3 + + 3.0.0-dev.experimental11 - 2.1.0 + 3.0.0-experimental.260818.21 1.4.0 false diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index cfecdea59..b217f6f54 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -334,27 +334,53 @@ internal bool TryResolveFromControlRegistry(Type elementType, out IV1HandlerEntr /// /// /// A shared DataTemplate containing a ContentControl shell. - /// Parsed once via XamlReader.Load, reused across all items controls (ListView, GridView, FlipView). + /// Built once from code via new DataTemplate(factory) and reused across + /// all items controls (ListView, GridView, FlipView). The factory overload + /// replaces a XamlReader.Load of an equivalent markup string — see the + /// PR description for why parsing markup at runtime was a problem here. /// internal static readonly Lazy SharedContentControlTemplate = new(() => - (DataTemplate)Microsoft.UI.Xaml.Markup.XamlReader.Load( - "" + - "" + - "")); + new DataTemplate(() => new Microsoft.UI.Xaml.Controls.ContentControl + { + HorizontalContentAlignment = Microsoft.UI.Xaml.HorizontalAlignment.Stretch, + VerticalContentAlignment = Microsoft.UI.Xaml.VerticalAlignment.Stretch, + })); /// /// Spec 047 §14 Phase 3 finish — text-bound TreeView item template /// shared between the legacy MountTreeView arm and the /// strategy. - /// In node mode the template's DataContext is TreeViewNode, so - /// {Binding Content.Content} resolves TreeViewNode.Content - /// (a TreeViewNodeData) → its Content (the display string). + /// In node mode the template's DataContext is TreeViewNode, so the + /// handler reads TreeViewNode.Content (a TreeViewNodeData) → + /// its Content (the display string). /// internal static readonly Lazy TreeViewTextItemTemplate = new(() => - (DataTemplate)Microsoft.UI.Xaml.Markup.XamlReader.Load( - "" + - "" + - "")); + new DataTemplate(() => + { + var tb = new Microsoft.UI.Xaml.Controls.TextBlock(); + + // Strongly-typed replacement for {Binding Content.Content}. A classic + // Binding resolves that path by string through CsWinRT's + // ICustomPropertyProvider, i.e. by reflection — which NativeAOT trims + // unless the source type is annotated, and which costs a reflective + // lookup per realized row even when it works. Reading the same chain in + // a DataContextChanged handler is reflection-free, so it needs no + // annotation and does no lookup. The event re-fires when a virtualized + // row is recycled onto new data, so reused rows retext correctly. + tb.DataContextChanged += static (sender, args) => + { + if (sender is not Microsoft.UI.Xaml.Controls.TextBlock text) return; + text.Text = args.NewValue switch + { + Microsoft.UI.Xaml.Controls.TreeViewNode { Content: TreeViewNodeData d } => d.Content, + Microsoft.UI.Xaml.Controls.TreeViewNode { Content: string s } => s, + TreeViewNodeData d => d.Content, + _ => string.Empty, + }; + }; + + return tb; + })); // ════════════════════════════════════════════════════════════════════ // ReactorAttached.StateProperty (ReactorState) diff --git a/tests/Reactor.AppTests.Host/Reactor.AppTests.Host.csproj b/tests/Reactor.AppTests.Host/Reactor.AppTests.Host.csproj index f0e11f4e0..996667709 100644 --- a/tests/Reactor.AppTests.Host/Reactor.AppTests.Host.csproj +++ b/tests/Reactor.AppTests.Host/Reactor.AppTests.Host.csproj @@ -13,6 +13,15 @@ true + + true + + + + + From 7b0195fc717b4137b3791daea95074f70c882073 Mon Sep 17 00:00:00 2001 From: Copilot App <223556219+Copilot@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:25:19 -0700 Subject: [PATCH 2/2] Drop the TreeViewNodeData binding-metadata attribute, now unused #1108 annotated TreeViewNodeData with [WinRT.GeneratedBindableCustomProperty] for exactly one reason: Reactor's own text-node TreeView template bound "Content.Content", and that string path is resolved by reflection, which NativeAOT trims. This branch replaces that binding with a DataContextChanged handler, so the reason is gone -- there is now no {Binding} anywhere in src/ and no SetBinding call site. The attribute never shipped: v0.1.0-preview.13 and every earlier tag predate #1108, so no released build has it, and binding to TreeViewNodeData from a consumer-supplied template was already broken under NativeAOT in all of them. Removing it therefore takes nothing away that ever worked in a release; it declines to keep a side effect of an internal fix. Keeping it would carry a workaround, plus `partial` on a public record, for a scenario Reactor does not use and does not document. The limitation is now written down instead: docs/aot-support.md gains a row explaining that a classic {Binding} in a hand-supplied DataTemplate is not trim-safe, that Reactor's own templates avoid it by populating content in a DataContextChanged handler, and that a consumer installing their own template via an escape hatch should do the same or annotate their own type. Also disambiguates the IElementFactory cref in IItemsRepeaterFactorySource. The Windows App SDK this branch targets adds Microsoft.UI.Xaml.IElementFactory alongside the existing Microsoft.UI.Xaml.Controls.IElementFactory, so the bare cref became ambiguous (CS0419) and failed the warnings-as-errors Release build. Points at the Controls one, which is what the member returns. Validated: src/Reactor Release build 0 warnings / 0 errors; selftest suite 1447 planned with only the known WindowLevel_RuntimeFlip_Topmost Z-order flake failing under full-suite load (passes 3/3 in isolation, unrelated to these files). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17bce8bb-d57a-4f76-a55d-27c9029fc9a3 --- docs/aot-support.md | 1 + src/Reactor/Core/Element.cs | 13 +------------ src/Reactor/Core/IItemsRepeaterFactorySource.cs | 2 +- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/docs/aot-support.md b/docs/aot-support.md index 0180d6f7a..217241b69 100644 --- a/docs/aot-support.md +++ b/docs/aot-support.md @@ -31,6 +31,7 @@ These subsystems compile cleanly with `IsAotCompatible=true` (the warnings are s | **DataGrid `AutoColumns`** | Reflects over `T`'s public properties via `TypeRegistry`. The `T` parameter is annotated `[DynamicallyAccessedMembers(PublicProperties)]` so the trimmer keeps the members, but `AutoColumns` ultimately funnels into `TypeRegistry.Resolve`, which is `RequiresUnreferencedCode`. Use explicit `Column(…)` definitions instead. | Issue #70 | | **`UseObservable` on POCOs** | `ObservableTreeTracker` walks public properties via reflection to subscribe to INPC. Observables built explicitly (`Observable`, `IObservableCollection`) are fine; the implicit-INPC path is not. | Issue #70 | | **Form validation** | `FormField`'s default editor resolution goes through `TypeRegistry`. Same caveat as PropertyGrid. | Issue #70 | +| **Classic `{Binding}` in a hand-supplied `DataTemplate`** | A `{Binding Some.Path}` resolves the path *by string* through CsWinRT's `ICustomPropertyProvider`, i.e. reflection, so the trimmer drops it and the target renders blank — with no build-time warning, because the reflection happens across the WinRT ABI where the IL analyzers cannot see it. Reactor itself contains no bindings: item templates are built from code and populated in a `DataContextChanged` handler, which is strongly typed and needs no metadata. If you install your own template through an escape hatch such as `TreeView(...).Set(tv => tv.ItemTemplate = ...)`, do the same — or annotate your data type with `[WinRT.GeneratedBindableCustomProperty]`. Note `{x:Bind}` is not an option from code: it is a XAML-compiler feature. | Issue #70 | | **Component discovery (`ReactorApp.Run` reflection paths)** | The instantiation of `TApp` itself is annotated and works. Devtools component enumeration is available only when the app opts into `Reactor.DevtoolsSupport` at build time and launches with `--devtools`; leave the switch off for retail/AOT builds. | Issue #70 | | **Devtools `properties` / `setProperty` DP discovery** | The tools locate `DependencyProperty` statics by reflection, and the trimmer keeps no reflection metadata for them unless something roots the members — so under AOT the lookups find nothing. Not specific to WinUI's CsWinRT-projected static properties: a C#-authored DP *field* on a custom control is equally undiscoverable. Works under JIT, which is where the devtools inner loop lives. The tool reports the likely cause rather than an empty result. See [Devtools DependencyProperty discovery](#devtools-dependencyproperty-discovery) for the measured cost of fixing it. Affected fixture: `Devtools_PropertyToolsDpDiscovery`. | Issue #1109 | | **Theme resource lookup (`Theme.X`, `ThemeRef.Resolve`)** | Works once the WindowsAppSDK#6394 workaround target ships the project `.pri` into the publish output (see [Required publish-time workarounds](#required-publish-time-workarounds)). Reactor's library itself is AOT-clean here. | WindowsAppSDK#6394 | diff --git a/src/Reactor/Core/Element.cs b/src/Reactor/Core/Element.cs index 7c5a5b934..a2d5ea496 100644 --- a/src/Reactor/Core/Element.cs +++ b/src/Reactor/Core/Element.cs @@ -2550,18 +2550,7 @@ public record PivotItemData(string Header, Element Content); public record BreadcrumbBarItemData(string Label, object? Tag = null); -// The legacy node-mode TreeView renders node text through a classic {Binding} -// (Reconciler.TreeViewTextItemTemplate resolves "Content.Content"). That path -// goes through CsWinRT's ICustomPropertyProvider, which is reflection-based and -// gets trimmed under NativeAOT — the TextBlock then silently renders empty, with -// no build-time warning. This attribute makes the CsWinRT source generator emit -// strongly-typed binding metadata for this type so the hop survives trimming. -// -// Scoped to just "Content": that is the only member the template binds, and the -// parameterless overload would also emit an accessor for the [Obsolete] -// ContentElement, which fails the warnings-as-errors Release build (CS0618). -[WinRT.GeneratedBindableCustomProperty(["Content"], [])] -public partial record TreeViewNodeData(string Content, TreeViewNodeData[]? Children = null) +public record TreeViewNodeData(string Content, TreeViewNodeData[]? Children = null) { public bool IsExpanded { get; init; } diff --git a/src/Reactor/Core/IItemsRepeaterFactorySource.cs b/src/Reactor/Core/IItemsRepeaterFactorySource.cs index 8eb42fec4..ed1d79a02 100644 --- a/src/Reactor/Core/IItemsRepeaterFactorySource.cs +++ b/src/Reactor/Core/IItemsRepeaterFactorySource.cs @@ -36,7 +36,7 @@ internal interface IItemsRepeaterFactorySource void ConfigureLayout(WinUI.ItemsRepeater repeater); /// - /// Produce a fresh closure that knows + /// Produce a fresh closure that knows /// how to realize element index N into a UIElement subtree. Called on /// first mount AND whenever the existing factory's type no longer /// matches (e.g. element re-keyed to a different TItem).