fix(hot-reload,devtools): recover from hook-order edits; default DevtoolsMenu items to null#186
Conversation
…oolsMenu items to null
Two related fixes surfaced while putting together a demo app:
1. Hot Reload survives hook-order / hook-type edits.
Editing a component to add, remove, or reorder hooks (or change a
hook's generic type) used to leave the app stuck on the error
fallback after the very next save — the surviving RenderContext had
the *previous* hook list pinned by index, and the new Render() body
couldn't line up against it. The dev loop died until the user
restarted the app.
New behavior: when a hook-order/type mismatch surfaces during a
render that was triggered by .NET Hot Reload, the host runs effect
cleanups, drops the hook list, and re-renders from scratch. Hook
state is lost (we cannot reliably re-key arbitrary surviving values
onto a shape that may have changed) but the app keeps running.
- New HookOrderException : InvalidOperationException so the host
can sniff the specific subtype without changing existing
`catch (InvalidOperationException)` semantics for callers.
- All 10 hook-order/type-mismatch throw sites in RenderContext.cs
converted from InvalidOperationException to HookOrderException.
Cross-thread setter and UseNavigation lookup throws are
intentionally left as plain InvalidOperationException — neither
is a hot-reload-recoverable failure.
- RenderContext.ResetForHotReload(): runs cleanups, clears hook
list, resets index. New API, internal-only.
- HotReloadService.UpdatePending: signals "the next render is a
hot-reload-induced render". Set by UpdateApplication, consumed
(capture-and-clear) by ReactorHostControl.Render at the top of
the render attempt.
- ReactorHostControl.Render(): catch (HookOrderException) when
(hotReloadRender) recovers; any subsequent failure on the
recovery render falls through to the existing ShowErrorFallback
path. The capture-and-clear pattern guarantees at-most-once
recovery per UpdateApplication, so genuinely broken hook code
surfaces the error fallback once instead of an infinite reset
loop.
Tests:
- 5 existing Assert.Throws<InvalidOperationException> hook-order
tests tightened to Assert.Throws<HookOrderException>. The looser
assertion was incidental — these tests have always been about a
specific contract that now has a specific type.
- 2 new tests in HookStateRefactorTests.cs:
ResetForHotReload_RunsCleanups_And_ClearsHooks_So_Next_Render_Sees_Fresh_Sequence
and ResetForHotReload_Allows_Different_Hook_Type_At_Same_Index.
2. DevtoolsMenu items factory defaults to null.
Calling DevtoolsMenu() with no arguments to get the built-in
toggles (Highlight reconcile / Show layout cost) used to require
passing a no-op `() => Array.Empty<MenuFlyoutItemBase>()`. The
implementation already null-coalesces inside (`items?.Invoke()
?.ToArray() ?? Array.Empty<...>()`) — the parameter was just
non-nullable. Changed to `Func<...>? items = null` so
`DevtoolsMenu()` with no args is valid.
Test results:
- dotnet test tests/Reactor.Tests: 6821 passed, 0 failed (2 new).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR improves the developer experience during .NET Hot Reload by recovering from hook-order/type edits (resetting hook state + rerendering) and makes DevtoolsMenu() callable without providing a no-op items factory.
Changes:
- Introduces
HookOrderExceptionand updates hook-order throw sites + tests to use it. - Adds hot-reload recovery flow:
RenderContext.ResetForHotReload()+ host-side catch-and-rerender gated by a “hot reload render” flag. - Updates
DevtoolsMenufactory signature to allowitemsto be omitted (nulldefault).
Show a summary per file
| File | Description |
|---|---|
| tests/Reactor.Tests/PersistedStateTests.cs | Updates hook-order assertion to expect HookOrderException. |
| tests/Reactor.Tests/NavigationLifecycleTests.cs | Updates hook-order assertion to expect HookOrderException. |
| tests/Reactor.Tests/HookStateRefactorTests.cs | Updates exception expectations and adds new tests for ResetForHotReload() recovery behavior. |
| tests/Reactor.Tests/ContextSystemUnitTests.cs | Updates hook-order assertion to expect HookOrderException. |
| src/Reactor/Hosting/ReactorHostControl.cs | Captures/clears hot-reload flag and adds recovery catch blocks that reset context + rerender. |
| src/Reactor/Hosting/HotReloadService.cs | Adds UpdatePending flag and sets it during UpdateApplication. |
| src/Reactor/Hosting/Devtools/DevtoolsMenuFactory.cs | Makes items optional (null default) for DevtoolsMenu. |
| src/Reactor/Core/RenderContext.cs | Throws HookOrderException at hook mismatch sites and adds ResetForHotReload(). |
| src/Reactor/Core/HookOrderException.cs | Adds new exception type used to identify hook-order/type mismatches. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 5
| 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."); |
- HotReloadService.UpdatePending now backed by an int with Volatile.Write/Read on the producer side and Interlocked.Exchange via ConsumeUpdatePending() on the consumer side. UpdateApplication runs on a hot-reload runtime thread while Render runs on the UI dispatcher; the previous read-then-write pattern had a window where a concurrent UpdateApplication could raise the flag between Render's read and clear, losing the new pending update. Single CAS removes the window. - HotReloadService XML doc rewritten to match the actual contract: the flag is consumed (atomically) at the start of each render attempt, not "after the render completes" as the previous comment said. - ReactorHostControl.Render: extracted a RecoverFromHookOrder local function so the two HookOrderException catch branches (component-mode / function-mode) share a single log → reset → RequestRender sequence. Future tweaks (telemetry, throttling, additional reset steps) edit one place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Update pushed ( 1 — TOCTOU on capture-and-clear ( 2 — XML doc on 3 — 4 & 5 — Dedup the two catch blocks: Fixed. Extracted a local function Test gates: 6821 unit tests pass. |
#186 added hot-reload hook-order recovery to ReactorHostControl but missed ReactorHost — the path used by `ReactorApp.Run<T>(...)`. Apps launched via the app-builder (the demo template, samples, every non-XAML-Islands consumer) hit a HookOrderException on the first hot reload that adds, removes, or reorders a hook, fall through to the generic catch, and land on the error fallback until the user restarts the process. Mirror the same recovery sequence in ReactorHost.Render(): - Capture HotReloadService.ConsumeUpdatePending() once at the top. - Add a `catch (HookOrderException ex) when (hotReloadRender)` filter ahead of the generic catch on both component-mode and function-component-mode branches. - Recovery runs effect cleanups, drops the hook list (RenderContext.ResetForHotReload), and RequestRenders — same contract as ReactorHostControl, so devs working on a hook-shape edit see one warning log and the app keeps rendering.
Summary
Two related issues surfaced while putting together a demo app:
Hot Reload now survives hook-order / hook-type edits. Adding, removing, or reordering hooks (or changing a hook's generic type) used to leave the app stuck on the error fallback after the very next save — the surviving
RenderContexthad the previous hook list pinned by index. The dev loop died until the user restarted. New behavior: when a hook mismatch surfaces during a render triggered by .NET Hot Reload, the host runs effect cleanups, drops the hook list, and re-renders from scratch. State is lost (cannot reliably re-key surviving values onto a changed shape), but the app keeps running.DevtoolsMenu()works with no arguments. Calling it with no args used to require a no-op() => Array.Empty<MenuFlyoutItemBase>()to get the built-in toggles. The implementation already null-coalesced internally; only the parameter signature blocked it.Why one PR
Both surfaced in the same demo session. Together they're "things that should just work when you're prototyping." Splitting felt like ceremony for two tightly related quality-of-life fixes. Happy to split if reviewers prefer.
Hot Reload recovery — design
HookOrderException : InvalidOperationExceptionso the host can sniff the specific subtype without changing existingcatch (InvalidOperationException)semantics for callers.RenderContext.csconverted fromInvalidOperationExceptiontoHookOrderException. Cross-thread setter andUseNavigationlookup throws are intentionally left as plainInvalidOperationException— neither is a hot-reload-recoverable failure.RenderContext.ResetForHotReload()(internal): runs cleanups, clears hook list, resets index.HotReloadService.UpdatePending: signals "the next render is a hot-reload-induced render." Set byUpdateApplication, consumed (capture-and-clear) byReactorHostControl.Renderat the top.ReactorHostControl.Render()picks up the flag and addscatch (HookOrderException) when (hotReloadRender)recovery filters on each render branch.Infinite-loop guard
The capture-and-clear pattern guarantees at-most-once recovery per
UpdateApplicationcall:A developer who saved genuinely broken hook code sees the error fallback once, not an infinite reset loop. Each subsequent save grants exactly one more retry.
Test plan
dotnet test tests/Reactor.Tests— 6821 passed, 0 failed (2 new for the recovery path)dotnet test tests/Reactor.SelfTests— pending (running)Assert.Throws<InvalidOperationException>hook-order tests tightened toAssert.Throws<HookOrderException>. The looser assertion was incidental — these tests have always been about a specific contract that now has a specific type.HookStateRefactorTests.cs:ResetForHotReload_RunsCleanups_And_ClearsHooks_So_Next_Render_Sees_Fresh_SequenceandResetForHotReload_Allows_Different_Hook_Type_At_Same_Index.dotnet watchagainst a Reactor sample, edit a component to add/remove/reorder a hook, save, confirm the app reloads instead of hard-failing.🤖 Generated with Claude Code