From ac19a52ad0043c1f2d60235e073aa247118f1149 Mon Sep 17 00:00:00 2001 From: hangyu <108393416+hangyujin@users.noreply.github.com> Date: Wed, 23 Aug 2023 18:53:33 -0700 Subject: [PATCH 1/6] 1 --- packages/go_router/CHANGELOG.md | 4 ++ packages/go_router/lib/src/builder.dart | 25 +++++++----- packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/go_router_test.dart | 44 +++++++++++++++++++++ 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 138c02d9edd..1f883ceb6ca 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -2,6 +2,10 @@ * Updates minimum supported SDK version to Flutter 3.7/Dart 2.19. +## 10.1.1 + +- Fixes mapping from `Page` to `RouteMatch`s. + ## 10.1.0 - Supports setting `requestFocus`. diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index 15c58a96da7..b466827cf6f 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/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 page) => - pagePopContext.getRouteMatchForPage(page) != null)); + pagePopContext.getRouteMatchesForPage(page) != null)); } /// Clean up previous cache to prevent memory leak, making sure any nested @@ -299,7 +299,7 @@ class RouteBuilder { } if (page != null) { registry[page] = state; - pagePopContext._setRouteMatchForPage(page, match); + pagePopContext._addRouteMatchForPage(page, match); } } @@ -561,8 +561,8 @@ typedef _PageBuilderForAppType = Page Function({ class _PagePopContext { _PagePopContext._(this.onPopPageWithRouteMatch); - final Map, RouteMatch> _routeMatchLookUp = - , RouteMatch>{}; + final Map, List> _routeMatchesLookUp = + , List>{}; /// On pop page callback that includes the associated [RouteMatch]. final PopPageWithRouteMatchCallback onPopPageWithRouteMatch; @@ -571,18 +571,25 @@ class _PagePopContext { /// /// The [Page] must have been previously built via the [RouteBuilder] that /// created this [PagePopContext]; otherwise, this method returns null. - RouteMatch? getRouteMatchForPage(Page page) => - _routeMatchLookUp[page]; + List? getRouteMatchesForPage(Page page) => + _routeMatchesLookUp[page]; - void _setRouteMatchForPage(Page page, RouteMatch match) => - _routeMatchLookUp[page] = match; + void _addRouteMatchForPage(Page page, RouteMatch match) { + _routeMatchesLookUp.putIfAbsent(page, () => []).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 route, dynamic result) { final Page page = route.settings as Page; - return onPopPageWithRouteMatch(route, result, _routeMatchLookUp[page]); + + final RouteMatch match = _routeMatchesLookUp[page]!.last; + _routeMatchesLookUp[page]!.removeLast(); + + return onPopPageWithRouteMatch(route, result, match); } } diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 3bcafc3e151..3e735c0b6cc 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: 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 diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index e840b00cbd7..23ba9271a67 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -309,6 +309,50 @@ 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 navKey = GlobalKey(); + final List routes = [ + GoRoute( + path: '/', + pageBuilder: (_, __) => const MaterialPage(child: HomeScreen()), + ), + GoRoute( + path: '/page1', + pageBuilder: (_, __) => const MaterialPage(child: Page1Screen()), + ), + GoRoute( + path: '/page2', + pageBuilder: (_, __) => const MaterialPage(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 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 routes = [ GoRoute( From a4f27c68515b042918fbc83dfab157c508405f29 Mon Sep 17 00:00:00 2001 From: hangyu <108393416+hangyujin@users.noreply.github.com> Date: Thu, 24 Aug 2023 11:33:51 -0700 Subject: [PATCH 2/6] Update go_router_test.dart --- packages/go_router/test/go_router_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index 23ba9271a67..fcc75247257 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -317,15 +317,15 @@ void main() { final List routes = [ GoRoute( path: '/', - pageBuilder: (_, __) => const MaterialPage(child: HomeScreen()), + pageBuilder: (_, __) => const MaterialPage(child: HomeScreen()), ), GoRoute( path: '/page1', - pageBuilder: (_, __) => const MaterialPage(child: Page1Screen()), + pageBuilder: (_, __) => const MaterialPage(child: Page1Screen()), ), GoRoute( path: '/page2', - pageBuilder: (_, __) => const MaterialPage(child: Page2Screen()), + pageBuilder: (_, __) => const MaterialPage(child: Page2Screen()), ), ]; final GoRouter router = From fea60d9cd4242004d827cc6deebe6a60c5b5af5d Mon Sep 17 00:00:00 2001 From: hangyu <108393416+hangyujin@users.noreply.github.com> Date: Thu, 24 Aug 2023 11:41:21 -0700 Subject: [PATCH 3/6] lint --- packages/go_router/lib/src/builder.dart | 6 ++++-- packages/go_router/test/go_router_test.dart | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index b466827cf6f..f0816b7d1b2 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -575,14 +575,16 @@ class _PagePopContext { _routeMatchesLookUp[page]; void _addRouteMatchForPage(Page page, RouteMatch match) { - _routeMatchesLookUp.putIfAbsent(page, () => []).insert(0, match); + _routeMatchesLookUp + .putIfAbsent(page, () => []) + .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 route, dynamic result) { final Page page = route.settings as Page; diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index fcc75247257..755b3e3b9bd 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -317,15 +317,18 @@ void main() { final List routes = [ GoRoute( path: '/', - pageBuilder: (_, __) => const MaterialPage(child: HomeScreen()), + pageBuilder: (_, __) => + const MaterialPage(child: HomeScreen()), ), GoRoute( path: '/page1', - pageBuilder: (_, __) => const MaterialPage(child: Page1Screen()), + pageBuilder: (_, __) => + const MaterialPage(child: Page1Screen()), ), GoRoute( path: '/page2', - pageBuilder: (_, __) => const MaterialPage(child: Page2Screen()), + pageBuilder: (_, __) => + const MaterialPage(child: Page2Screen()), ), ]; final GoRouter router = From 04da4625c59dd9c33ccb953c9aa4b4bf29df97ff Mon Sep 17 00:00:00 2001 From: hangyu <108393416+hangyujin@users.noreply.github.com> Date: Thu, 24 Aug 2023 11:46:40 -0700 Subject: [PATCH 4/6] Update builder.dart --- packages/go_router/lib/src/builder.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index f0816b7d1b2..fca1e428bd5 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -299,7 +299,8 @@ class RouteBuilder { } if (page != null) { registry[page] = state; - pagePopContext._addRouteMatchForPage(page, match); + // Insert the route match in reverse order. + pagePopContext._insertRouteMatchAtStartForPage(page, match); } } @@ -574,7 +575,8 @@ class _PagePopContext { List? getRouteMatchesForPage(Page page) => _routeMatchesLookUp[page]; - void _addRouteMatchForPage(Page page, RouteMatch match) { + /// This is called in _buildRecursive to insert route matches in reverse order. + void _insertRouteMatchAtStartForPage(Page page, RouteMatch match) { _routeMatchesLookUp .putIfAbsent(page, () => []) .insert(0, match); From d1d72f3c89c94601c03782111f582dff28126448 Mon Sep 17 00:00:00 2001 From: hangyu <108393416+hangyujin@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:31:48 -0700 Subject: [PATCH 5/6] Update builder.dart --- packages/go_router/lib/src/builder.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index fca1e428bd5..1e46417b30f 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -562,6 +562,8 @@ typedef _PageBuilderForAppType = Page Function({ class _PagePopContext { _PagePopContext._(this.onPopPageWithRouteMatch); + /// A page can be mapped to a RouteMatch list, such as a const page being + /// pushed multiple times. final Map, List> _routeMatchesLookUp = , List>{}; From eaaa2a14f093fd01db0477625fdd2277309e7071 Mon Sep 17 00:00:00 2001 From: hangyu <108393416+hangyujin@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:33:06 -0700 Subject: [PATCH 6/6] Update CHANGELOG.md --- packages/go_router/CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 1f883ceb6ca..de453b43092 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,10 +1,7 @@ -## NEXT - -* Updates minimum supported SDK version to Flutter 3.7/Dart 2.19. - ## 10.1.1 - Fixes mapping from `Page` to `RouteMatch`s. +- Updates minimum supported SDK version to Flutter 3.7/Dart 2.19. ## 10.1.0