[go_router] Make replace use pop and push to generate a new pageKey (#2747)

* 🐛 Use pop and push in replace to generate a new pageKey

* ✅ Test that replace creates a new page key

* ⬆️ Increase the version number

* ♻️ Move the asserts to the router deleguate

* Wrap _debugAssertMatchListNotEmpty in an assert

* Update packages/go_router/lib/src/delegate.dart

Co-authored-by: John Ryan <[email protected]>

Author: ValentinVignal

packages/go_router/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 5.1.8
+
+- Fixes a bug with `replace` where it was not generated a new `pageKey`.
+
 ## 5.1.7
 
 - Adds documentation using dartdoc topics

packages/go_router/lib/src/delegate.dart

@@ -45,6 +45,18 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
   final bool routerNeglect;
 
   RouteMatchList _matchList = RouteMatchList.empty();
+
+  /// Stores the number of times each route route has been pushed.
+  ///
+  /// This is used to generate a unique key for each route.
+  ///
+  /// For example, it would could be equal to:
+  /// ```dart
+  /// {
+  ///   'family': 1,
+  ///   'family/:fid': 2,
+  /// }
+  /// ```
   final Map<String, int> _pushCounts = <String, int>{};
   final RouteConfiguration _configuration;
 
@@ -136,9 +148,21 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
     return navigatorKey.currentState?.canPop() ?? false;
   }
 
+  void _debugAssertMatchListNotEmpty() {
+    assert(
+      _matchList.isNotEmpty,
+      'You have popped the last page off of the stack,'
+      ' there are no pages left to show',
+    );
+  }
+
   /// Pop the top page off the GoRouter's page stack.
   void pop() {
     _matchList.pop();
+    assert(() {
+      _debugAssertMatchListNotEmpty();
+      return true;
+    }());
     notifyListeners();
   }
 
@@ -147,8 +171,8 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
   /// See also:
   /// * [push] which pushes the given location onto the page stack.
   void replace(RouteMatch match) {
-    _matchList.matches.last = match;
-    notifyListeners();
+    _matchList.pop();
+    push(match); // [push] will notify the listeners.
   }
 
   /// For internal use; visible for testing only.

packages/go_router/lib/src/matching.dart

@@ -74,14 +74,10 @@ class RouteMatchList {
   void pop() {
     _matches.removeLast();
 
-    _debugAssertNotEmpty();
-
     // Also pop ShellRoutes when there are no subsequent route matches
     while (_matches.isNotEmpty && _matches.last.route is ShellRoute) {
       _matches.removeLast();
     }
-
-    _debugAssertNotEmpty();
   }
 
   /// An optional object provided by the app during navigation.
@@ -98,13 +94,6 @@ class RouteMatchList {
 
   /// Returns the error that this match intends to display.
   Exception? get error => matches.first.error;
-
-  void _debugAssertNotEmpty() {
-    assert(
-        _matches.isNotEmpty,
-        'You have popped the last page off of the stack,'
-        ' there are no pages left to show');
-  }
 }
 
 /// An error that occurred during matching.

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: 5.1.7
+version: 5.1.8
 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/delegate_test.dart

@@ -154,6 +154,35 @@ void main() {
         );
       },
     );
+    testWidgets(
+      'It should return different pageKey when replace is called',
+      (WidgetTester tester) async {
+        final GoRouter goRouter = await createGoRouter(tester);
+        expect(goRouter.routerDelegate.matches.matches.length, 1);
+        expect(
+          goRouter.routerDelegate.matches.matches[0].pageKey,
+          null,
+        );
+
+        goRouter.push('/a');
+        await tester.pumpAndSettle();
+
+        expect(goRouter.routerDelegate.matches.matches.length, 2);
+        expect(
+          goRouter.routerDelegate.matches.matches.last.pageKey,
+          const Key('/a-p1'),
+        );
+
+        goRouter.replace('/a');
+        await tester.pumpAndSettle();
+
+        expect(goRouter.routerDelegate.matches.matches.length, 2);
+        expect(
+          goRouter.routerDelegate.matches.matches.last.pageKey,
+          const Key('/a-p2'),
+        );
+      },
+    );
   });
 
   group('replaceNamed', () {