[go_router] Fix: Consistent PopScope Handling on Root Routes issue #140869

packages/go_router/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 14.6.1
+
+- Fixed `PopScope`, and `WillPopScop` was not handled properly in the Root routes.
+
 ## 14.6.0
 
 - Allows going to a path relatively by prefixing `./`
@@ -6,6 +10,7 @@
 
 - Adds preload support to StatefulShellRoute, configurable via `preload` parameter on StatefulShellBranch.
 
+
 ## 14.4.1
 
 - Adds `missing_code_block_language_in_doc_comment` lint.

packages/go_router/lib/src/delegate.dart

@@ -57,9 +57,13 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
   Future<bool> popRoute() async {
     final NavigatorState? state = _findCurrentNavigator();
     if (state != null) {
-      return state.maybePop();
+      final bool didPop = await state.maybePop(); // Call maybePop() directly
+      if (didPop) {
+        return true; // Return true if maybePop handled the pop
+      }
     }
-    // This should be the only place where the last GoRoute exit the screen.
+
+    // 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!(
@@ -68,6 +72,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
             .buildState(_configuration, currentConfiguration),
       ));
     }
+
     return false;
   }
 
@@ -89,26 +94,29 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
   /// Pops the top-most route.
   void pop<T extends Object?>([T? result]) {
     final NavigatorState? state = _findCurrentNavigator();
-    if (state == null) {
+    if (state == null || !state.canPop()) {
       throw GoError('There is nothing to pop');
     }
     state.pop(result);
   }
 
   NavigatorState? _findCurrentNavigator() {
     NavigatorState? state;
-    if (navigatorKey.currentState?.canPop() ?? false) {
-      state = navigatorKey.currentState;
-    }
+    state =
+        navigatorKey.currentState; // Set state directly without canPop check
+
     RouteMatchBase walker = currentConfiguration.matches.last;
     while (walker is ShellRouteMatch) {
       final NavigatorState potentialCandidate =
           walker.navigatorKey.currentState!;
-      if (!ModalRoute.of(potentialCandidate.context)!.isCurrent) {
-        // There is a pageless route on top of the shell route. it needs to be
-        // popped first.
+
+      final ModalRoute<dynamic>? modalRoute =
+          ModalRoute.of(potentialCandidate.context);
+      if (modalRoute == null || !modalRoute.isCurrent) {
+        // Stop if there is a pageless route on top of the shell route.
         break;
       }
+
       if (potentialCandidate.canPop()) {
         state = walker.navigatorKey.currentState;
       }
@@ -117,14 +125,6 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
     return state;
   }
 
-  void _debugAssertMatchListNotEmpty() {
-    assert(
-      currentConfiguration.isNotEmpty,
-      'You have popped the last page off of the stack,'
-      ' there are no pages left to show',
-    );
-  }
-
   bool _handlePopPageWithRouteMatch(
       Route<Object?> route, Object? result, RouteMatchBase match) {
     if (route.willHandlePopInternally) {
@@ -154,6 +154,14 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
     return false;
   }
 
+  void _debugAssertMatchListNotEmpty() {
+    assert(
+      currentConfiguration.isNotEmpty,
+      'You have popped the last page off of the stack,'
+      ' there are no pages left to show',
+    );
+  }
+
   void _completeRouteMatch(Object? result, RouteMatchBase match) {
     RouteMatchBase walker = match;
     while (walker is ShellRouteMatch) {
@@ -162,12 +170,14 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
     if (walker is ImperativeRouteMatch) {
       walker.complete(result);
     }
+
+    // Unconditionally remove the match from the current configuration
     currentConfiguration = currentConfiguration.remove(match);
+
     notifyListeners();
-    assert(() {
-      _debugAssertMatchListNotEmpty();
-      return true;
-    }());
+
+    // Ensure the configuration is not empty
+    _debugAssertMatchListNotEmpty();
   }
 
   /// The top [GoRouterState], the state of the route that was

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.6.0
+version: 14.6.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
 

packages/go_router/test/delegate_test.dart

@@ -92,6 +92,44 @@ void main() {
           false);
     });
 
+    testWidgets('PopScope intercepts back button on root route',
+        (WidgetTester tester) async {
+      bool didPop = false;
+
+      final GoRouter goRouter = GoRouter(
+        initialLocation: '/',
+        routes: <GoRoute>[
+          GoRoute(
+            path: '/',
+            builder: (_, __) => PopScope(
+              onPopInvokedWithResult: (bool result, _) {
+                didPop = true;
+              },
+              canPop: false,
+              child: const Text('Home'),
+            ),
+          ),
+        ],
+      );
+
+      addTearDown(goRouter.dispose);
+
+      await tester.pumpWidget(MaterialApp.router(
+        routerConfig: goRouter,
+      ));
+
+      expect(find.text('Home'), findsOneWidget);
+
+      // Simulate back button press
+      await tester.binding.handlePopRoute();
+
+      await tester.pumpAndSettle();
+
+      // Verify that PopScope intercepted the back button
+      expect(didPop, isTrue);
+      expect(find.text('Home'), findsOneWidget);
+    });
+
     testWidgets('pops more than matches count should return false',
         (WidgetTester tester) async {
       final GoRouter goRouter = await createGoRouter(tester)