From 3f48e2291df96ec96157453760d23d646cbac278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Sun, 23 Mar 2025 18:00:13 +0000 Subject: [PATCH 01/15] fix: call route.didPop(result) when onExit returns true --- packages/go_router/lib/src/delegate.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index aa95fdb39eb3..21f9c2e05333 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -151,6 +151,7 @@ class GoRouterDelegate extends RouterDelegate match.buildState(_configuration, currentConfiguration), ); if (onExitResult) { + route.didPop(result); _completeRouteMatch(result, match); } }); From f8d2118cd7fad4d19df241143513360e2ea47cc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Sun, 23 Mar 2025 18:00:25 +0000 Subject: [PATCH 02/15] test: add tests for PopScope onPopInvokedWithResult behavior --- ..._exit_on_pop_invoked_with_result_test.dart | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart diff --git a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart new file mode 100644 index 000000000000..8ba572038cb6 --- /dev/null +++ b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart @@ -0,0 +1,138 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; + +void main() { + testWidgets( + 'PopScope onPopInvokedWithResult should be called only once', + (WidgetTester tester) async { + int counter = 0; + + final GoRouter goRouter = GoRouter( + initialLocation: '/', + routes: [ + GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), + GoRoute( + path: '/page-1', + onExit: (_, __) => true, + builder: (_, __) => PopScope( + onPopInvokedWithResult: (bool didPop, __) { + if (didPop) { + counter++; + return; + } + }, + child: const Text('Page 1'), + ), + ), + ], + ); + + addTearDown(goRouter.dispose); + + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + + goRouter.push('/page-1'); + + await tester.pumpAndSettle(); + + expect(find.text('Page 1'), findsOneWidget); + expect( + goRouter.routerDelegate.currentConfiguration.matches.length, + equals(2), + ); + expect(goRouter.routerDelegate.canPop(), true); + + goRouter.routerDelegate.pop(); + + await tester.pumpAndSettle(); + + expect(counter, equals(1)); + }, + ); + + testWidgets( + r'PopScope onPopInvokedWithResult should be called only once with GoRouteData.$route', + (WidgetTester tester) async { + int counter = 0; + + final GoRouter goRouter = GoRouter( + initialLocation: '/', + routes: [ + GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), + GoRouteData.$route( + path: '/page-1', + factory: (GoRouterState state) => _Page1( + onPop: () { + counter++; + }, + ), + ), + ], + ); + + addTearDown(goRouter.dispose); + + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + + goRouter.push('/page-1'); + + await tester.pumpAndSettle(); + + expect(find.text('Page 1'), findsOneWidget); + expect( + goRouter.routerDelegate.currentConfiguration.matches.length, + equals(2), + ); + expect(goRouter.routerDelegate.canPop(), true); + + goRouter.routerDelegate.pop(); + + await tester.pumpAndSettle(); + + expect(counter, equals(1)); + }, + ); +} + +class _Page1 extends GoRouteData { + const _Page1({ + required this.onPop, + }); + + final VoidCallback onPop; + + @override + Page buildPage(BuildContext context, GoRouterState state) => + MaterialPage( + child: PopScope( + onPopInvokedWithResult: (bool didPop, __) { + if (didPop) { + onPop(); + return; + } + }, + child: const Text('Page 1'), + ), + ); +} + +class DummyStatefulWidget extends StatefulWidget { + const DummyStatefulWidget({super.key}); + + @override + State createState() => _DummyStatefulWidgetState(); +} + +class _DummyStatefulWidgetState extends State { + @override + Widget build(BuildContext context) => Container(); +} From e001872381b7ce289b825ce0f567c774de146d56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Sun, 23 Mar 2025 18:20:27 +0000 Subject: [PATCH 03/15] chore: bump version to 14.8.2-dev in pubspec.yaml --- packages/go_router/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 86b0e9643fa9..a14d52f57859 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: 14.8.1 +version: 14.8.2-dev 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 From d3dfd249d499f9ea61550557478ed8a95e84793e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Sun, 23 Mar 2025 18:21:29 +0000 Subject: [PATCH 04/15] chore: update CHANGELOG for version 14.8.2-dec with PopScope fix --- packages/go_router/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 19a6cdf7061c..0628882cf701 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 14.8.2-dec + +- Fixes `PopScope.onPopInvokedWithResult` getting called twice when popping a route when `GoRouteData.onExit` is not null + ## 14.8.1 - Secured canPop method for the lack of matches in routerDelegate's configuration. From 88d973903d123cd4a5897c31763071b370e6266e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Sun, 23 Mar 2025 18:26:38 +0000 Subject: [PATCH 05/15] chore: fix typo in changelog --- packages/go_router/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 0628882cf701..d746d9080fa7 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,4 +1,4 @@ -## 14.8.2-dec +## 14.8.2-dev - Fixes `PopScope.onPopInvokedWithResult` getting called twice when popping a route when `GoRouteData.onExit` is not null From fcd8dfc149b65a1ccd854420529ec66844dbaf8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Wed, 26 Mar 2025 05:11:28 +0000 Subject: [PATCH 06/15] chore: fix version number --- packages/go_router/CHANGELOG.md | 4 ++-- packages/go_router/pubspec.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index d746d9080fa7..e1d312b1a3d0 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,6 +1,6 @@ -## 14.8.2-dev +## 14.8.2 -- Fixes `PopScope.onPopInvokedWithResult` getting called twice when popping a route when `GoRouteData.onExit` is not null +- Fixes `PopScope.onPopInvokedWithResult` getting called twice when popping a route if `GoRouteData.onExit` is not null ## 14.8.1 diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index a14d52f57859..1f2b5fb1649f 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: 14.8.2-dev +version: 14.8.2 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 From ec6137ed0676abf40a678282d52ab9111492514c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Tue, 1 Apr 2025 21:32:18 +0100 Subject: [PATCH 07/15] fix: update onExit callback to handle null values and improve type safety --- packages/go_router/lib/src/delegate.dart | 24 ++++++++++++------- packages/go_router/lib/src/route.dart | 3 ++- packages/go_router/lib/src/route_data.dart | 6 ++--- ..._exit_on_pop_invoked_with_result_test.dart | 1 - 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index 21f9c2e05333..71809334eada 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -66,11 +66,12 @@ class GoRouterDelegate extends RouterDelegate // Fallback to onExit if maybePop did not handle the pop final GoRoute lastRoute = currentConfiguration.last.route; if (lastRoute.onExit != null && navigatorKey.currentContext != null) { - return !(await lastRoute.onExit!( - navigatorKey.currentContext!, - currentConfiguration.last - .buildState(_configuration, currentConfiguration), - )); + return !(await lastRoute.onExit?.call( + navigatorKey.currentContext!, + currentConfiguration.last + .buildState(_configuration, currentConfiguration), + ) ?? + true); // @TODO(techouse) not sure if returning true by default is correct } return false; @@ -146,12 +147,14 @@ 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!( + final bool? onExitResult = await routeBase.onExit?.call( navigatorKey.currentContext!, match.buildState(_configuration, currentConfiguration), ); - if (onExitResult) { + if (onExitResult == null) { route.didPop(result); + } + if (onExitResult ?? true) { _completeRouteMatch(result, match); } }); @@ -299,14 +302,17 @@ class GoRouterDelegate extends RouterDelegate return SynchronousFuture(false); } - final FutureOr exitFuture = goRoute.onExit!( + final FutureOr? exitFuture = goRoute.onExit?.call( context, match.buildState(_configuration, currentConfiguration), ); if (exitFuture is bool) { return handleOnExitResult(exitFuture); } - return exitFuture.then(handleOnExitResult); + return exitFuture?.then(handleOnExitResult) ?? + SynchronousFuture( + true, // @TODO(techouse) not sure if returning true by default is correct + ); } Future _setCurrentConfiguration(RouteMatchList configuration) { diff --git a/packages/go_router/lib/src/route.dart b/packages/go_router/lib/src/route.dart index 703e470e9a65..dc7566851535 100644 --- a/packages/go_router/lib/src/route.dart +++ b/packages/go_router/lib/src/route.dart @@ -67,7 +67,7 @@ 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( +typedef ExitCallback = FutureOr? Function( BuildContext context, GoRouterState state); /// The base class for [GoRoute] and [ShellRoute]. @@ -1441,6 +1441,7 @@ class _RestorableRouteMatchList extends RestorableProperty { RouteMatchList get value => _value; RouteMatchList _value = RouteMatchList.empty; + set value(RouteMatchList newValue) { if (newValue != _value) { _value = newValue; diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index a088cc96652b..691c58df1b78 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -66,7 +66,7 @@ abstract class GoRouteData extends RouteData { /// Called when this route is removed from GoRouter's route history. /// /// Corresponds to [GoRoute.onExit]. - FutureOr onExit(BuildContext context, GoRouterState state) => true; + ExitCallback? get onExit => null; /// A helper function used by generated code. /// @@ -111,8 +111,8 @@ 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); + FutureOr? onExit(BuildContext context, GoRouterState state) => + factoryImpl(state).onExit?.call(context, state); return GoRoute( path: path, diff --git a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart index 8ba572038cb6..844098248f4a 100644 --- a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart +++ b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart @@ -14,7 +14,6 @@ void main() { GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), GoRoute( path: '/page-1', - onExit: (_, __) => true, builder: (_, __) => PopScope( onPopInvokedWithResult: (bool didPop, __) { if (didPop) { From 469c8cf3c12f682f72a10e6b88a14eb2193f3220 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Tue, 1 Apr 2025 22:50:09 +0100 Subject: [PATCH 08/15] :recycle: convert SubRoute.onExit to a getter --- .../example/lib/on_exit_example.dart | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/go_router_builder/example/lib/on_exit_example.dart b/packages/go_router_builder/example/lib/on_exit_example.dart index b574c852c6e4..0b2cdba4754a 100644 --- a/packages/go_router_builder/example/lib/on_exit_example.dart +++ b/packages/go_router_builder/example/lib/on_exit_example.dart @@ -40,25 +40,25 @@ class SubRoute extends GoRouteData { const SubRoute(); @override - Future onExit(BuildContext context, GoRouterState state) async { - final bool? confirmed = await showDialog( - context: context, - builder: (_) => AlertDialog( - content: const Text('Are you sure to leave this page?'), - actions: [ - TextButton( - onPressed: () => Navigator.of(context).pop(false), - child: const Text('Cancel'), + ExitCallback get onExit => (BuildContext context, GoRouterState state) async { + final bool? confirmed = await showDialog( + context: context, + builder: (_) => AlertDialog( + content: const Text('Are you sure to leave this page?'), + actions: [ + TextButton( + onPressed: () => Navigator.of(context).pop(false), + child: const Text('Cancel'), + ), + ElevatedButton( + onPressed: () => Navigator.of(context).pop(true), + child: const Text('Confirm'), + ), + ], ), - ElevatedButton( - onPressed: () => Navigator.of(context).pop(true), - child: const Text('Confirm'), - ), - ], - ), - ); - return confirmed ?? false; - } + ); + return confirmed ?? false; + }; @override Widget build(BuildContext context, GoRouterState state) => const SubScreen(); From 4a83178a85f6bc842a4aedbafb5d8dd209e655f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 19:54:04 +0100 Subject: [PATCH 09/15] fix: ensure non-null onExit callback usage to prevent potential null dereference --- packages/go_router/lib/src/delegate.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/go_router/lib/src/delegate.dart b/packages/go_router/lib/src/delegate.dart index 71809334eada..bc0751b6df3b 100644 --- a/packages/go_router/lib/src/delegate.dart +++ b/packages/go_router/lib/src/delegate.dart @@ -66,7 +66,7 @@ class GoRouterDelegate extends RouterDelegate // Fallback to onExit if maybePop did not handle the pop final GoRoute lastRoute = currentConfiguration.last.route; if (lastRoute.onExit != null && navigatorKey.currentContext != null) { - return !(await lastRoute.onExit?.call( + return !(await lastRoute.onExit!.call( navigatorKey.currentContext!, currentConfiguration.last .buildState(_configuration, currentConfiguration), @@ -147,7 +147,7 @@ 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?.call( + final bool? onExitResult = await routeBase.onExit!.call( navigatorKey.currentContext!, match.buildState(_configuration, currentConfiguration), ); From 9accb643b6ed86d7633133dfa5f983d9eb70e5a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 20:17:27 +0100 Subject: [PATCH 10/15] fix: update onExit to return a FutureOr and ensure proper callback invocation --- packages/go_router/lib/src/route_data.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index 899dc8fe209c..a5252a5b4735 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -66,7 +66,7 @@ abstract class GoRouteData extends RouteData { /// Called when this route is removed from GoRouter's route history. /// /// Corresponds to [GoRoute.onExit]. - ExitCallback? get onExit => null; + FutureOr? onExit(BuildContext context, GoRouterState state) => null; /// A helper function used by generated code. /// @@ -113,7 +113,7 @@ abstract class GoRouteData extends RouteData { factoryImpl(state).redirect(context, state); FutureOr? onExit(BuildContext context, GoRouterState state) => - factoryImpl(state).onExit?.call(context, state); + factoryImpl(state).onExit(context, state); return GoRoute( path: path, From c3e6acea76f96be5e285501c90aa14215ffc1c62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 20:18:59 +0100 Subject: [PATCH 11/15] chore: revert change --- .../example/lib/on_exit_example.dart | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/go_router_builder/example/lib/on_exit_example.dart b/packages/go_router_builder/example/lib/on_exit_example.dart index 0b2cdba4754a..b574c852c6e4 100644 --- a/packages/go_router_builder/example/lib/on_exit_example.dart +++ b/packages/go_router_builder/example/lib/on_exit_example.dart @@ -40,25 +40,25 @@ class SubRoute extends GoRouteData { const SubRoute(); @override - ExitCallback get onExit => (BuildContext context, GoRouterState state) async { - final bool? confirmed = await showDialog( - context: context, - builder: (_) => AlertDialog( - content: const Text('Are you sure to leave this page?'), - actions: [ - TextButton( - onPressed: () => Navigator.of(context).pop(false), - child: const Text('Cancel'), - ), - ElevatedButton( - onPressed: () => Navigator.of(context).pop(true), - child: const Text('Confirm'), - ), - ], + Future onExit(BuildContext context, GoRouterState state) async { + final bool? confirmed = await showDialog( + context: context, + builder: (_) => AlertDialog( + content: const Text('Are you sure to leave this page?'), + actions: [ + TextButton( + onPressed: () => Navigator.of(context).pop(false), + child: const Text('Cancel'), ), - ); - return confirmed ?? false; - }; + ElevatedButton( + onPressed: () => Navigator.of(context).pop(true), + child: const Text('Confirm'), + ), + ], + ), + ); + return confirmed ?? false; + } @override Widget build(BuildContext context, GoRouterState state) => const SubScreen(); From 0b3119ff16ecf0aa874e37c591235cb5704bc247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 20:29:02 +0100 Subject: [PATCH 12/15] chore: add copyright notice to on_exit_on_pop_invoked_with_result_test.dart --- .../test/on_exit_on_pop_invoked_with_result_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart index 844098248f4a..900b0214ab1b 100644 --- a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart +++ b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart @@ -1,3 +1,7 @@ +// Copyright 2015 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; From c4f47b7ce78aa066531f42d161229966f3ee370f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 20:29:09 +0100 Subject: [PATCH 13/15] fix: bump version to 15.1.2 --- packages/go_router/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 31e9debc0b99..dcf98a3d9839 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: 15.1.1 +version: 15.1.2 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 From 2dff8e86f320ba4fabf9739d8b200283d44c42d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 20:47:02 +0100 Subject: [PATCH 14/15] test: move onPopInvokedWithResult tests to on_exit_test.dart --- ..._exit_on_pop_invoked_with_result_test.dart | 141 ------------------ packages/go_router/test/on_exit_test.dart | 132 ++++++++++++++++ 2 files changed, 132 insertions(+), 141 deletions(-) delete mode 100644 packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart diff --git a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart b/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart deleted file mode 100644 index 900b0214ab1b..000000000000 --- a/packages/go_router/test/on_exit_on_pop_invoked_with_result_test.dart +++ /dev/null @@ -1,141 +0,0 @@ -// Copyright 2015 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:go_router/go_router.dart'; - -void main() { - testWidgets( - 'PopScope onPopInvokedWithResult should be called only once', - (WidgetTester tester) async { - int counter = 0; - - final GoRouter goRouter = GoRouter( - initialLocation: '/', - routes: [ - GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), - GoRoute( - path: '/page-1', - builder: (_, __) => PopScope( - onPopInvokedWithResult: (bool didPop, __) { - if (didPop) { - counter++; - return; - } - }, - child: const Text('Page 1'), - ), - ), - ], - ); - - addTearDown(goRouter.dispose); - - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - - goRouter.push('/page-1'); - - await tester.pumpAndSettle(); - - expect(find.text('Page 1'), findsOneWidget); - expect( - goRouter.routerDelegate.currentConfiguration.matches.length, - equals(2), - ); - expect(goRouter.routerDelegate.canPop(), true); - - goRouter.routerDelegate.pop(); - - await tester.pumpAndSettle(); - - expect(counter, equals(1)); - }, - ); - - testWidgets( - r'PopScope onPopInvokedWithResult should be called only once with GoRouteData.$route', - (WidgetTester tester) async { - int counter = 0; - - final GoRouter goRouter = GoRouter( - initialLocation: '/', - routes: [ - GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), - GoRouteData.$route( - path: '/page-1', - factory: (GoRouterState state) => _Page1( - onPop: () { - counter++; - }, - ), - ), - ], - ); - - addTearDown(goRouter.dispose); - - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - - goRouter.push('/page-1'); - - await tester.pumpAndSettle(); - - expect(find.text('Page 1'), findsOneWidget); - expect( - goRouter.routerDelegate.currentConfiguration.matches.length, - equals(2), - ); - expect(goRouter.routerDelegate.canPop(), true); - - goRouter.routerDelegate.pop(); - - await tester.pumpAndSettle(); - - expect(counter, equals(1)); - }, - ); -} - -class _Page1 extends GoRouteData { - const _Page1({ - required this.onPop, - }); - - final VoidCallback onPop; - - @override - Page buildPage(BuildContext context, GoRouterState state) => - MaterialPage( - child: PopScope( - onPopInvokedWithResult: (bool didPop, __) { - if (didPop) { - onPop(); - return; - } - }, - child: const Text('Page 1'), - ), - ); -} - -class DummyStatefulWidget extends StatefulWidget { - const DummyStatefulWidget({super.key}); - - @override - State createState() => _DummyStatefulWidgetState(); -} - -class _DummyStatefulWidgetState extends State { - @override - Widget build(BuildContext context) => Container(); -} diff --git a/packages/go_router/test/on_exit_test.dart b/packages/go_router/test/on_exit_test.dart index 8a7bd83ab648..5ce16033eb07 100644 --- a/packages/go_router/test/on_exit_test.dart +++ b/packages/go_router/test/on_exit_test.dart @@ -471,4 +471,136 @@ void main() { expect(onExitState2.fullPath, '/route-2/:id2'); }, ); + + testWidgets( + 'PopScope onPopInvokedWithResult should be called only once', + (WidgetTester tester) async { + int counter = 0; + + final GoRouter goRouter = GoRouter( + initialLocation: '/', + routes: [ + GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), + GoRoute( + path: '/page-1', + builder: (_, __) => PopScope( + onPopInvokedWithResult: (bool didPop, __) { + if (didPop) { + counter++; + return; + } + }, + child: const Text('Page 1'), + ), + ), + ], + ); + + addTearDown(goRouter.dispose); + + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + + goRouter.push('/page-1'); + + await tester.pumpAndSettle(); + + expect(find.text('Page 1'), findsOneWidget); + expect( + goRouter.routerDelegate.currentConfiguration.matches.length, + equals(2), + ); + expect(goRouter.routerDelegate.canPop(), true); + + goRouter.routerDelegate.pop(); + + await tester.pumpAndSettle(); + + expect(counter, equals(1)); + }, + ); + + testWidgets( + r'PopScope onPopInvokedWithResult should be called only once with GoRouteData.$route', + (WidgetTester tester) async { + int counter = 0; + + final GoRouter goRouter = GoRouter( + initialLocation: '/', + routes: [ + GoRoute(path: '/', builder: (_, __) => const DummyStatefulWidget()), + GoRouteData.$route( + path: '/page-1', + factory: (GoRouterState state) => _Page1( + onPop: () { + counter++; + }, + ), + ), + ], + ); + + addTearDown(goRouter.dispose); + + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + + goRouter.push('/page-1'); + + await tester.pumpAndSettle(); + + expect(find.text('Page 1'), findsOneWidget); + expect( + goRouter.routerDelegate.currentConfiguration.matches.length, + equals(2), + ); + expect(goRouter.routerDelegate.canPop(), true); + + goRouter.routerDelegate.pop(); + + await tester.pumpAndSettle(); + + expect(counter, equals(1)); + }, + ); +} + +class _Page1 extends GoRouteData { + const _Page1({ + required this.onPop, + }); + + final VoidCallback onPop; + + @override + Page buildPage(BuildContext context, GoRouterState state) => + MaterialPage( + child: PopScope( + onPopInvokedWithResult: (bool didPop, __) { + if (didPop) { + onPop(); + return; + } + }, + child: const Text('Page 1'), + ), + ); +} + +class DummyStatefulWidget extends StatefulWidget { + const DummyStatefulWidget({super.key}); + + @override + State createState() => _DummyStatefulWidgetState(); +} + +class _DummyStatefulWidgetState extends State { + @override + Widget build(BuildContext context) => Container(); } From a1228df50aa0712cef45b5287ee9ce7a9e19213c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemen=20Tu=C5=A1ar?= Date: Thu, 8 May 2025 21:06:39 +0100 Subject: [PATCH 15/15] test: simplify parameters in onPopInvokedWithResult and buildPage methods --- packages/go_router/test/on_exit_test.dart | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/go_router/test/on_exit_test.dart b/packages/go_router/test/on_exit_test.dart index 5ce16033eb07..997db069ef08 100644 --- a/packages/go_router/test/on_exit_test.dart +++ b/packages/go_router/test/on_exit_test.dart @@ -572,17 +572,14 @@ void main() { } class _Page1 extends GoRouteData { - const _Page1({ - required this.onPop, - }); + const _Page1({required this.onPop}); final VoidCallback onPop; @override - Page buildPage(BuildContext context, GoRouterState state) => - MaterialPage( + Page buildPage(_, __) => MaterialPage( child: PopScope( - onPopInvokedWithResult: (bool didPop, __) { + onPopInvokedWithResult: (bool didPop, _) { if (didPop) { onPop(); return; @@ -602,5 +599,5 @@ class DummyStatefulWidget extends StatefulWidget { class _DummyStatefulWidgetState extends State { @override - Widget build(BuildContext context) => Container(); + Widget build(_) => const SizedBox(); }