diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 075e950d61..c1fa3db30c 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 04c6656fb8..238dd35b62 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 @@ class RouteBuilder { final GoRouterStateRegistry _registry = GoRouterStateRegistry(); + final Map, RouteMatch> _routeMatchLookUp = + , 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 page) => + _routeMatchLookUp[page]; + + // final Map<> + /// Builds the top-level Navigator for the given [RouteMatchList]. Widget build( BuildContext context, @@ -57,6 +70,7 @@ class RouteBuilder { 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 @@ class RouteBuilder { GlobalKey navigatorKey, Map, GoRouterState> registry) { try { + assert(_routeMatchLookUp.isEmpty); final Map, List>> keyToPage = , List>>{}; _buildRecursive(context, matchList, 0, onPopPage, routerNeglect, keyToPage, navigatorKey, registry); + + // Every Page should have a corresponding RouteMatch. + assert(keyToPage.values.flattened + .every((Page page) => _routeMatchLookUp.containsKey(page))); return keyToPage[navigatorKey]!; } on _RouteBuilderError catch (e) { return >[ @@ -271,12 +290,13 @@ class RouteBuilder { 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 @@ class RouteBuilder { _cacheAppType(context); return _pageBuilderForAppType!( key: state.pageKey, - name: state.name ?? state.fullpath, + name: state.name ?? state.path, arguments: {...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 b34aac04f8..07bcdf0dbb 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -16,7 +16,7 @@ import 'typedefs.dart'; /// GoRouter implementation of [RouterDelegate]. class GoRouterDelegate extends RouterDelegate - with PopNavigatorRouterDelegateMixin, ChangeNotifier { + with ChangeNotifier { /// Constructor for GoRouter's implementation of the RouterDelegate base /// class. GoRouterDelegate({ @@ -132,7 +132,12 @@ class GoRouterDelegate extends RouterDelegate if (!route.didPop(result)) { return false; } - _matchList.pop(); + final Page page = route.settings as Page; + final RouteMatch? match = builder.getRouteMatchForPage(page); + if (match == null) { + return true; + } + _matchList.remove(match); notifyListeners(); assert(() { _debugAssertMatchListNotEmpty(); @@ -146,7 +151,7 @@ class GoRouterDelegate extends RouterDelegate /// 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 @@ class GoRouterDelegate extends RouterDelegate RouteMatchList get matches => _matchList; /// For use by the Router architecture as part of the RouterDelegate. - @override GlobalKey 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 9d01fa56fb..62a4d55005 100644 --- a/packages/go_router/lib/src/match.dart +++ b/packages/go_router/lib/src/match.dart @@ -73,6 +73,6 @@ class RouteMatch { /// 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 pageKey; } diff --git a/packages/go_router/lib/src/matching.dart b/packages/go_router/lib/src/matching.dart index 4297249f8b..140c1d1ea0 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 @@ class RouteMatchList { static RouteMatchList empty = RouteMatchList([], Uri.parse(''), const {}); - static String _generateFullPath(List matches) { + static String _generateFullPath(Iterable matches) { final StringBuffer buffer = StringBuffer(); bool addsSlash = false; for (final RouteMatch match in matches) { @@ -96,17 +97,27 @@ class RouteMatchList { _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 newParameters = []; + patternToRegExp(fullPath, newParameters); + final Set 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 d7a20ff22d..e9db923d71 100644 --- a/packages/go_router/lib/src/path_utils.dart +++ b/packages/go_router/lib/src/path_utils.dart @@ -47,44 +47,6 @@ RegExp patternToRegExp(String pattern, List parameters) { 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 edcfd0a077..46982a9f93 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 560a429d50..969b32993e 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -953,6 +953,41 @@ void main() { ]); }); + testWidgets('on pop twice', (WidgetTester tester) async { + final List routes = [ + GoRoute( + path: '/', + builder: (_, __) => const DummyScreen(), + routes: [ + GoRoute( + path: 'settings', + builder: (_, __) => const DummyScreen(), + routes: [ + 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, [ + isMethodCall('selectMultiEntryHistory', arguments: null), + isMethodCall('routeInformationUpdated', arguments: { + 'location': '/', + 'state': null, + 'replace': false + }), + ]); + }); + testWidgets('on pop with path parameters', (WidgetTester tester) async { final List routes = [ GoRoute( @@ -1012,6 +1047,80 @@ void main() { ]); }); + testWidgets('Can manually pop root navigator and display correct url', + (WidgetTester tester) async { + final GlobalKey rootNavigatorKey = + GlobalKey(); + + final List routes = [ + GoRoute( + path: '/', + builder: (BuildContext context, GoRouterState state) { + return const Scaffold( + body: Text('Home'), + ); + }, + routes: [ + ShellRoute( + builder: + (BuildContext context, GoRouterState state, Widget child) { + return Scaffold( + appBar: AppBar(), + body: child, + ); + }, + routes: [ + GoRoute( + path: 'b', + builder: (BuildContext context, GoRouterState state) { + return const Scaffold( + body: Text('Screen B'), + ); + }, + routes: [ + 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, [ + isMethodCall('selectMultiEntryHistory', arguments: null), + isMethodCall('routeInformationUpdated', arguments: { + 'location': '/b/c', + 'state': null, + 'replace': false + }), + ]); + + log.clear(); + rootNavigatorKey.currentState!.pop(); + await tester.pumpAndSettle(); + + expect(find.text('Home'), findsOneWidget); + expect(log, [ + isMethodCall('selectMultiEntryHistory', arguments: null), + isMethodCall('routeInformationUpdated', arguments: { + 'location': '/', + 'state': null, + 'replace': false + }), + ]); + }); + testWidgets('works correctly with async redirect', (WidgetTester tester) async { final UniqueKey login = UniqueKey();