build: update CsWinRT to 2.3.0-prerelease + apply OSClient defaults#199
Draft
jevansaks wants to merge 1 commit into
Draft
build: update CsWinRT to 2.3.0-prerelease + apply OSClient defaults#199jevansaks wants to merge 1 commit into
jevansaks wants to merge 1 commit into
Conversation
Pin Microsoft.Windows.CsWinRT to 2.3.0-prerelease.251115.2 (latest 2.3.x;
no stable 2.3 yet) so projects no longer pick up an old transitive 2.0.x
from WindowsAppSDK 2.0.1.
Apply the subset of CsWinRT defaults from OSClient's UES.Common.CSharp.SDK.props
that make sense for a pure WinAppSDK repo (no UWP, no OS XAML mixing, no
AOT-merged WinRT components):
CsWinRTEnableLogging=true — verbose CsWinRT diagnostics
CsWinRTIIDOptimizerOptOut=true — we don't author projections
CsWinRTUseEnvironmentalTools=false — match CI/lab toolset
CsWinRTLoadComponentsInDefaultALC=true — JIT test/sample apps
Properties live in Directory.Build.props gated on UseWinUI=true; the package
reference is added in Directory.Build.targets behind the same condition with
PrivateAssets=all (CsWinRT is a build-time generator).
Cleanup driven by the upgrade:
- src/Reactor/Charting/Charts*.cs (5 sites): cast IReadOnlyList<T> collection
expressions to T[] for AOT/trimming compatibility (CsWinRT1032).
- samples/TodoApp + ReactorCharting.Gallery + Reactor.TestApp: same fix at
the call sites that hand collection literals to APIs taking
IReadOnlyList<T>/IEnumerable<T>.
- src/Reactor/Hosting/ReactorHostControl.cs: pre-existing CS8604 (unsafe
_logger.LogWarning) made null-conditional like every other call site.
tests/Reactor.Tests + tests/Reactor.AppTests.Host: NoWarn CsWinRT1032/1033
with a comment. Test code calls managed Reactor APIs directly (no WinRT
projection ABI) and isn't AOT-published, so the warnings are noise.
Build: 0 errors, 17 warnings (all pre-existing — REACTOR_A11Y demos in the
a11y-showcase sample, NU1903 transitively from GitHub.Copilot.SDK).
Reactor.Tests: 6820 pass / 1 pre-existing fail (FormatDate_Short, ICU locale
drift on .NET 10, fails on main without these changes too).
Reactor.SelfTests: 671 / 671 pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Pin
Microsoft.Windows.CsWinRTto2.3.0-prerelease.251115.2(latest 2.3.x; no stable 2.3 released yet) and apply the subset of CsWinRT defaults from OSClient'sUES.Common.CSharp.SDK.propsthat make sense for a pure WinAppSDK repo.Before this change, the repo picked up CsWinRT only transitively from
Microsoft.WindowsAppSDK 2.0.1, which dragged in an old 2.0.x build and missed two years of generator/analyzer improvements.Why
Directory.Build.propsand every WinUI project picks it up automatically.CsWinRT defaults applied
Centralized in
Directory.Build.props, gated onUseWinUI=true:CsWinRTEnableLoggingtrueCsWinRTIIDOptimizerOptOuttrueCsWinRTUseEnvironmentalToolsfalseCsWinRTLoadComponentsInDefaultALCtrueThe
Microsoft.Windows.CsWinRTPackageReferencelives inDirectory.Build.targetsbehind the sameUseWinUI=truecondition, withPrivateAssets="all"because it's a build-time generator.Defaults intentionally not applied (with rationale in
plan.md)ReportAnalyzer=true— analyzer-timing reports are noisy/expensive for a smaller repo.CsWinRTGenerateManagedDllGetActivationFactory=true— only useful for AOT-merged WinRT components; nothing here authors WinRT components.UseUwp/CsWinRTUseWindowsUIXamlProjections=false/EnableUnsafeMixedMicrosoftWindowsUIXamlProjections=truecluster — these turn on UWP/OS-XAML mixing, which is OSClient-specific. We're a pure WinAppSDK repo.DisableRuntimeMarshalling=true— wide-blast-radius assembly attribute, separate work.Cleanup driven by the upgrade
CsWinRT 2.3 introduces
CsWinRT1032(collection literals targeting non-mutable WinRT-projected interfaces aren't AOT-safe) andCsWinRT1033([ComImport] casts aren't compatible with CsWinRT/AOT). 144 new warnings appeared.Product code (5 sites, fixed properly)
src/Reactor/Charting/Charts.cs(3)src/Reactor/Charting/Charts.Tree.cs(2)These implement
IReadOnlyList<ChartSeriesDescriptor>/IReadOnlyList<ChartAxisDescriptor>properties returning collection expressions. Cast toT[]so the analyzer knows the concrete type.Sample code (~10 sites, fixed properly)
samples/TodoApp/Program.cs,samples/Reactor.TestApp, foursamples/ReactorCharting.Gallery/Samples/*.csfiles.Same
(T[])[...]cast pattern at the call sites.Test code (
tests/Reactor.Tests,tests/Reactor.AppTests.Host): suppressedCsWinRT1032/CsWinRT1033with rationale commentThese tests:
So the warnings don't apply, and rewriting 100+ test sites with
(T[])[...]casts is significant churn for zero runtime benefit. The suppression is documented in each.csprojnext to theNoWarnline.Pre-existing baseline issue fixed
src/Reactor/Hosting/ReactorHostControl.cs:360—_logger.LogWarning(...)(CS8604) made null-conditional like every other call site in the file.Build state
main)Remaining 17 warnings are all pre-existing:
REACTOR_A11Y_001/002/003insamples/apps/a11y-showcase— intentional analyzer-demo (per code comments)NU1903Nerdbank.MessagePack 1.0.2 vulnerability — transitive fromGitHub.Copilot.SDK 0.1.32, out of scopeTest plan
dotnet build Reactor.sln— 0 errors, 17 warnings (all pre-existing)dotnet test tests/Reactor.Tests— 6820 pass, 1 fail, 46 skippedIntlAccessorTests.FormatDate_Short_ReturnsShortDate) is pre-existing onmain:.NET 10ICU returns"2026-01-15"foren-USshort date format, the test still expects"1/15/2026". Same root cause as PR fix: locale and environment-sensitive test failures #169. Verified bygit stash+ re-run.dotnet test tests/Reactor.SelfTests— 671/671 passtests/Reactor.AppTests) not run — requires WinAppDriver, not part of the standard PR loop.Risk / breaking changes
.csprojfiles only.