[go_router] ShellRoute will merge GoRouter's observers (#9436)

`ShellRoute` currently does not trigger `observers` defined in `GoRouter`, which is unexpected behavior.

Fixes flutter/flutter#112196

## Pre-Review Checklist

[^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.

Author: huanghui1998hhh

packages/go_router/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 17.0.0
+
+- **BREAKING CHANGE**
+  - `ShellRoute`'s navigating changes notify `GoRouter`'s observers by default.
+  - Adds `notifyRootObserver` to `ShellRouteBase`, `ShellRoute`, `StatefulShellRoute`, `ShellRouteData.$route`, `TypedShellRoute`, `TypedStatefulShellRoute`.
+
 ## 16.3.0
 
 - Adds a top-level `onEnter` callback with access to current and next route states.

packages/go_router/README.md

@@ -38,6 +38,7 @@ See the API documentation for details on the following topics:
 - [State restoration](https://pub.dev/documentation/go_router/latest/topics/State%20restoration-topic.html)
 
 ## Migration Guides
+- [Migrating to 17.0.0](https://flutter.dev/go/go-router-v17-breaking-changes).
 - [Migrating to 16.0.0](https://flutter.dev/go/go-router-v16-breaking-changes).
 - [Migrating to 15.0.0](https://flutter.dev/go/go-router-v15-breaking-changes).
 - [Migrating to 14.0.0](https://flutter.dev/go/go-router-v14-breaking-changes).

packages/go_router/lib/src/route.dart

@@ -498,8 +498,17 @@ abstract class ShellRouteBase extends RouteBase {
     super.redirect,
     required super.routes,
     required super.parentNavigatorKey,
+    this.notifyRootObserver = true,
   }) : super._();
 
+  /// Whether navigation changes will notify the GoRouter's observers.
+  ///
+  /// When `true`, navigation changes within this shell route will notify
+  /// the GoRouter's observers.
+  ///
+  /// Defaults to `true`.
+  final bool notifyRootObserver;
+
   static void _debugCheckSubRouteParentNavigatorKeys(
     List<RouteBase> subRoutes,
     GlobalKey<NavigatorState> navigatorKey,
@@ -577,14 +586,28 @@ class ShellRouteContext {
   final NavigatorBuilder navigatorBuilder;
 
   Widget _buildNavigatorForCurrentRoute(
+    BuildContext context,
     List<NavigatorObserver>? observers,
+    bool notifyRootObserver,
     String? restorationScopeId,
   ) {
+    final List<NavigatorObserver> effectiveObservers = <NavigatorObserver>[
+      ...?observers,
+    ];
+
+    if (notifyRootObserver) {
+      final List<NavigatorObserver>? rootObservers =
+          GoRouter.maybeOf(context)?.observers;
+      if (rootObservers != null) {
+        effectiveObservers.add(_MergedNavigatorObserver(rootObservers));
+      }
+    }
+
     return navigatorBuilder(
       navigatorKey,
       match,
       routeMatchList,
-      observers,
+      effectiveObservers,
       restorationScopeId,
     );
   }
@@ -691,6 +714,7 @@ class ShellRoute extends ShellRouteBase {
     super.redirect,
     this.builder,
     this.pageBuilder,
+    super.notifyRootObserver,
     this.observers,
     required super.routes,
     super.parentNavigatorKey,
@@ -732,7 +756,9 @@ class ShellRoute extends ShellRouteBase {
   ) {
     if (builder != null) {
       final Widget navigator = shellRouteContext._buildNavigatorForCurrentRoute(
+        context,
         observers,
+        notifyRootObserver,
         restorationScopeId,
       );
       return builder!(context, state, navigator);
@@ -748,7 +774,9 @@ class ShellRoute extends ShellRouteBase {
   ) {
     if (pageBuilder != null) {
       final Widget navigator = shellRouteContext._buildNavigatorForCurrentRoute(
+        context,
         observers,
+        notifyRootObserver,
         restorationScopeId,
       );
       return pageBuilder!(context, state, navigator);
@@ -870,6 +898,7 @@ class StatefulShellRoute extends ShellRouteBase {
     super.redirect,
     this.builder,
     this.pageBuilder,
+    super.notifyRootObserver,
     required this.navigatorContainerBuilder,
     super.parentNavigatorKey,
     this.restorationScopeId,
@@ -900,6 +929,7 @@ class StatefulShellRoute extends ShellRouteBase {
   /// for a complete runnable example using StatefulShellRoute.indexedStack.
   StatefulShellRoute.indexedStack({
     required List<StatefulShellBranch> branches,
+    bool notifyRootObserver = true,
     GoRouterRedirect? redirect,
     StatefulShellRouteBuilder? builder,
     GlobalKey<NavigatorState>? parentNavigatorKey,
@@ -911,6 +941,7 @@ class StatefulShellRoute extends ShellRouteBase {
          redirect: redirect,
          builder: builder,
          pageBuilder: pageBuilder,
+         notifyRootObserver: notifyRootObserver,
          parentNavigatorKey: parentNavigatorKey,
          restorationScopeId: restorationScopeId,
          navigatorContainerBuilder: _indexedStackContainerBuilder,
@@ -1422,7 +1453,9 @@ class StatefulNavigationShellState extends State<StatefulNavigationShell>
         previousBranchLocation != currentBranchLocation;
     if (locationChanged || !hasExistingNavigator) {
       branchState.navigator = shellRouteContext._buildNavigatorForCurrentRoute(
+        context,
         branch.observers,
+        route.notifyRootObserver,
         branch.restorationScopeId,
       );
     }
@@ -1674,3 +1707,67 @@ class _IndexedStackedRouteBranchContainer extends StatelessWidget {
     );
   }
 }
+
+/// A wrapper that merges multiple [NavigatorObserver]s into a single observer.
+///
+/// This is necessary because a [NavigatorObserver] can only be attached to one
+/// [NavigatorState] at a time.
+class _MergedNavigatorObserver extends NavigatorObserver {
+  /// Default constructor for the merged navigator observer.
+  _MergedNavigatorObserver(this.observers);
+
+  /// The observers to be merged.
+  final List<NavigatorObserver> observers;
+
+  @override
+  void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    for (final NavigatorObserver observer in observers) {
+      observer.didPush(route, previousRoute);
+    }
+  }
+
+  @override
+  void didPop(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    for (final NavigatorObserver observer in observers) {
+      observer.didPop(route, previousRoute);
+    }
+  }
+
+  @override
+  void didRemove(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    for (final NavigatorObserver observer in observers) {
+      observer.didRemove(route, previousRoute);
+    }
+  }
+
+  @override
+  void didReplace({Route<dynamic>? newRoute, Route<dynamic>? oldRoute}) {
+    for (final NavigatorObserver observer in observers) {
+      observer.didReplace(newRoute: newRoute, oldRoute: oldRoute);
+    }
+  }
+
+  @override
+  void didChangeTop(Route<dynamic> topRoute, Route<dynamic>? previousTopRoute) {
+    for (final NavigatorObserver observer in observers) {
+      observer.didChangeTop(topRoute, previousTopRoute);
+    }
+  }
+
+  @override
+  void didStartUserGesture(
+    Route<dynamic> route,
+    Route<dynamic>? previousRoute,
+  ) {
+    for (final NavigatorObserver observer in observers) {
+      observer.didStartUserGesture(route, previousRoute);
+    }
+  }
+
+  @override
+  void didStopUserGesture() {
+    for (final NavigatorObserver observer in observers) {
+      observer.didStopUserGesture();
+    }
+  }
+}

packages/go_router/lib/src/route_data.dart

@@ -314,6 +314,7 @@ abstract class ShellRouteData extends RouteData {
     GlobalKey<NavigatorState>? navigatorKey,
     GlobalKey<NavigatorState>? parentNavigatorKey,
     List<RouteBase> routes = const <RouteBase>[],
+    bool notifyRootObserver = true,
     List<NavigatorObserver>? observers,
     String? restorationScopeId,
   }) {
@@ -342,6 +343,7 @@ abstract class ShellRouteData extends RouteData {
       parentNavigatorKey: parentNavigatorKey,
       routes: routes,
       navigatorKey: navigatorKey,
+      notifyRootObserver: notifyRootObserver,
       observers: observers,
       restorationScopeId: restorationScopeId,
       redirect: redirect,
@@ -559,7 +561,16 @@ class TypedRelativeGoRoute<T extends RelativeGoRouteData>
 @Target(<TargetKind>{TargetKind.library, TargetKind.classType})
 class TypedShellRoute<T extends ShellRouteData> extends TypedRoute<T> {
   /// Default const constructor
-  const TypedShellRoute({this.routes = const <TypedRoute<RouteData>>[]});
+  const TypedShellRoute({
+    this.notifyRootObserver = true,
+    this.routes = const <TypedRoute<RouteData>>[],
+  });
+
+  /// Whether navigation changes within this shell route will notify the
+  /// GoRouter's observers.
+  ///
+  /// See [ShellRouteBase.notifyRootObserver].
+  final bool notifyRootObserver;
 
   /// Child route definitions.
   ///
@@ -573,9 +584,16 @@ class TypedStatefulShellRoute<T extends StatefulShellRouteData>
     extends TypedRoute<T> {
   /// Default const constructor
   const TypedStatefulShellRoute({
+    this.notifyRootObserver = true,
     this.branches = const <TypedStatefulShellBranch<StatefulShellBranchData>>[],
   });
 
+  /// Whether navigation changes within this shell route will notify the
+  /// GoRouter's observers.
+  ///
+  /// See [ShellRouteBase.notifyRootObserver].
+  final bool notifyRootObserver;
+
   /// Child route definitions.
   ///
   /// See [RouteBase.routes].

packages/go_router/lib/src/router.dart

@@ -235,7 +235,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
     String? initialLocation,
     this.overridePlatformDefaultLocation = false,
     Object? initialExtra,
-    List<NavigatorObserver>? observers,
+    this.observers,
     bool debugLogDiagnostics = false,
     GlobalKey<NavigatorState>? navigatorKey,
     String? restorationScopeId,
@@ -369,6 +369,9 @@ class GoRouter implements RouterConfig<RouteMatchList> {
   @override
   late final GoRouteInformationParser routeInformationParser;
 
+  /// The navigator observers used by [GoRouter].
+  final List<NavigatorObserver>? observers;
+
   void _handleRoutingConfigChanged() {
     // Reparse is needed to update its builder
     restore(configuration.reparse(routerDelegate.currentConfiguration));

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: 16.3.0
+version: 17.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
 

packages/go_router/test/shell_route_observers_test.dart

@@ -6,6 +6,8 @@ import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:go_router/go_router.dart';
 
+import 'test_helpers.dart';
+
 void main() {
   test('ShellRoute observers test', () {
     final ShellRoute shell = ShellRoute(
@@ -25,4 +27,71 @@ void main() {
 
     expect(shell.observers!.length, 1);
   });
+
+  testWidgets(
+    'GoRouter observers should be notified when navigating within ShellRoute',
+    (WidgetTester tester) async {
+      final MockObserver observer = MockObserver();
+
+      final GlobalKey<NavigatorState> root = GlobalKey<NavigatorState>(
+        debugLabel: 'root',
+      );
+      await createRouter(
+        <RouteBase>[
+          GoRoute(path: '/', builder: (_, __) => const Text('Home')),
+          ShellRoute(
+            builder: (_, __, Widget child) => child,
+            routes: <RouteBase>[
+              GoRoute(path: '/test1', builder: (_, __) => const Text('Test1')),
+            ],
+          ),
+          StatefulShellRoute.indexedStack(
+            builder: (_, __, Widget child) => child,
+            branches: <StatefulShellBranch>[
+              StatefulShellBranch(
+                routes: <RouteBase>[
+                  GoRoute(
+                    path: '/test2',
+                    builder: (_, __) => const Text('Test2'),
+                  ),
+                ],
+              ),
+            ],
+          ),
+        ],
+        tester,
+        navigatorKey: root,
+        observers: <NavigatorObserver>[observer],
+      );
+      await tester.pumpAndSettle();
+
+      root.currentContext!.push('/test1');
+      await tester.pumpAndSettle();
+      expect(observer.getCallCount('/test1'), 1);
+
+      root.currentContext!.push('/test2');
+      await tester.pumpAndSettle();
+      expect(observer.getCallCount('/test2'), 1);
+    },
+  );
+}
+
+class MockObserver extends NavigatorObserver {
+  final Map<String, int> _callCounts = <String, int>{};
+
+  @override
+  void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    final String? routeName = route.settings.name;
+    if (routeName != null) {
+      test(routeName);
+    }
+  }
+
+  void test(String name) {
+    _callCounts[name] = (_callCounts[name] ?? 0) + 1;
+  }
+
+  int getCallCount(String name) {
+    return _callCounts[name] ?? 0;
+  }
 }

packages/go_router/test/test_helpers.dart

@@ -176,6 +176,7 @@ Future<GoRouter> createRouter(
   GoExceptionHandler? onException,
   bool requestFocus = true,
   bool overridePlatformDefaultLocation = false,
+  List<NavigatorObserver>? observers,
 }) async {
   final GoRouter goRouter = GoRouter(
     routes: routes,
@@ -190,6 +191,7 @@ Future<GoRouter> createRouter(
     restorationScopeId: restorationScopeId,
     requestFocus: requestFocus,
     overridePlatformDefaultLocation: overridePlatformDefaultLocation,
+    observers: observers,
   );
   addTearDown(goRouter.dispose);
   await tester.pumpWidget(