From 26f660b52626912ba93f0318bcc412d0e99de4cd Mon Sep 17 00:00:00 2001 From: Valentin Vignal <32538273+ValentinVignal@users.noreply.github.com> Date: Thu, 25 Apr 2024 06:58:47 +0800 Subject: [PATCH] [go_router] Add `GoRouterState state` parameter to `GoRouterData.onExit` (#6495) Part of https://github.com/flutter/flutter/issues/137394 I need to add the `state` parameter to the `onExit` method so I can use the `factoryImpl(state)`: https://github.com/flutter/packages/blob/d4cd4f00254b2fdb50767c837ecd27bcbac488cd/packages/go_router/lib/src/route_data.dart#L100-L118 --- packages/go_router/CHANGELOG.md | 17 +++-- packages/go_router/README.md | 1 + packages/go_router/example/lib/on_exit.dart | 5 +- packages/go_router/lib/src/delegate.dart | 58 +++++++++----- packages/go_router/lib/src/route.dart | 3 +- packages/go_router/lib/src/route_data.dart | 9 +++ packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/on_exit_test.dart | 85 +++++++++++++++++++-- 8 files changed, 145 insertions(+), 35 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 4cb037ef2ada..d38b99e530a3 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,6 +1,7 @@ -## 13.2.5 +## 14.0.0 -- Fixes an issue where route future does not complete when popping shell route. +- **BREAKING CHANGE** + - `GoRouteData`'s `onExit` now takes 2 parameters `BuildContext context, GoRouterState state`. ## 13.2.4 @@ -30,7 +31,7 @@ ## 13.0.1 -* Fixes new lint warnings. +- Fixes new lint warnings. ## 13.0.0 @@ -41,12 +42,12 @@ ## 12.1.3 -* Fixes a typo in `navigation.md`. +- Fixes a typo in `navigation.md`. ## 12.1.2 -* Fixes an incorrect use of `extends` for Dart 3 compatibility. -* Updates minimum supported SDK version to Flutter 3.10/Dart 3.0. +- Fixes an incorrect use of `extends` for Dart 3 compatibility. +- Updates minimum supported SDK version to Flutter 3.10/Dart 3.0. ## 12.1.1 @@ -121,7 +122,7 @@ ## 10.1.2 -* Adds pub topics to package metadata. +- Adds pub topics to package metadata. ## 10.1.1 @@ -452,7 +453,7 @@ - Fixes a bug where intermediate route redirect methods are not called. - GoRouter implements the RouterConfig interface, allowing you to call - MaterialApp.router(routerConfig: _myGoRouter) instead of passing + MaterialApp.router(routerConfig: \_myGoRouter) instead of passing the RouterDelegate, RouteInformationParser, and RouteInformationProvider fields. - **BREAKING CHANGE** diff --git a/packages/go_router/README.md b/packages/go_router/README.md index a9f9b6b27111..7d506d0c3463 100644 --- a/packages/go_router/README.md +++ b/packages/go_router/README.md @@ -37,6 +37,7 @@ See the API documentation for details on the following topics: - [Error handling](https://pub.dev/documentation/go_router/latest/topics/Error%20handling-topic.html) ## Migration Guides +- [Migrating to 14.0.0](https://flutter.dev/go/go-router-v14-breaking-changes). - [Migrating to 13.0.0](https://flutter.dev/go/go-router-v13-breaking-changes). - [Migrating to 12.0.0](https://flutter.dev/go/go-router-v12-breaking-changes). - [Migrating to 11.0.0](https://flutter.dev/go/go-router-v11-breaking-changes). diff --git a/packages/go_router/example/lib/on_exit.dart b/packages/go_router/example/lib/on_exit.dart index fba83a7d1fd5..bf397efedd3f 100644 --- a/packages/go_router/example/lib/on_exit.dart +++ b/packages/go_router/example/lib/on_exit.dart @@ -22,7 +22,10 @@ final GoRouter _router = GoRouter( builder: (BuildContext context, GoRouterState state) { return const DetailsScreen(); }, - onExit: (BuildContext context) async { + onExit: ( + BuildContext context, + GoRouterState state, + ) async { final bool? confirmed = await showDialog( context: context, builder: (_) { diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index bf5840cbad4c..f0185893bb3b 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -66,13 +66,17 @@ class GoRouterDelegate extends RouterDelegate } walker = walker.matches.last; } + assert(walker is RouteMatch); if (state != null) { return state.maybePop(); } // This should be the only place where the last GoRoute exit the screen. final GoRoute lastRoute = currentConfiguration.last.route; if (lastRoute.onExit != null && navigatorKey.currentContext != null) { - return !(await lastRoute.onExit!(navigatorKey.currentContext!)); + return !(await lastRoute.onExit!( + navigatorKey.currentContext!, + walker.buildState(_configuration, currentConfiguration), + )); } return false; } @@ -137,8 +141,10 @@ class GoRouterDelegate extends RouterDelegate // a microtask in case the onExit callback want to launch dialog or other // navigator operations. scheduleMicrotask(() async { - final bool onExitResult = - await routeBase.onExit!(navigatorKey.currentContext!); + final bool onExitResult = await routeBase.onExit!( + navigatorKey.currentContext!, + match.buildState(_configuration, currentConfiguration), + ); if (onExitResult) { _completeRouteMatch(result, match); } @@ -221,14 +227,14 @@ class GoRouterDelegate extends RouterDelegate } if (indexOfFirstDiff < currentGoRouteMatches.length) { - final List exitingGoRoutes = currentGoRouteMatches - .sublist(indexOfFirstDiff) - .map((RouteMatch match) => match.route) - .whereType() - .toList(); - return _callOnExitStartsAt(exitingGoRoutes.length - 1, - context: navigatorContext, routes: exitingGoRoutes) - .then((bool exit) { + final List exitingMatches = + currentGoRouteMatches.sublist(indexOfFirstDiff).toList(); + return _callOnExitStartsAt( + exitingMatches.length - 1, + context: navigatorContext, + matches: exitingMatches, + configuration: configuration, + ).then((bool exit) { if (!exit) { return SynchronousFuture(null); } @@ -244,24 +250,42 @@ class GoRouterDelegate extends RouterDelegate /// /// The returned future resolves to true if all routes below the index all /// return true. Otherwise, the returned future resolves to false. - static Future _callOnExitStartsAt(int index, - {required BuildContext context, required List routes}) { + Future _callOnExitStartsAt( + int index, { + required BuildContext context, + required List matches, + required RouteMatchList configuration, + }) { if (index < 0) { return SynchronousFuture(true); } - final GoRoute goRoute = routes[index]; + final RouteMatch match = matches[index]; + final GoRoute goRoute = match.route; if (goRoute.onExit == null) { - return _callOnExitStartsAt(index - 1, context: context, routes: routes); + return _callOnExitStartsAt( + index - 1, + context: context, + matches: matches, + configuration: configuration, + ); } Future handleOnExitResult(bool exit) { if (exit) { - return _callOnExitStartsAt(index - 1, context: context, routes: routes); + return _callOnExitStartsAt( + index - 1, + context: context, + matches: matches, + configuration: configuration, + ); } return SynchronousFuture(false); } - final FutureOr exitFuture = goRoute.onExit!(context); + final FutureOr exitFuture = goRoute.onExit!( + context, + match.buildState(_configuration, configuration), + ); if (exitFuture is bool) { return handleOnExitResult(exitFuture); } diff --git a/packages/go_router/lib/src/route.dart b/packages/go_router/lib/src/route.dart index 2fa4bd04640a..69dd904f087d 100644 --- a/packages/go_router/lib/src/route.dart +++ b/packages/go_router/lib/src/route.dart @@ -63,7 +63,8 @@ typedef NavigatorBuilder = Widget Function( /// /// If the return value is true or the future resolve to true, the route will /// exit as usual. Otherwise, the operation will abort. -typedef ExitCallback = FutureOr Function(BuildContext context); +typedef ExitCallback = FutureOr Function( + BuildContext context, GoRouterState state); /// The base class for [GoRoute] and [ShellRoute]. /// diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index 26a6fc42a96a..b10f2b11158f 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -63,6 +63,11 @@ abstract class GoRouteData extends RouteData { /// Corresponds to [GoRoute.redirect]. FutureOr redirect(BuildContext context, GoRouterState state) => null; + /// Called when this route is removed from GoRouter's route history. + /// + /// Corresponds to [GoRoute.onExit]. + FutureOr onExit(BuildContext context, GoRouterState state) => true; + /// A helper function used by generated code. /// /// Should not be used directly. @@ -106,6 +111,9 @@ abstract class GoRouteData extends RouteData { FutureOr redirect(BuildContext context, GoRouterState state) => factoryImpl(state).redirect(context, state); + FutureOr onExit(BuildContext context, GoRouterState state) => + factoryImpl(state).onExit(context, state); + return GoRoute( path: path, name: name, @@ -114,6 +122,7 @@ abstract class GoRouteData extends RouteData { redirect: redirect, routes: routes, parentNavigatorKey: parentNavigatorKey, + onExit: onExit, ); } diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 25ec466df0d2..febb4db4ddbf 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: 13.2.5 +version: 14.0.0 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/on_exit_test.dart b/packages/go_router/test/on_exit_test.dart index 044a6ff399bc..ceeb6806d156 100644 --- a/packages/go_router/test/on_exit_test.dart +++ b/packages/go_router/test/on_exit_test.dart @@ -25,7 +25,7 @@ void main() { path: '1', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: page1), - onExit: (BuildContext context) { + onExit: (BuildContext context, GoRouterState state) { return allow; }, ) @@ -61,7 +61,7 @@ void main() { path: '/1', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: page1), - onExit: (BuildContext context) { + onExit: (BuildContext context, GoRouterState state) { return allow; }, ) @@ -95,7 +95,7 @@ void main() { path: '1', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: page1), - onExit: (BuildContext context) async { + onExit: (BuildContext context, GoRouterState state) async { return allow.future; }, ) @@ -139,7 +139,7 @@ void main() { path: '/1', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: page1), - onExit: (BuildContext context) async { + onExit: (BuildContext context, GoRouterState state) async { return allow.future; }, ) @@ -176,7 +176,7 @@ void main() { path: '/', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: home), - onExit: (BuildContext context) { + onExit: (BuildContext context, GoRouterState state) { return allow; }, ), @@ -201,7 +201,7 @@ void main() { path: '/', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: home), - onExit: (BuildContext context) async { + onExit: (BuildContext context, GoRouterState state) async { return allow; }, ), @@ -227,7 +227,7 @@ void main() { path: '/', builder: (BuildContext context, GoRouterState state) => DummyScreen(key: home), - onExit: (BuildContext context) { + onExit: (BuildContext context, GoRouterState state) { return allow; }, ), @@ -243,4 +243,75 @@ void main() { allow = true; expect(await router.routerDelegate.popRoute(), false); }); + + testWidgets('It should provide the correct state to the onExit callback', + (WidgetTester tester) async { + final UniqueKey home = UniqueKey(); + final UniqueKey page1 = UniqueKey(); + final UniqueKey page2 = UniqueKey(); + final UniqueKey page3 = UniqueKey(); + late final GoRouterState onExitState1; + late final GoRouterState onExitState2; + late final GoRouterState onExitState3; + final List routes = [ + GoRoute( + path: '/', + builder: (BuildContext context, GoRouterState state) => + DummyScreen(key: home), + routes: [ + GoRoute( + path: '1', + builder: (BuildContext context, GoRouterState state) => + DummyScreen(key: page1), + onExit: (BuildContext context, GoRouterState state) { + onExitState1 = state; + return true; + }, + routes: [ + GoRoute( + path: '2', + builder: (BuildContext context, GoRouterState state) => + DummyScreen(key: page2), + onExit: (BuildContext context, GoRouterState state) { + onExitState2 = state; + return true; + }, + routes: [ + GoRoute( + path: '3', + builder: (BuildContext context, GoRouterState state) => + DummyScreen(key: page3), + onExit: (BuildContext context, GoRouterState state) { + onExitState3 = state; + return true; + }, + ) + ], + ) + ], + ) + ], + ), + ]; + + final GoRouter router = + await createRouter(routes, tester, initialLocation: '/1/2/3'); + expect(find.byKey(page3), findsOneWidget); + + router.pop(); + await tester.pumpAndSettle(); + expect(find.byKey(page2), findsOneWidget); + + expect(onExitState3.uri.toString(), '/1/2/3'); + + router.pop(); + await tester.pumpAndSettle(); + expect(find.byKey(page1), findsOneWidget); + expect(onExitState2.uri.toString(), '/1/2'); + + router.pop(); + await tester.pumpAndSettle(); + expect(find.byKey(home), findsOneWidget); + expect(onExitState1.uri.toString(), '/1'); + }); }