Refactor in app HR handling #22009

dr1rrb · 2025-12-01T18:55:25Z

✨ Feature

Fix broken HR UI updates pausing
Refactor UI update code to make it readable

What is the current behavior? 🤔

UI pause is achieved using unrelated TypeMappings class
Visual tree enumeration is achieved by calling unexpected ToArray

What is the new behavior? 🚀

Dedicated class for UI pause, with explicit resume phase
Use proper foreach

PR Checklist ✅

📝 Commits must be following the Conventional Commits specification.
🧪 Added Runtime tests, UI tests, or a manual test sample for the changes have been added (for bug fixes / features) (if applicable)
📚 Docs have been added/updated which fit documentation template (for bug fixes / features)
🖼️ Validated PR Screenshots Compare Test Run results.
❗ Contains NO breaking changes

dr1rrb · 2025-12-01T18:59:43Z

Note: This is a draft PR as the final goal is to split the "Dev Server client" from the "Hot Reload" logic as they are no longer deeply inter-dependent.

Copilot

Pull request overview

This PR refactors the hot reload UI update pause/resume functionality by extracting it from the TypeMappings class into a dedicated HotReloadService class. It also simplifies visual tree enumeration from async to synchronous iteration, improving code readability and maintainability.

Key Changes:

Introduced HotReloadService with a disposable UIUpdatePauseHandle pattern for pausing/resuming UI updates
Refactored TypeMappings to use ImmutableDictionary for thread-safe type mapping storage
Simplified visual tree enumeration from async IAsyncEnumerable to synchronous IEnumerable with direct foreach loops

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
HotReloadService.cs	New dedicated service for managing hot-reload UI update pausing with disposable handle pattern
ElementUpdateHandlerCollection.cs	New collection class for efficiently managing and caching element update handlers
TypeMappings.cs	Refactored to use ImmutableDictionary and removed pause/resume logic (moved to HotReloadService)
ClientHotReloadProcessor.cs	Added singleton initialization and HotReloadService integration
ClientHotReloadProcessor.Status.cs	Made internal members accessible and added optional source parameter
ClientHotReloadProcessor.MetadataUpdate.cs	Major refactoring: multi-window support, synchronous tree iteration, improved error handling
ClientHotReloadProcessor.Common.cs	Simplified visual tree enumeration to synchronous iteration
RemoteControlExtensions.cs	Updated to use HotReloadService.IsUIUpdatePaused instead of TypeMappings
Given_TextBlock.cs	Updated test to use new PauseUIUpdates() disposable pattern
Given_Frame.cs	Updated tests to use new PauseUIUpdates() disposable pattern
BaseTestClass.cs	Removed TypeMappings.ClearMappings() and TypeMappings.Resume() calls from test initialization

Copilot · 2025-12-01T19:06:28Z

src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs

+			if (windows is null or { Count: 0 })
 			{
-				if (_log.IsEnabled(LogLevel.Error))
+				var errorMsg = "Cannot update UI as no window has been found. Make sure you have enabled hot-reload (Window.UseStudio()) in app startup. See https://aka.platform.uno/hot-reloa";


Typo in URL: "hot-reloa" appears to be truncated and should be "hot-reload".

Suggested change

var errorMsg = "Cannot update UI as no window has been found. Make sure you have enabled hot-reload (Window.UseStudio()) in app startup. See https://aka.platform.uno/hot-reloa";

var errorMsg = "Cannot update UI as no window has been found. Make sure you have enabled hot-reload (Window.UseStudio()) in app startup. See https://aka.platform.uno/hot-reload";

Copilot · 2025-12-01T19:06:28Z

src/Uno.UI.RemoteControl/HotReload/HotReloadService.cs

+		UIUpdated = 200,
+
+		/// <summary>
+		/// Indicates that the UI ahs not been updated to latest changes because another process is holding the updates.


Typo in documentation: "ahs" should be "has".

Suggested change

/// Indicates that the UI ahs not been updated to latest changes because another process is holding the updates.

/// Indicates that the UI has not been updated to latest changes because another process is holding the updates.

Copilot · 2025-12-01T19:06:29Z

src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs


-					// Invoke initializers so default types and other resources get updated.
-					registerResourceDictionariesBySourceLocalMethod.Invoke(null, null);
+			// This is needed for the head only (causes some extra invalid registration ins the ResourceResolver, but it has no negative impact)


Missing space in comment: "Ins" should be "in". The comment says "extra invalid registration ins the ResourceResolver" but should read "extra invalid registration in the ResourceResolver".

Suggested change

// This is needed for the head only (causes some extra invalid registration ins the ResourceResolver, but it has no negative impact)

// This is needed for the head only (causes some extra invalid registration in the ResourceResolver, but it has no negative impact)

Copilot · 2025-12-01T19:06:29Z

src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs

 using static Uno.UI.RemoteControl.HotReload.MetadataUpdater.ElementUpdateAgent;
+using Microsoft.VisualBasic;
+using static Microsoft.UI.Xaml.Controls.CollectionChangedOperation;
+using System.Xml.Linq;


Missing System.Xml.Linq namespace: The using statement for System.Xml.Linq at line 28 appears to be unused. If it's not needed, it should be removed for code cleanliness.

Suggested change

using System.Xml.Linq;

Copilot · 2025-12-01T19:06:29Z

src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs

+#if HAS_UNO
+		var windows = ApplicationHelper.Windows;
+#else
+		IReadOnlyList<Window> windows = CurrentWindow is { } cw ? [cw] : [];


Inconsistent conditional compilation: At line 91, the #else branch references CurrentWindow which is undefined. The field was renamed to _mainWindow at line 43, but this reference wasn't updated. This will cause a compilation error on non-HAS_UNO platforms.

Suggested change

IReadOnlyList<Window> windows = CurrentWindow is { } cw ? [cw] : [];

IReadOnlyList<Window> windows = _mainWindow is { } cw ? [cw] : [];

Copilot · 2025-12-01T19:06:31Z

src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.Common.cs

+#if __APPLE_UIKIT__
+using UIKit;
+using _View = UIKit.UIView;


Duplicate conditional compilation directives: The #if __APPLE_UIKIT__ block for UIKit imports appears twice (lines 23-24 and lines 29-31). The second block should be removed as it's redundant.

Copilot · 2025-12-01T19:06:31Z

src/Uno.UI.RuntimeTests/Tests/HotReload/Frame/HRApp/Tests/BaseTestClass.cs

 	public void InitHotReload()
 	{
-		TypeMappings.ClearMappings();
-
-		// Make sure type mappings is running
-		TypeMappings.Resume();
 	}


[nitpick] Consider test isolation: The TypeMappings.ClearMappings() call was removed from test initialization. With the new ImmutableDictionary approach, type mappings will persist across tests, which could lead to unexpected test interactions. Consider whether this is intentional or if a cleanup mechanism is still needed for test isolation.

Copilot · 2025-12-01T19:06:31Z

src/Uno.UI.RemoteControl/HotReload/HotReloadService.cs

+	internal static void Init(ClientHotReloadProcessor processor)
+	{
+		_instance = new(processor);
+	}


Missing documentation: The Init method is marked as internal and is a critical initialization point for the service, but it lacks XML documentation. Consider adding documentation to explain when and how this method should be called, especially given the comment at line 35 about replacing it with a module initializer.

Copilot · 2025-12-01T19:06:31Z

src/Uno.UI.RemoteControl/HotReload/ClientHotReloadProcessor.MetadataUpdate.cs

+using Microsoft.VisualBasic;
+using static Microsoft.UI.Xaml.Controls.CollectionChangedOperation;


Unnecessary using statements: Microsoft.VisualBasic and static Microsoft.UI.Xaml.Controls.CollectionChangedOperation are imported but never used in this file. These should be removed to keep the code clean.

Suggested change

using Microsoft.VisualBasic;

using static Microsoft.UI.Xaml.Controls.CollectionChangedOperation;

Copilot · 2025-12-01T19:06:32Z

src/Uno.UI.RemoteControl/HotReload/HotReloadService.cs

+
+	#region Legacy temp compat members
+	// For now, this is initialized when the ClientHRProcessor initializes.
+	// But this should be replaced ny a module initializer in the main application.


Typo in comment: "ny" should be "by" in the comment "But this should be replaced ny a module initializer".

Suggested change

// But this should be replaced ny a module initializer in the main application.

// But this should be replaced by a module initializer in the main application.

unodevops · 2025-12-01T19:18:27Z

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-22009/docs/index.html

unodevops · 2025-12-01T19:21:26Z

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-22009/wasm-skia-net9/index.html

carldebilly

LGTM

dr1rrb added 2 commits December 1, 2025 13:40

chore: Remove UI related code from TypeMappings

ed43857

chore: Refactor UI update logic to make it readable

10ba331

Copilot AI review requested due to automatic review settings December 1, 2025 18:55

Copilot started reviewing on behalf of dr1rrb December 1, 2025 18:55 View session

chore: Adjust public API

603a5c2

dr1rrb changed the title ~~Dev/dr/hr info2~~ Refactor in app HR handling Dec 1, 2025

dr1rrb marked this pull request as draft December 1, 2025 18:58

Copilot finished reviewing on behalf of dr1rrb December 1, 2025 18:59

Copilot AI reviewed Dec 1, 2025

View reviewed changes

carldebilly reviewed Dec 1, 2025

View reviewed changes

	var errorMsg = "Cannot update UI as no window has been found. Make sure you have enabled hot-reload (Window.UseStudio()) in app startup. See https://aka.platform.uno/hot-reloa";
	var errorMsg = "Cannot update UI as no window has been found. Make sure you have enabled hot-reload (Window.UseStudio()) in app startup. See https://aka.platform.uno/hot-reload";

	/// Indicates that the UI ahs not been updated to latest changes because another process is holding the updates.
	/// Indicates that the UI has not been updated to latest changes because another process is holding the updates.

	// This is needed for the head only (causes some extra invalid registration ins the ResourceResolver, but it has no negative impact)
	// This is needed for the head only (causes some extra invalid registration in the ResourceResolver, but it has no negative impact)

	IReadOnlyList<Window> windows = CurrentWindow is { } cw ? [cw] : [];
	IReadOnlyList<Window> windows = _mainWindow is { } cw ? [cw] : [];

		using Microsoft.VisualBasic;
		using static Microsoft.UI.Xaml.Controls.CollectionChangedOperation;

	// But this should be replaced ny a module initializer in the main application.
	// But this should be replaced by a module initializer in the main application.

Refactor in app HR handling #22009

Are you sure you want to change the base?

Refactor in app HR handling #22009

Conversation

dr1rrb commented Dec 1, 2025

✨ Feature

What is the current behavior? 🤔

What is the new behavior? 🚀

PR Checklist ✅

Uh oh!

dr1rrb commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

unodevops commented Dec 1, 2025

Uh oh!

unodevops commented Dec 1, 2025

Uh oh!

carldebilly left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants