Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/Reactor/Core/HookOrderException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
namespace Microsoft.UI.Reactor.Core;

/// <summary>
/// Thrown when a component's hook call sequence diverges from the previous
/// render's sequence — either a hook is called at a different index, or the
/// hook type at a given index doesn't match what was previously declared.
///
/// In normal operation this is a programming bug (hooks must be called in
/// the same order every render). During Hot Reload, however, an edit that
/// reorders or changes hook types is the *expected* outcome of the user
/// editing their code — so the host catches this exception, runs cleanups
/// on the surviving RenderContext, resets its hook state, and re-renders.
/// State is lost (we cannot reliably re-key surviving hook values to a
/// shape that may have changed), but the app keeps running rather than
/// hard-failing the dev loop.
///
/// Derives from <see cref="InvalidOperationException"/> so existing
/// `catch (InvalidOperationException)` handlers around component render
/// continue to work; the host's render catch sniffs the more specific
/// type to gate the hot-reload recovery path.
/// </summary>
public sealed class HookOrderException : InvalidOperationException
{
public HookOrderException(string message) : base(message) { }
}
37 changes: 27 additions & 10 deletions src/Reactor/Core/RenderContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ internal void UseStateSetterByIndex<T>(int index, T newValue)
_hookIndex++;

if (_hooks[currentIndex] is not ValueHookState<T> hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {currentIndex} is {_hooks[currentIndex].GetType().Name}, expected ValueHookState<{typeof(T).Name}> (UseState). " +
"Hooks must be called in the same order every render.");
Comment on lines 72 to 75

Expand Down Expand Up @@ -129,7 +129,7 @@ void Setter(T newValue)
_hookIndex++;

if (_hooks[currentIndex] is not ValueHookState<T> hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {currentIndex} is {_hooks[currentIndex].GetType().Name}, expected ValueHookState<{typeof(T).Name}> (UseReducer). " +
"Hooks must be called in the same order every render.");

Expand Down Expand Up @@ -194,7 +194,7 @@ void Updater(Func<T, T> reducer)
_hookIndex++;

if (_hooks[currentIndex] is not ValueHookState<TState> hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {currentIndex} is {_hooks[currentIndex].GetType().Name}, expected ValueHookState<{typeof(TState).Name}> (UseReducer). " +
"Hooks must be called in the same order every render.");

Expand Down Expand Up @@ -248,7 +248,7 @@ public void UseEffect(Action effect, params object[] dependencies)
}

if (_hooks[_hookIndex] is not EffectHookState hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {_hookIndex} is {_hooks[_hookIndex].GetType().Name}, expected EffectHookState. " +
"Hooks must be called in the same order every render.");
_hookIndex++;
Expand All @@ -274,7 +274,7 @@ public void UseEffect(Func<Action> effectWithCleanup, params object[] dependenci
}

if (_hooks[_hookIndex] is not EffectHookState hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {_hookIndex} is {_hooks[_hookIndex].GetType().Name}, expected EffectHookState. " +
"Hooks must be called in the same order every render.");
_hookIndex++;
Expand All @@ -300,7 +300,7 @@ public T UseMemo<T>(Func<T> factory, params object[] dependencies)
}

if (_hooks[_hookIndex] is not MemoHookState<T> hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {_hookIndex} is {_hooks[_hookIndex].GetType().Name}, expected MemoHookState<{typeof(T).Name}>. " +
"Hooks must be called in the same order every render.");
_hookIndex++;
Expand Down Expand Up @@ -336,7 +336,7 @@ public Ref<T> UseRef<T>(T initialValue = default!)
_hookIndex++;

if (_hooks[currentIndex] is not ValueHookState<Ref<T>> hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {currentIndex} expected ValueHookState<Ref<{typeof(T).Name}>>, got {_hooks[currentIndex].GetType().Name}. " +
"Hooks must be called in the same order every render.");
return hook.Value;
Expand Down Expand Up @@ -386,7 +386,7 @@ public Ref<T> UseRef<T>(T initialValue = default!)
_hookIndex++;

if (_hooks[currentIndex] is not PersistedHookState<T> hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {currentIndex} is {_hooks[currentIndex].GetType().Name}, expected PersistedHookState<{typeof(T).Name}> (UsePersisted). " +
"Hooks must be called in the same order every render.");

Expand Down Expand Up @@ -593,7 +593,7 @@ public void UseNavigationLifecycle(
}

if (_hooks[_hookIndex] is not NavigationLifecycleHookState hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {_hookIndex} is {_hooks[_hookIndex].GetType().Name}, expected NavigationLifecycleHookState. " +
"Hooks must be called in the same order every render.");
_hookIndex++;
Expand Down Expand Up @@ -636,7 +636,7 @@ public T UseContext<T>(Context<T> context)
}

if (_hooks[_hookIndex] is not ContextHookState hook)
throw new InvalidOperationException(
throw new HookOrderException(
$"Hook at index {_hookIndex} is {_hooks[_hookIndex].GetType().Name}, expected ContextHookState (UseContext). " +
"Hooks must be called in the same order every render.");
_hookIndex++;
Expand Down Expand Up @@ -1033,6 +1033,23 @@ internal void RunCleanups()
}
}

/// <summary>
/// Hot Reload recovery: run effect cleanups and discard the entire hook
/// list so the next BeginRender starts with a fresh hook sequence. Used
/// by the host when a HookOrderException surfaces during a hot-reload-
/// triggered render — an edit that reorders or changes hook types is
/// the expected outcome of a developer change, so we trade away state
/// (which we cannot reliably re-key onto a new hook shape) to keep the
/// dev loop alive instead of leaving the user staring at an error
/// fallback.
/// </summary>
internal void ResetForHotReload()
{
RunCleanups();
_hooks.Clear();
_hookIndex = 0;
}

private static bool DepsEqual(object[] prev, object[] next)
{
if (prev.Length != next.Length) return false;
Expand Down
2 changes: 1 addition & 1 deletion src/Reactor/Hosting/Devtools/DevtoolsMenuFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static partial class Factories
/// component via <c>ctx.UseObservable(flag)</c>.
/// </summary>
public static Element DevtoolsMenu(
Func<IEnumerable<MenuFlyoutItemBase>> items,
Func<IEnumerable<MenuFlyoutItemBase>>? items = null,
string glyph = "⚡",
string toolTip = "Devtools",
string? automationId = null)
Expand Down
36 changes: 35 additions & 1 deletion src/Reactor/Hosting/HotReloadService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Reflection.Metadata;
using System.Threading;

[assembly: MetadataUpdateHandler(typeof(Microsoft.UI.Reactor.Hosting.HotReloadService))]

Expand All @@ -10,14 +11,47 @@ namespace Microsoft.UI.Reactor.Hosting;
/// UseState values survive because the RenderContext and its hooks list
/// remain in memory — only the Render() method body changes.
/// </summary>
static class HotReloadService
internal static class HotReloadService
{
// Underlying int storage (0 = false, 1 = true) so we can use
// Interlocked.Exchange for atomic capture-and-clear. .NET Hot Reload
// calls UpdateApplication from a runtime-controlled thread (typically
// the threadpool callback that just delivered the metadata update),
// while Render runs on the UI dispatcher. A non-atomic read-then-write
// would race: a second UpdateApplication firing between
// ReactorHostControl.Render's read and write of the flag could lose
// the new pending update. Exchange = single CAS, no window.
private static int _updatePending;

/// <summary>
/// True from when <see cref="UpdateApplication"/> sets it until
/// <see cref="ConsumeUpdatePending"/> atomically clears it (called by
/// the host at the start of each render attempt). When the consuming
/// render observes <c>true</c>, the host treats a
/// <see cref="Microsoft.UI.Reactor.Core.HookOrderException"/> raised
/// during that render as a hot-reload recovery trigger (run cleanups,
/// drop hook state, re-render) instead of escalating to the error
/// fallback.
/// </summary>
internal static bool UpdatePending => Volatile.Read(ref _updatePending) == 1;

/// <summary>
/// Atomically reads-and-clears <see cref="UpdatePending"/>. Returns
/// true exactly once per <see cref="UpdateApplication"/> call, even
/// if another <c>UpdateApplication</c> fires concurrently from the
/// hot-reload thread.
/// </summary>
internal static bool ConsumeUpdatePending() =>
Interlocked.Exchange(ref _updatePending, 0) == 1;

/// <summary>Called by the runtime to clear any caches of metadata.</summary>
public static void ClearCache(Type[]? updatedTypes) { }

/// <summary>Called after the metadata update is applied. Re-renders the UI.</summary>
public static void UpdateApplication(Type[]? updatedTypes)
{
Volatile.Write(ref _updatePending, 1);

// force: true bypasses component memo (Props/deps equality) for this
// pass — the updated Render() body would otherwise be skipped because
// props and hook deps haven't changed.
Expand Down
45 changes: 45 additions & 0 deletions src/Reactor/Hosting/ReactorHostControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,41 @@
private void Render()
{
_isRendering = true;
// Atomic capture-and-clear gives us at-most-once recovery per
// UpdateApplication call:
//
// UpdateApplication fires: UpdatePending = 1
// Render runs: ConsumeUpdatePending() → true
// (atomic Interlocked.Exchange to 0)
// hooks throw: recover + RequestRender, return
// Recovery render runs: ConsumeUpdatePending() → false
// hooks throw again: falls through `when (hotReloadRender)`
// filter → ShowErrorFallback
//
// A developer who has saved genuinely broken code (hooks that
// continue to throw after a fresh hook list) sees the error
// fallback once, not an infinite reset loop. Each subsequent save
// raises UpdatePending again and grants exactly one more retry.
//
// Atomicity matters because UpdateApplication is invoked by the
// hot-reload runtime on a non-UI thread; a non-atomic read-then-
// write here could miss a pending update if the hot-reload thread
// raises the flag in the window between the read and the write.
bool hotReloadRender = HotReloadService.ConsumeUpdatePending();

// Local helper centralizes the recovery sequence (log → reset
// RenderContext → request a fresh render). Both component-mode
// and function-mode catches share it; future tweaks (telemetry,
// additional reset steps, throttling) only need editing here.
void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode)
{
_logger.LogWarning(ex,
"Hot reload: hook order/type changed — resetting {Mode} state and re-rendering",
mode);
ctx.ResetForHotReload();
RequestRender();

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Pack (x64 + arm64)

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Pack (x64 + arm64)

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Pack (x64 + arm64)

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Pack (x64 + arm64)

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Selftests

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Selftests

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Build solution

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Build solution

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Build solution

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Build solution

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Unit Tests

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.

Check warning on line 360 in src/Reactor/Hosting/ReactorHostControl.cs

View workflow job for this annotation

GitHub Actions / Unit Tests

Possible null reference argument for parameter 'logger' in 'void LoggerExtensions.LogWarning(ILogger logger, Exception? exception, string? message, params object?[] args)'.
}

try
{
Element? newTree = null;
Expand All @@ -338,6 +373,11 @@
{
newTree = _rootComponent.Render();
}
catch (HookOrderException ex) when (hotReloadRender)
{
RecoverFromHookOrder(ex, _rootComponent.Context, "component");
return;
}
Comment thread
codemonkeychris marked this conversation as resolved.
catch (Exception ex)
{
_logger.LogError(ex, "Component Render() threw");
Expand All @@ -352,6 +392,11 @@
{
newTree = _rootRenderFunc(_funcContext);
}
catch (HookOrderException ex) when (hotReloadRender)
{
RecoverFromHookOrder(ex, _funcContext, "function-component");
return;
}
Comment thread
codemonkeychris marked this conversation as resolved.
catch (Exception ex)
{
_logger.LogError(ex, "Function component threw");
Expand Down
2 changes: 1 addition & 1 deletion tests/Reactor.Tests/ContextSystemUnitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public void UseContext_Hook_Order_Violation_Throws()

// Second render: try UseContext where UseState was
ctx.BeginRender(() => { }, scope);
var ex = Assert.Throws<InvalidOperationException>(() => ctx.UseContext(TestTheme));
var ex = Assert.Throws<HookOrderException>(() => ctx.UseContext(TestTheme));
Assert.Contains("ContextHookState", ex.Message);
}

Expand Down
61 changes: 59 additions & 2 deletions tests/Reactor.Tests/HookStateRefactorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public void Hook_Order_Violation_Throws_Descriptive_Error_With_New_Type_Names()

// Second render: try to call UseEffect where UseState was — should throw
ctx.BeginRender(() => { });
var ex = Assert.Throws<InvalidOperationException>(() => ctx.UseEffect(() => { }, "dep"));
var ex = Assert.Throws<HookOrderException>(() => ctx.UseEffect(() => { }, "dep"));
Assert.Contains("ValueHookState", ex.Message);
}

Expand All @@ -255,11 +255,68 @@ public void Hook_Order_Violation_UseState_At_Effect_Position_Throws()

// Second render: UseState where UseEffect was — should throw
ctx.BeginRender(() => { });
var ex = Assert.Throws<InvalidOperationException>(() => ctx.UseState(0));
var ex = Assert.Throws<HookOrderException>(() => ctx.UseState(0));
Assert.Contains("EffectHookState", ex.Message);
Assert.Contains("ValueHookState", ex.Message);
}

// ════════════════════════════════════════════════════════════════
// Hot Reload recovery: ResetForHotReload clears state + cleanups
// so a render after a hook-order break can succeed
// ════════════════════════════════════════════════════════════════

[Fact]
public void ResetForHotReload_RunsCleanups_And_ClearsHooks_So_Next_Render_Sees_Fresh_Sequence()
{
var ctx = new RenderContext();
bool cleanupRan = false;

// Render 1 (pre-edit): UseState + UseEffect with cleanup.
ctx.BeginRender(() => { });
ctx.UseState(42);
ctx.UseEffect(() => () => cleanupRan = true, "dep");
ctx.FlushEffects();

// Render 2 (post-edit): the developer reordered hooks. Calling
// UseEffect at index 0 (where UseState lived) throws.
ctx.BeginRender(() => { });
Assert.Throws<HookOrderException>(() => ctx.UseEffect(() => { }, "dep"));

// Hot reload recovery: cleanups run, hook list reset.
ctx.ResetForHotReload();
Assert.True(cleanupRan, "Effect cleanup should run during ResetForHotReload");

// Render 3 (recovery): the new hook order is accepted as if first
// mount — UseEffect-then-UseState now works because the hook list
// starts empty. State is lost (the 42 from render 1), which is the
// documented trade-off.
ctx.BeginRender(() => { });
ctx.UseEffect(() => { }, "dep");
var (value, _) = ctx.UseState(0);
Assert.Equal(0, value);
}

[Fact]
public void ResetForHotReload_Allows_Different_Hook_Type_At_Same_Index()
{
var ctx = new RenderContext();

// Render 1: UseState<int>.
ctx.BeginRender(() => { });
ctx.UseState(7);

// Render 2: developer changed UseState<int> to UseState<string>.
// Same hook *kind* but different generic — different ValueHookState<T>.
ctx.BeginRender(() => { });
Assert.Throws<HookOrderException>(() => ctx.UseState("hello"));

// After recovery the new shape works.
ctx.ResetForHotReload();
ctx.BeginRender(() => { });
var (s, _) = ctx.UseState("hello");
Assert.Equal("hello", s);
}

// ════════════════════════════════════════════════════════════════
// Effect cleanup ordering (post-render)
// ════════════════════════════════════════════════════════════════
Expand Down
2 changes: 1 addition & 1 deletion tests/Reactor.Tests/NavigationLifecycleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void UseNavigationLifecycle_Throws_If_Hook_Order_Changes()

// Second render: try UseNavigationLifecycle at UseState's index
ctx.BeginRender(() => { });
Assert.Throws<InvalidOperationException>(() => ctx.UseNavigationLifecycle());
Assert.Throws<HookOrderException>(() => ctx.UseNavigationLifecycle());
}

// ════════════════════════════════════════════════════════════════
Expand Down
2 changes: 1 addition & 1 deletion tests/Reactor.Tests/PersistedStateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ public void UsePersisted_Hook_Order_Violation_Throws()

// Second render: try UsePersisted where UseState was
ctx.BeginRender(() => { });
var ex = Assert.Throws<InvalidOperationException>(() => ctx.UsePersisted("key", "hello"));
var ex = Assert.Throws<HookOrderException>(() => ctx.UsePersisted("key", "hello"));
Assert.Contains("PersistedHookState", ex.Message);
}
}
Loading