Fix popping repeated pages unexpectedly

packages/go_router/CHANGELOG.md

@@ -1,6 +1,7 @@
-## NEXT
+## 10.1.1
 
-* Updates minimum supported SDK version to Flutter 3.7/Dart 2.19.
+- Fixes mapping from `Page` to `RouteMatch`s. 
+- Updates minimum supported SDK version to Flutter 3.7/Dart 2.19.
 
 ## 10.1.0
 

packages/go_router/lib/src/builder.dart

@@ -172,7 +172,7 @@ class RouteBuilder {
 
       // Every Page should have a corresponding RouteMatch.
       assert(keyToPage.values.flattened.every((Page<Object?> page) =>
-          pagePopContext.getRouteMatchForPage(page) != null));
+          pagePopContext.getRouteMatchesForPage(page) != null));
     }
 
     /// Clean up previous cache to prevent memory leak, making sure any nested
@@ -299,7 +299,8 @@ class RouteBuilder {
     }
     if (page != null) {
       registry[page] = state;
-      pagePopContext._setRouteMatchForPage(page, match);
+      // Insert the route match in reverse order.
+      pagePopContext._insertRouteMatchAtStartForPage(page, match);
     }
   }
 
@@ -561,8 +562,10 @@ typedef _PageBuilderForAppType = Page<void> Function({
 class _PagePopContext {
   _PagePopContext._(this.onPopPageWithRouteMatch);
 
-  final Map<Page<dynamic>, RouteMatch> _routeMatchLookUp =
-      <Page<Object?>, RouteMatch>{};
+  /// A page can be mapped to a RouteMatch list, such as a const page being
+  /// pushed multiple times.
+  final Map<Page<dynamic>, List<RouteMatch>> _routeMatchesLookUp =
+      <Page<Object?>, List<RouteMatch>>{};
 
   /// On pop page callback that includes the associated [RouteMatch].
   final PopPageWithRouteMatchCallback onPopPageWithRouteMatch;
@@ -571,18 +574,28 @@ class _PagePopContext {
   ///
   /// The [Page] must have been previously built via the [RouteBuilder] that
   /// created this [PagePopContext]; otherwise, this method returns null.
-  RouteMatch? getRouteMatchForPage(Page<Object?> page) =>
-      _routeMatchLookUp[page];
-
-  void _setRouteMatchForPage(Page<Object?> page, RouteMatch match) =>
-      _routeMatchLookUp[page] = match;
+  List<RouteMatch>? getRouteMatchesForPage(Page<Object?> page) =>
+      _routeMatchesLookUp[page];
+
+  /// This is called in _buildRecursive to insert route matches in reverse order.
+  void _insertRouteMatchAtStartForPage(Page<Object?> page, RouteMatch match) {
+    _routeMatchesLookUp
+        .putIfAbsent(page, () => <RouteMatch>[])
+        .insert(0, match);
+  }
 
   /// Function used as [Navigator.onPopPage] callback when creating Navigators.
   ///
   /// This function forwards to [onPopPageWithRouteMatch], including the
   /// [RouteMatch] associated with the popped route.
+  ///
+  /// This assumes always pop the last route match for the page.
   bool onPopPage(Route<dynamic> route, dynamic result) {
     final Page<Object?> page = route.settings as Page<Object?>;
-    return onPopPageWithRouteMatch(route, result, _routeMatchLookUp[page]);
+
+    final RouteMatch match = _routeMatchesLookUp[page]!.last;
+    _routeMatchesLookUp[page]!.removeLast();
+
+    return onPopPageWithRouteMatch(route, result, match);
   }
 }

packages/go_router/pubspec.yaml

@@ -1,7 +1,7 @@
 name: go_router
 description: A declarative router for Flutter based on Navigation 2 supporting
   deep linking, data-driven routes and more
-version: 10.1.0
+version: 10.1.1
 repository: https://github.com/flutter/packages/tree/main/packages/go_router
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
 

packages/go_router/test/go_router_test.dart

@@ -309,6 +309,53 @@ void main() {
       expect(find.byKey(settings), findsOneWidget);
     });
 
+    testWidgets('can correctly pop stacks of repeated pages',
+        (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/#132229.
+
+      final GlobalKey<NavigatorState> navKey = GlobalKey<NavigatorState>();
+      final List<GoRoute> routes = <GoRoute>[
+        GoRoute(
+          path: '/',
+          pageBuilder: (_, __) =>
+              const MaterialPage<Object>(child: HomeScreen()),
+        ),
+        GoRoute(
+          path: '/page1',
+          pageBuilder: (_, __) =>
+              const MaterialPage<Object>(child: Page1Screen()),
+        ),
+        GoRoute(
+          path: '/page2',
+          pageBuilder: (_, __) =>
+              const MaterialPage<Object>(child: Page2Screen()),
+        ),
+      ];
+      final GoRouter router =
+          await createRouter(routes, tester, navigatorKey: navKey);
+      expect(find.byType(HomeScreen), findsOneWidget);
+
+      router.push('/page1');
+      router.push('/page2');
+      router.push('/page1');
+      router.push('/page2');
+      await tester.pumpAndSettle();
+
+      expect(find.byType(HomeScreen), findsNothing);
+      expect(find.byType(Page1Screen), findsNothing);
+      expect(find.byType(Page2Screen), findsOneWidget);
+
+      router.pop();
+      await tester.pumpAndSettle();
+
+      final List<RouteMatch> matches =
+          router.routerDelegate.currentConfiguration.matches;
+      expect(matches.length, 4);
+      expect(find.byType(HomeScreen), findsNothing);
+      expect(find.byType(Page1Screen), findsOneWidget);
+      expect(find.byType(Page2Screen), findsNothing);
+    });
+
     testWidgets('match sub-route', (WidgetTester tester) async {
       final List<GoRoute> routes = <GoRoute>[
         GoRoute(