fix(router): avoid notifying listeners during build on page removal

[Eimen2018]

Summary

Fixes #1184: browser back navigation on a Stacked web app (default stacked create app -t web template) throws setState() or markNeedsBuild() called during build from _RootRouter.

Root cause

Matched against the reported stack trace:

1. Navigator.onDidRemovePage in route_navigator.dart calls router.removeRoute(routeData).
2. StackRouter._removeRoute called notifyAll(forceUrlRebuild: true) unconditionally, even when the page was already gone from _pages.
3. notifyAll triggers _RootRouterState._handleRebuild, which calls setState().

When the removal is a side effect of the Navigator updating while the router widget's own rebuild is still in progress, that synchronous setState() lands during build and throws.

Fix

lib/src/router/controller/routing_controller.dart:

• No-op removal no longer notifies. When the page is not in _pages, nothing is removed, shared path data is left alone and listeners are not notified. Redirect-guard cleanup and _removeTopRouterOf still run, as on master, so pages taken out directly by removeWhere, _reset or updateDeclarativeRoutes still get their guard listeners and child routers cleaned up.
• _removeRedirectGuard is null-safe. It does nothing when the guard has no registered listener.
• Build-phase guard. A single _notifySafely helper defers the notification with addPostFrameCallback when called during SchedulerPhase.persistentCallbacks, and stays synchronous in every other phase. It is used by _removeRoute, removeWhere, _removeUntil and _addEntry, so the fix covers the class of bug rather than the one stack frame in the report. setActiveIndex and the declarative updateRoutes path are left as they are.
• Dispose guard. StackRouter tracks _disposed. On a disposed router the deferred callback skips its own listeners, which would throw, but still notifies the root and rebuilds the URL when it is a nested router, so the browser URL does not keep showing a removed nested path.

Tests

• browser_back_navigation_test.dart: removing the top page from a descendant's build while the router widget is rebuilding, via removeRoute, removeWhere and removeUntil. All three reproduce the reported exception without the fix. Only these reproduce it. The handlePopRoute() test is an end-to-end sanity check that a real pop completes and leaves one page, and it passes on master too.
• router_cleanup_test.dart: a route with a RedirectGuard is removed via removeWhere, then the Navigator's follow-up removeRoute hits the not-found path. Asserts the guard listener is detached and a later reevaluate does not re-push. Failed with the earlier early return, passes now.
• nested_router_dispose_test.dart: a nested router schedules the deferred notify and is disposed in the same frame. Threw "used after being disposed" before the guard, passes now.

Verification

• flutter test: 99 passed, 1 pre-existing skip
• dart analyze lib test: no new issues
• No public API change

Note: #1197 also touches _removeRoute. Whichever merges second will need a small rebase.

🤖 Generated with Claude Code

[github-actions]

LCOV of commit f1a5b64 during Test Package #161

📊 Summary Coverage

Metric	Coverage	Details
📈 Lines	🔴 25.3%	936 / 3702 lines
🔧 Functions	⚪ N/A	no data found
🌿 Branches	⚪ N/A	no data found

📁 Files Changed Coverage

File	Lines	Functions	Branches
lib/src/code_generation/router_annotation/extended_navigator.dart	⚪ – (0)	⚪ – (0)	⚪ – (0)
lib/src/router/controller/routing_controller.dart	🔴 0.0% (192)	⚪ – (0)	⚪ – (0)

🟢 ≥ 90% · 🟡 ≥ 70% · 🔴 below 70% · ⚪ no data
Full coverage report

[ferrarafer]

Early return skips the redirect-guard cleanup and child-router cleanup when the page is already gone from _pages.

removeWhere (and _reset / updateDeclarativeRoutes) take pages out of _pages directly. They never call _removeRedirectGuard or _removeTopRouterOf. Before this PR, when the Navigator then dropped that page, onDidRemovePage -> removeRoute hit pageIndex == -1 but still ran the guard loop and _removeTopRouterOf, so the listener got cleaned up there. With this early return that never happens.

Scenario: a route guarded by a RedirectGuard is removed with router.removeWhere(...). Its listener stays in _redirectGuardsListeners. The next time the guard calls notifyListeners() (for example on an auth state change), _reevaluate runs on this router. No guarded route is left in the stack, so it goes straight to _strategy.reevaluate(...), and that can re-push routes or removeAllAndPush for a page the user already left.

Suggestion: skip only the _pages.removeAt and the notification when pageIndex == -1. Keep the cleanup, but make it null-safe. For example, _removeRedirectGuard should do nothing if _redirectGuardsListeners[guard] is null.

[ferrarafer]

The deferred notifyAll can run on a router that has already been disposed.

The post-frame closure holds onto this and never checks whether the controller is still alive. Nested routers dispose their StackRouter in State.dispose (nested_router.dart, nested_router_delegate.dart). Unmounting happens during finalizeTree, which is still in the same persistentCallbacks phase that scheduled the callback.

Scenario: a nested router gets onDidRemovePage during the build phase, and its widget is unmounted later in that same frame. The post-frame callback then calls notifyListeners() on the disposed ChangeNotifier. In debug builds this throws "A StackRouter was used after being disposed". It also still triggers root.notifyListeners() and navigationHistory.rebuildUrl() from a stale router, which can overwrite the URL with the torn-down state.

Suggestion: track disposal (for example a _disposed flag set in dispose) and return early from the callback when it is set.

[ferrarafer]

[medium] The build-phase guard is applied only to _removeRoute, so the reported crash is still reachable through the sibling removal paths.

notifyAll(forceUrlRebuild: true) is also called synchronously from removeWhere (~line 1089) and _removeUntil (~line 1075), and notifyAll() from _addEntry (~line 1181). None of them go through _notifyRouteRemoved, so they still call setState()/markNeedsBuild() during build.

I verified this on this branch: a descendant widget whose build calls router.removeWhere((r) => r.name == 'SecondRoute') (the same shape as the first new test, which uses removeRoute) still throws:

The following assertion was thrown while dispatching notifications for _R:
setState() or markNeedsBuild() called during build.
The widget on which setState() or markNeedsBuild() was called was: _RootRouter
...
The following assertion was thrown while dispatching notifications for NestedRouterDelegate:
setState() or markNeedsBuild() called during build.
The widget on which setState() or markNeedsBuild() was called was: Router<Object>

Note this also shows the second setState source (NestedRouterDelegate._handleRebuild via navigationHistory.rebuildUrl()), which _notifyRouteRemoved happens to cover for the _removeRoute path but not elsewhere.

Since the helper already exists, consider making it a general _notifySafely({bool forceUrlRebuild}) and routing removeWhere, _removeUntil and _addEntry through it too — otherwise the fix is specific to the one stack frame in #1184 rather than to the class of bug.

[ferrarafer]

[low] This second test passes on master too, so it does not guard the fix — and it never asserts that the back navigation actually did anything.

I ran the three new test files against the pre-fix routing_controller.dart: browser_back_navigation_test.dart test #1 and nested_router_dispose_test.dart fail (good reproductions), but this handlePopRoute() test passes both before and after. So the "real system/browser back navigation" path as modelled here never actually hit the reported exception.

It is also vacuous in a second way: the only assertion is takeException() == null. If handlePopRoute() silently did nothing, the test would still pass. Instrumenting it shows the pop does happen (router.stack.length == 1, currentUrl == '/'), so those are cheap assertions to add:

Suggested change

	await tester.binding.handlePopRoute();
	for (var i = 0; i < 10; i++) {
	await tester.pump(const Duration(milliseconds: 50));
	expect(tester.takeException(), isNull);
	}
	await tester.binding.handlePopRoute();
	for (var i = 0; i < 10; i++) {
	await tester.pump(const Duration(milliseconds: 50));
	expect(tester.takeException(), isNull);
	}
	expect(router.stack, hasLength(1));
	expect(find.text('Home'), findsOneWidget);
	expect(router.currentUrl, '/');

Worth flagging in the PR description too, since it is presented as the end-to-end check for #1184 — the only test that actually reproduces the reported exception is test #1, which triggers the removal from a descendant's build rather than from a genuine browser back.