[go_router] Make `replace` use `pop` and `push` to generate a new `pageKey` (#2747)
* :bug: Use pop and push in replace to generate a new pageKey
* :white_check_mark: Test that replace creates a new page key
* :arrow_up: Increase the version number
* :recycle: 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 <ryjohn@google.com>
diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
index 7653429..5c448b0 100644
--- a/packages/go_router/CHANGELOG.md
+++ b/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
diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart
index ae15383..1077358 100644
--- a/packages/go_router/lib/src/delegate.dart
+++ b/packages/go_router/lib/src/delegate.dart
@@ -45,6 +45,18 @@
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 @@
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 @@
/// 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.
diff --git a/packages/go_router/lib/src/matching.dart b/packages/go_router/lib/src/matching.dart
index 1775c71..36add3a 100644
--- a/packages/go_router/lib/src/matching.dart
+++ b/packages/go_router/lib/src/matching.dart
@@ -74,14 +74,10 @@
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 @@
/// 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.
diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml
index ff28571..2adf1ac 100644
--- a/packages/go_router/pubspec.yaml
+++ b/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
diff --git a/packages/go_router/test/delegate_test.dart b/packages/go_router/test/delegate_test.dart
index f60ce65..61110b4 100644
--- a/packages/go_router/test/delegate_test.dart
+++ b/packages/go_router/test/delegate_test.dart
@@ -154,6 +154,35 @@
);
},
);
+ 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', () {
