[XSG] Fix XAML SourceGenerator reusing modified node tree #32870
Conversation
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
StephaneDelcroix
force-pushed
the
dev/stdelc/xsg-c#hr
branch
from
cfb5f32 to
c709ea8
Compare
StephaneDelcroix
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a hot reload bug where the XAML node tree was being reused across source generator invocations, causing incorrect behavior when the tree was modified during processing. The solution stores raw XAML strings instead of parsed node trees and parses on-demand during code generation.
Key Changes
- Replaced
SGRootNodecaching with raw XAML string storage inXamlProjectItemForIC - Renamed and enhanced
CompilationReferencesComparertoCompilationComparerto detect both reference and syntax tree changes - Parse XAML on-demand in
InitializeComponentCodeWriterinstead of reusing cached parse trees - Added comprehensive multi-file hot reload tests in
XTypeMultiFileHotReloadTests
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/src/SourceGen/XamlProjectItemForIC.cs | Changed from storing parsed SGRootNode to storing raw XAML string |
| src/Controls/src/SourceGen/CompilationReferencesComparer.cs | Renamed to CompilationComparer and added syntax tree comparison logic |
| src/Controls/src/SourceGen/XamlGenerator.cs | Updated to use CompilationComparer and renamed tracking provider |
| src/Controls/src/SourceGen/TrackingNames.cs | Renamed ReferenceCompilationProvider to CompilationProvider |
| src/Controls/src/SourceGen/InitializeComponentCodeWriter.cs | Parse XAML on-demand using GeneratorHelpers.ParseXaml |
| src/Controls/src/SourceGen/GeneratorHelpers.cs | Changed to return XAML string instead of parsed tree, made ParseXaml public |
| src/Controls/tests/SourceGen.UnitTests/SourceGenXamlCodeBehindTests.cs | Updated test expectations to use new CompilationProvider tracking name |
| src/Controls/tests/SourceGen.UnitTests/XTypeMultiFileHotReloadTests.cs | New comprehensive test suite for multi-file hot reload scenarios |
src/Controls/tests/SourceGen.UnitTests/XTypeMultiFileHotReloadTests.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/SourceGen.UnitTests/XTypeMultiFileHotReloadTests.cs
Outdated
Show resolved
Hide resolved
StephaneDelcroix
force-pushed
the
dev/stdelc/xsg-c#hr
branch
2 times, most recently
from
ade43b7 to
747bbcb
Compare
- Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean)
StephaneDelcroix
force-pushed
the
dev/stdelc/xsg-c#hr
branch
from
747bbcb to
01f655e
Compare
StephaneDelcroix
added
area-xaml
StephaneDelcroix
changed the title
simonrozsival
approved these changes
Nov 27, 2025
StephaneDelcroix
added a commit
that referenced
this pull request
Dec 2, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
PureWeen
added a commit
that referenced
this pull request
Dec 3, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
PureWeen
added a commit
that referenced
this pull request
Dec 3, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
PureWeen
added a commit
that referenced
this pull request
Dec 3, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
PureWeen
added a commit
that referenced
this pull request
Dec 3, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
PureWeen
added a commit
that referenced
this pull request
Dec 3, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
github-actions bot
pushed a commit
that referenced
this pull request
Dec 5, 2025
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <[email protected]> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <[email protected]> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <[email protected]> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <[email protected]> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <[email protected]> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <[email protected]> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <[email protected]> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <[email protected]> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <[email protected]> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <[email protected]> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <[email protected]> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <[email protected]> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <[email protected]> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <[email protected]> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <[email protected]> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <[email protected]> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <[email protected]> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <[email protected]> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <[email protected]> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <[email protected]> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <[email protected]> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <[email protected]> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <[email protected]> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <[email protected]> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <[email protected]> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <[email protected]> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <[email protected]> * Fix documentation typos Co-authored-by: PureWeen <[email protected]> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <[email protected]> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: PureWeen <[email protected]> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: PureWeen <[email protected]> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <[email protected]> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <[email protected]> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <[email protected]> * nullability --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Simon Rozsival <[email protected]> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <[email protected]> Co-authored-by: Stephane Delcroix <[email protected]> Co-authored-by: Shane Neuville <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: simonrozsival <[email protected]> Co-authored-by: Šimon Rozsíval <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Gerald Versluis <[email protected]> Co-authored-by: PureWeen <[email protected]> Co-authored-by: jfversluis <[email protected]> Co-authored-by: Simon Rozsival <[email protected]>
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
Fixes an issue where the XAML node tree was being reused across source generator invocations. Since the tree can be modified during processing, reusing it caused incorrect behavior in C# hot reload scenarios.
Changes
Performance Impact
No significant performance regression:
(10 runs each, outliers removed, building Xaml.UnitTests)