[go_router] Fixes crashes when popping navigators manually (#2952)
* [go_router] Fixes crashes when popping navigators manually
* rename
* add additional test case
diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
index 075e950..c1fa3db 100644
--- a/packages/go_router/CHANGELOG.md
+++ b/packages/go_router/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 6.0.1
+
+- Fixes crashes when popping navigators manually.
+- Fixes trailing slashes after pops.
+
## 6.0.0
- **BREAKING CHANGE**
diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart
index 04c6656..238dd35 100644
--- a/packages/go_router/lib/src/builder.dart
+++ b/packages/go_router/lib/src/builder.dart
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'package:collection/collection.dart';
import 'package:flutter/widgets.dart';
import 'configuration.dart';
@@ -50,6 +51,18 @@
final GoRouterStateRegistry _registry = GoRouterStateRegistry();
+ final Map<Page<Object?>, RouteMatch> _routeMatchLookUp =
+ <Page<Object?>, RouteMatch>{};
+
+ /// Looks the the [RouteMatch] for a given [Page].
+ ///
+ /// The [Page] must be in the latest [Navigator.pages]; otherwise, this method
+ /// returns null.
+ RouteMatch? getRouteMatchForPage(Page<Object?> page) =>
+ _routeMatchLookUp[page];
+
+ // final Map<>
+
/// Builds the top-level Navigator for the given [RouteMatchList].
Widget build(
BuildContext context,
@@ -57,6 +70,7 @@
PopPageCallback onPopPage,
bool routerNeglect,
) {
+ _routeMatchLookUp.clear();
if (matchList.isEmpty) {
// The build method can be called before async redirect finishes. Build a
// empty box until then.
@@ -119,10 +133,15 @@
GlobalKey<NavigatorState> navigatorKey,
Map<Page<Object?>, GoRouterState> registry) {
try {
+ assert(_routeMatchLookUp.isEmpty);
final Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPage =
<GlobalKey<NavigatorState>, List<Page<Object?>>>{};
_buildRecursive(context, matchList, 0, onPopPage, routerNeglect,
keyToPage, navigatorKey, registry);
+
+ // Every Page should have a corresponding RouteMatch.
+ assert(keyToPage.values.flattened
+ .every((Page<Object?> page) => _routeMatchLookUp.containsKey(page)));
return keyToPage[navigatorKey]!;
} on _RouteBuilderError catch (e) {
return <Page<Object?>>[
@@ -271,12 +290,13 @@
page = null;
}
+ page ??= buildPage(context, state, Builder(builder: (BuildContext context) {
+ return _callRouteBuilder(context, state, match, childWidget: child);
+ }));
+ _routeMatchLookUp[page] = match;
+
// Return the result of the route's builder() or pageBuilder()
- return page ??
- // Uses a Builder to make sure its rebuild scope is limited to the page.
- buildPage(context, state, Builder(builder: (BuildContext context) {
- return _callRouteBuilder(context, state, match, childWidget: child);
- }));
+ return page;
}
/// Calls the user-provided route builder from the [RouteMatch]'s [RouteBase].
@@ -363,7 +383,7 @@
_cacheAppType(context);
return _pageBuilderForAppType!(
key: state.pageKey,
- name: state.name ?? state.fullpath,
+ name: state.name ?? state.path,
arguments: <String, String>{...state.params, ...state.queryParams},
restorationId: state.pageKey.value,
child: child,
diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart
index b34aac0..07bcdf0 100644
--- a/packages/go_router/lib/src/delegate.dart
+++ b/packages/go_router/lib/src/delegate.dart
@@ -16,7 +16,7 @@
/// GoRouter implementation of [RouterDelegate].
class GoRouterDelegate extends RouterDelegate<RouteMatchList>
- with PopNavigatorRouterDelegateMixin<RouteMatchList>, ChangeNotifier {
+ with ChangeNotifier {
/// Constructor for GoRouter's implementation of the RouterDelegate base
/// class.
GoRouterDelegate({
@@ -132,7 +132,12 @@
if (!route.didPop(result)) {
return false;
}
- _matchList.pop();
+ final Page<Object?> page = route.settings as Page<Object?>;
+ final RouteMatch? match = builder.getRouteMatchForPage(page);
+ if (match == null) {
+ return true;
+ }
+ _matchList.remove(match);
notifyListeners();
assert(() {
_debugAssertMatchListNotEmpty();
@@ -146,7 +151,7 @@
/// See also:
/// * [push] which pushes the given location onto the page stack.
void pushReplacement(RouteMatchList matches) {
- _matchList.pop();
+ _matchList.remove(_matchList.last);
push(matches); // [push] will notify the listeners.
}
@@ -155,7 +160,6 @@
RouteMatchList get matches => _matchList;
/// For use by the Router architecture as part of the RouterDelegate.
- @override
GlobalKey<NavigatorState> get navigatorKey => _configuration.navigatorKey;
/// For use by the Router architecture as part of the RouterDelegate.
diff --git a/packages/go_router/lib/src/match.dart b/packages/go_router/lib/src/match.dart
index 9d01fa5..62a4d55 100644
--- a/packages/go_router/lib/src/match.dart
+++ b/packages/go_router/lib/src/match.dart
@@ -73,6 +73,6 @@
/// An exception if there was an error during matching.
final Exception? error;
- /// Optional value key of type string, to hold a unique reference to a page.
+ /// Value key of type string, to hold a unique reference to a page.
final ValueKey<String> pageKey;
}
diff --git a/packages/go_router/lib/src/matching.dart b/packages/go_router/lib/src/matching.dart
index 4297249..140c1d1 100644
--- a/packages/go_router/lib/src/matching.dart
+++ b/packages/go_router/lib/src/matching.dart
@@ -5,6 +5,7 @@
import 'package:flutter/widgets.dart';
import 'configuration.dart';
+import 'delegate.dart';
import 'match.dart';
import 'path_utils.dart';
@@ -56,7 +57,7 @@
static RouteMatchList empty =
RouteMatchList(<RouteMatch>[], Uri.parse(''), const <String, String>{});
- static String _generateFullPath(List<RouteMatch> matches) {
+ static String _generateFullPath(Iterable<RouteMatch> matches) {
final StringBuffer buffer = StringBuffer();
bool addsSlash = false;
for (final RouteMatch match in matches) {
@@ -96,17 +97,27 @@
_matches.add(match);
}
- /// Removes the last match.
- void pop() {
- if (_matches.last.route is GoRoute) {
- final GoRoute route = _matches.last.route as GoRoute;
- _uri = _uri.replace(path: removePatternFromPath(route.path, _uri.path));
- }
- _matches.removeLast();
+ /// Removes the match from the list.
+ void remove(RouteMatch match) {
+ final int index = _matches.indexOf(match);
+ assert(index != -1);
+ _matches.removeRange(index, _matches.length);
+
// Also pop ShellRoutes when there are no subsequent route matches
while (_matches.isNotEmpty && _matches.last.route is ShellRoute) {
_matches.removeLast();
}
+
+ final String fullPath = _generateFullPath(
+ _matches.where((RouteMatch match) => match is! ImperativeRouteMatch));
+ // Need to remove path parameters that are no longer in the fullPath.
+ final List<String> newParameters = <String>[];
+ patternToRegExp(fullPath, newParameters);
+ final Set<String> validParameters = newParameters.toSet();
+ pathParameters.removeWhere(
+ (String key, String value) => !validParameters.contains(key));
+
+ _uri = _uri.replace(path: patternToPath(fullPath, pathParameters));
}
/// An optional object provided by the app during navigation.
diff --git a/packages/go_router/lib/src/path_utils.dart b/packages/go_router/lib/src/path_utils.dart
index d7a20ff..e9db923 100644
--- a/packages/go_router/lib/src/path_utils.dart
+++ b/packages/go_router/lib/src/path_utils.dart
@@ -47,44 +47,6 @@
return RegExp(buffer.toString(), caseSensitive: false);
}
-/// Removes string from the end of the path that matches a `pattern`.
-///
-/// The path parameters can be specified by prefixing them with `:`. The
-/// `parameters` are used for storing path parameter names.
-///
-///
-/// For example:
-///
-/// `path` = `/user/123/book/345`
-/// `pattern` = `book/:id`
-///
-/// The return value = `/user/123`.
-String removePatternFromPath(String pattern, String path) {
- final StringBuffer buffer = StringBuffer();
- int start = 0;
- for (final RegExpMatch match in _parameterRegExp.allMatches(pattern)) {
- if (match.start > start) {
- buffer.write(RegExp.escape(pattern.substring(start, match.start)));
- }
- final String? optionalPattern = match[2];
- final String regex =
- optionalPattern != null ? _escapeGroup(optionalPattern) : '[^/]+';
- buffer.write(regex);
- start = match.end;
- }
-
- if (start < pattern.length) {
- buffer.write(RegExp.escape(pattern.substring(start)));
- }
-
- if (!pattern.endsWith('/')) {
- buffer.write(r'(?=/|$)');
- }
- buffer.write(r'$');
- final RegExp regexp = RegExp(buffer.toString(), caseSensitive: false);
- return path.replaceFirst(regexp, '');
-}
-
String _escapeGroup(String group, [String? name]) {
final String escapedGroup = group.replaceFirstMapped(
RegExp(r'[:=!]'), (Match match) => '\\${match[0]}');
diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml
index edcfd0a..46982a9 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: 6.0.0
+version: 6.0.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
diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart
index 560a429..969b329 100644
--- a/packages/go_router/test/go_router_test.dart
+++ b/packages/go_router/test/go_router_test.dart
@@ -953,6 +953,41 @@
]);
});
+ testWidgets('on pop twice', (WidgetTester tester) async {
+ final List<GoRoute> routes = <GoRoute>[
+ GoRoute(
+ path: '/',
+ builder: (_, __) => const DummyScreen(),
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'settings',
+ builder: (_, __) => const DummyScreen(),
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'profile',
+ builder: (_, __) => const DummyScreen(),
+ ),
+ ]),
+ ]),
+ ];
+
+ final GoRouter router = await createRouter(routes, tester,
+ initialLocation: '/settings/profile');
+
+ log.clear();
+ router.pop();
+ router.pop();
+ await tester.pumpAndSettle();
+ expect(log, <Object>[
+ isMethodCall('selectMultiEntryHistory', arguments: null),
+ isMethodCall('routeInformationUpdated', arguments: <String, dynamic>{
+ 'location': '/',
+ 'state': null,
+ 'replace': false
+ }),
+ ]);
+ });
+
testWidgets('on pop with path parameters', (WidgetTester tester) async {
final List<GoRoute> routes = <GoRoute>[
GoRoute(
@@ -1012,6 +1047,80 @@
]);
});
+ testWidgets('Can manually pop root navigator and display correct url',
+ (WidgetTester tester) async {
+ final GlobalKey<NavigatorState> rootNavigatorKey =
+ GlobalKey<NavigatorState>();
+
+ final List<RouteBase> routes = <RouteBase>[
+ GoRoute(
+ path: '/',
+ builder: (BuildContext context, GoRouterState state) {
+ return const Scaffold(
+ body: Text('Home'),
+ );
+ },
+ routes: <RouteBase>[
+ ShellRoute(
+ builder:
+ (BuildContext context, GoRouterState state, Widget child) {
+ return Scaffold(
+ appBar: AppBar(),
+ body: child,
+ );
+ },
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'b',
+ builder: (BuildContext context, GoRouterState state) {
+ return const Scaffold(
+ body: Text('Screen B'),
+ );
+ },
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'c',
+ builder: (BuildContext context, GoRouterState state) {
+ return const Scaffold(
+ body: Text('Screen C'),
+ );
+ },
+ ),
+ ],
+ ),
+ ],
+ ),
+ ],
+ ),
+ ];
+
+ await createRouter(routes, tester,
+ initialLocation: '/b/c', navigatorKey: rootNavigatorKey);
+ expect(find.text('Screen C'), findsOneWidget);
+ expect(log, <Object>[
+ isMethodCall('selectMultiEntryHistory', arguments: null),
+ isMethodCall('routeInformationUpdated', arguments: <String, dynamic>{
+ 'location': '/b/c',
+ 'state': null,
+ 'replace': false
+ }),
+ ]);
+
+ log.clear();
+ rootNavigatorKey.currentState!.pop();
+ await tester.pumpAndSettle();
+
+ expect(find.text('Home'), findsOneWidget);
+ expect(log, <Object>[
+ isMethodCall('selectMultiEntryHistory', arguments: null),
+ isMethodCall('routeInformationUpdated', arguments: <String, dynamic>{
+ 'location': '/',
+ 'state': null,
+ 'replace': false
+ }),
+ ]);
+ });
+
testWidgets('works correctly with async redirect',
(WidgetTester tester) async {
final UniqueKey login = UniqueKey();
