From 4756afd75b23f581cadba2125a7fee866925ae8d Mon Sep 17 00:00:00 2001 From: David Miguel Date: Thu, 16 Jul 2026 15:47:09 +0200 Subject: [PATCH] [go_router] Preserve live push completer on re-decode to stop spurious onExit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `refreshListenable` tick (e.g. tab refocus) re-parses the encoded current configuration. The codec mints a fresh `Completer` per `ImperativeRouteMatch`, so the re-decoded list compares unequal to the live one, the delegate's no-op fast path is defeated, and `onExit` fires on a route that never left the stack. Instead of dropping the completer from `ImperativeRouteMatch` identity — which breaks `replace()` to the same location (it reuses the old pageKey and differs only by its new completer, so the new configuration would compare equal, the `setNewRoutePath` fast path would no-op, and the returned future would be orphaned) — reconcile completers only on the re-decode path: for each decoded imperative match, adopt the live completer of the match with the same pageKey (unique per push). The re-decoded list then compares equal including completer, the fast path holds naturally, and in-flight push futures are preserved. Tests: onExit not fired on a refresh tick; live push completer resolves across a refresh; replace() to the same location still completes its future; reconcileImperativeCompleters adopts live completers by pageKey and keeps fresh ones when there is no live match. --- packages/go_router/lib/src/match.dart | 50 +++++++ packages/go_router/lib/src/parser.dart | 7 +- .../change_2026_07_16_1784204402825.yaml | 3 + packages/go_router/test/match_test.dart | 42 ++++++ packages/go_router/test/on_exit_test.dart | 139 ++++++++++++++++++ 5 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 packages/go_router/pending_changelogs/change_2026_07_16_1784204402825.yaml diff --git a/packages/go_router/lib/src/match.dart b/packages/go_router/lib/src/match.dart index b66edab0f4c1..c95db222510c 100644 --- a/packages/go_router/lib/src/match.dart +++ b/packages/go_router/lib/src/match.dart @@ -785,6 +785,56 @@ class RouteMatchList with Diagnosticable { ); } + /// Returns a copy of this list where each [ImperativeRouteMatch] adopts the + /// completer of the live match with the same [pageKey] in [source]. + /// + /// Decoding an encoded configuration (state restoration, browser + /// back/forward, or a `refreshListenable` re-parse) mints a fresh completer + /// per imperative match, since the push future cannot be serialized. Carrying + /// over the live completer keeps a re-decoded configuration equal to the one + /// already on screen, so the no-op fast path holds — no route spuriously + /// exits and the in-flight push future is preserved. A [pageKey] is unique + /// per push, so it identifies the live match unambiguously. + // TODO(loic-sharma): Remove meta library prefix. + // https://github.com/flutter/flutter/issues/171410 + @meta.internal + RouteMatchList reconcileImperativeCompleters(RouteMatchList source) { + final liveCompleters = , Completer>{}; + source.visitRouteMatches((RouteMatchBase match) { + if (match is ImperativeRouteMatch) { + liveCompleters[match.pageKey] = match.completer; + } + return true; + }); + if (liveCompleters.isEmpty) { + return this; + } + return copyWith(matches: _reconcileMatches(matches, liveCompleters)); + } + + static List _reconcileMatches( + List matches, + Map, Completer> liveCompleters, + ) { + return matches.map((RouteMatchBase match) { + if (match is ImperativeRouteMatch) { + final Completer? live = liveCompleters[match.pageKey]; + if (live == null || live == match.completer) { + return match; + } + return ImperativeRouteMatch( + pageKey: match.pageKey, + matches: match.matches, + completer: live, + ); + } + if (match is ShellRouteMatch) { + return match.copyWith(matches: _reconcileMatches(match.matches, liveCompleters)); + } + return match; + }).toList(); + } + @override bool operator ==(Object other) { if (other.runtimeType != runtimeType) { diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index 7ed750b4c3f0..da9ca7310d5f 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -91,7 +91,12 @@ class GoRouteInformationParser extends RouteInformationParser { incomingUri = routeInformation.uri; } else if (raw is! RouteInformationState) { // Restoration/back-forward: decode the stored match list and treat as restore. - final RouteMatchList decoded = _routeMatchListCodec.decode(raw as Map); + // Decoding mints fresh completers for imperative matches; carry over the + // live ones so a re-parse of the current configuration stays a no-op + // instead of spuriously exiting a route or orphaning its push future. + final RouteMatchList decoded = _routeMatchListCodec + .decode(raw as Map) + .reconcileImperativeCompleters(router.routerDelegate.currentConfiguration); infoState = RouteInformationState.restore(base: decoded); incomingUri = decoded.uri; } else { diff --git a/packages/go_router/pending_changelogs/change_2026_07_16_1784204402825.yaml b/packages/go_router/pending_changelogs/change_2026_07_16_1784204402825.yaml new file mode 100644 index 000000000000..939297e5a2dc --- /dev/null +++ b/packages/go_router/pending_changelogs/change_2026_07_16_1784204402825.yaml @@ -0,0 +1,3 @@ +changelog: | + - Fixes a spurious `onExit` call on a pushed route when `refreshListenable` fires (e.g. on tab refocus), by carrying over the live push completer when the current configuration is re-decoded. +version: patch diff --git a/packages/go_router/test/match_test.dart b/packages/go_router/test/match_test.dart index 7a1fcd956f62..c36ff9d33da2 100644 --- a/packages/go_router/test/match_test.dart +++ b/packages/go_router/test/match_test.dart @@ -199,6 +199,48 @@ void main() { expect(match1 == match2, isFalse); expect(match1.hashCode == match2.hashCode, isFalse); }); + + test('reconcileImperativeCompleters adopts live completers by pageKey', () async { + // A decoded configuration carries fresh completers; reconciling against the + // live configuration must make the two lists equal again (completer + // included in identity), so the delegate's no-op fast path holds. + final live = RouteMatchList( + matches: [ + matchList1.matches.first, + ImperativeRouteMatch(pageKey: key1, matches: matchList1, completer: completer1), + ], + uri: Uri.parse('/'), + pathParameters: const {}, + ); + final decoded = RouteMatchList( + matches: [ + matchList1.matches.first, + ImperativeRouteMatch(pageKey: key1, matches: matchList1, completer: completer2), + ], + uri: Uri.parse('/'), + pathParameters: const {}, + ); + + expect(decoded == live, isFalse); + final RouteMatchList reconciled = decoded.reconcileImperativeCompleters(live); + expect(reconciled == live, isTrue); + expect((reconciled.matches.last as ImperativeRouteMatch).completer, completer1); + }); + + test('reconcileImperativeCompleters keeps fresh completer when no live match', () async { + // Genuine state restoration (no live imperative match with that pageKey): + // the fresh completer is kept as-is. + final decoded = RouteMatchList( + matches: [ + matchList1.matches.first, + ImperativeRouteMatch(pageKey: key1, matches: matchList1, completer: completer2), + ], + uri: Uri.parse('/'), + pathParameters: const {}, + ); + final RouteMatchList reconciled = decoded.reconcileImperativeCompleters(RouteMatchList.empty); + expect((reconciled.matches.last as ImperativeRouteMatch).completer, completer2); + }); }); } diff --git a/packages/go_router/test/on_exit_test.dart b/packages/go_router/test/on_exit_test.dart index 990ee9a9a6a3..3a6e12930de5 100644 --- a/packages/go_router/test/on_exit_test.dart +++ b/packages/go_router/test/on_exit_test.dart @@ -606,4 +606,143 @@ void main() { await tester.pumpAndSettle(); expect(find.byKey(home), findsOneWidget); }); + + testWidgets('onExit is not called when refreshListenable fires on a pushed route', ( + WidgetTester tester, + ) async { + var onExitCount = 0; + final home = UniqueKey(); + final form = UniqueKey(); + final notifier = ValueNotifier(0); + addTearDown(notifier.dispose); + final router = GoRouter( + initialLocation: '/', + refreshListenable: notifier, + routes: [ + GoRoute( + path: '/', + builder: (_, _) => DummyScreen(key: home), + ), + GoRoute( + path: '/form', + builder: (_, _) => DummyScreen(key: form), + onExit: (BuildContext context, GoRouterState state) { + onExitCount++; + return true; + }, + ), + ], + ); + addTearDown(router.dispose); + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + + unawaited(router.push('/form')); + await tester.pumpAndSettle(); + expect(find.byKey(form), findsOneWidget); + expect(onExitCount, 0); + + // A refreshListenable tick (e.g. a tab-focus claims refresh) re-parses the + // encoded current route. That round-trip must be a no-op: onExit fires only + // when a route actually leaves the stack, not on a refresh re-decode. + notifier.value++; + await tester.pumpAndSettle(); + + expect(onExitCount, 0); + expect(find.byKey(form), findsOneWidget); + }); + + testWidgets('live push completer is preserved across a refreshListenable tick', ( + WidgetTester tester, + ) async { + final home = UniqueKey(); + final form = UniqueKey(); + final notifier = ValueNotifier(0); + addTearDown(notifier.dispose); + final router = GoRouter( + initialLocation: '/', + refreshListenable: notifier, + routes: [ + GoRoute( + path: '/', + builder: (_, _) => DummyScreen(key: home), + ), + GoRoute( + path: '/form', + builder: (_, _) => DummyScreen(key: form), + onExit: (BuildContext context, GoRouterState state) => true, + ), + ], + ); + addTearDown(router.dispose); + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + + // Capture completion via a flag rather than awaiting the future directly: if + // the refresh orphans the live completer (the bug), the future never completes + // and `completed` stays false — a synchronous, fail-fast assertion that cannot + // hang the suite. + var completed = false; + Object? completedWith; + unawaited( + router.push('/form').then((Object? value) { + completed = true; + completedWith = value; + }), + ); + await tester.pumpAndSettle(); + + notifier.value++; + await tester.pumpAndSettle(); + + router.pop('result'); + await tester.pumpAndSettle(); + + expect(completed, isTrue); + expect(completedWith, 'result'); + }); + + testWidgets('replace() to the same location still completes its future', ( + WidgetTester tester, + ) async { + final home = UniqueKey(); + final form = UniqueKey(); + final router = GoRouter( + initialLocation: '/', + routes: [ + GoRoute( + path: '/', + builder: (_, _) => DummyScreen(key: home), + ), + GoRoute( + path: '/form', + builder: (_, _) => DummyScreen(key: form), + ), + ], + ); + addTearDown(router.dispose); + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + + unawaited(router.push('/form')); + await tester.pumpAndSettle(); + expect(find.byKey(form), findsOneWidget); + + // replace() reuses the old match's pageKey and differs only by a new + // completer, so the completer must stay part of identity — otherwise the new + // configuration equals the current one, setNewRoutePath no-ops, and the + // returned future is orphaned. + var completed = false; + Object? completedWith; + unawaited( + router.replace('/form').then((Object? value) { + completed = true; + completedWith = value; + }), + ); + await tester.pumpAndSettle(); + + router.pop('done'); + await tester.pumpAndSettle(); + + expect(completed, isTrue); + expect(completedWith, 'done'); + }); }