Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/go_router/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 15.3.0

- `ShellRoute` will auto merge the `observers` passed into `GoRouter`.
- Adds `mergeObservers` to `ShellRouteBase`, `ShellRoute`, `StatefulShellRoute`, `ShellRouteData.$route`.

## 15.2.0

- `GoRouteData` now defines `.location`, `.go(context)`, `.push(context)`, `.pushReplacement(context)`, and `replace(context)` to be used for [Type-safe routing](https://pub.dev/documentation/go_router/latest/topics/Type-safe%20routes-topic.html). **Requires go_router_builder >= 3.0.0**.
Expand Down
1 change: 1 addition & 0 deletions packages/go_router/lib/go_router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export 'src/match.dart' hide RouteMatchListCodec;
export 'src/misc/errors.dart';
export 'src/misc/extensions.dart';
export 'src/misc/inherited_router.dart';
export 'src/misc/merged_observer.dart';
export 'src/pages/custom_transition_page.dart';
export 'src/parser.dart';
export 'src/route.dart';
Expand Down
61 changes: 61 additions & 0 deletions packages/go_router/lib/src/misc/merged_observer.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import 'package:flutter/widgets.dart';

/// A [NavigatorObserver] that merges the observers of the current
/// route with the observers of the previous route.
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();
}
}
}
51 changes: 45 additions & 6 deletions packages/go_router/lib/src/route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:meta/meta.dart';

import 'configuration.dart';
import 'match.dart';
import 'misc/merged_observer.dart';
import 'path_utils.dart';
import 'router.dart';
import 'state.dart';
Expand Down Expand Up @@ -482,8 +483,20 @@ abstract class ShellRouteBase extends RouteBase {
super.redirect,
required super.routes,
required super.parentNavigatorKey,
this.mergeObservers = true,
}) : super._();

/// Whether to merge the observers of the shell route's parent with the
/// observers of the shell route.
///
/// When `true`, the observers of the shell route's parent will be merged with
/// the observers of the shell route.
///
/// Only effective for `observers` passed into `GoRouter`.
///
/// Defaults to `false`.
final bool mergeObservers;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why make this a per route setting instead of a global setting in goRouter?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also what if it is a shellroute nested inside another shell route, if this is true, should the innershellroute notify the observer of outer shellroute?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should use Navigator.maybeOf(context).widget.observers to merge observers and keep the mergeObservers property for each ShellRoute.

This ensures that the case of ShellRoute nesting ShellRoute is handled correctly, but there is one exception: when ShellRoute is nested under a Navigator that is not built by ShellRoute. Maybe we should ignore this case.

The reason for adding mergeObservers property to each ShellRoute is to allow users to decide that a specific ShellRoute should not merge observers. Defining it as a global setting in GoRouter would make maintaining this case difficult.


static void _debugCheckSubRouteParentNavigatorKeys(
List<RouteBase> subRoutes, GlobalKey<NavigatorState> navigatorKey) {
for (final RouteBase route in subRoutes) {
Expand Down Expand Up @@ -552,9 +565,26 @@ class ShellRouteContext {
final NavigatorBuilder navigatorBuilder;

Widget _buildNavigatorForCurrentRoute(
List<NavigatorObserver>? observers, String? restorationScopeId) {
return navigatorBuilder(
navigatorKey, match, routeMatchList, observers, restorationScopeId);
BuildContext context,
List<NavigatorObserver>? observers,
bool mergeObservers,
String? restorationScopeId) {
final List<NavigatorObserver> effectiveObservers = <NavigatorObserver>[
...?observers
];

if (mergeObservers) {
final List<NavigatorObserver>? rootObservers =
GoRouter.maybeOf(context)?.observers;
if (rootObservers != null) {
effectiveObservers.add(MergedNavigatorObserver(
rootObservers,
));
}
}

return navigatorBuilder(navigatorKey, match, routeMatchList,
effectiveObservers, restorationScopeId);
}
}

Expand Down Expand Up @@ -659,6 +689,7 @@ class ShellRoute extends ShellRouteBase {
super.redirect,
this.builder,
this.pageBuilder,
super.mergeObservers,
this.observers,
required super.routes,
super.parentNavigatorKey,
Expand Down Expand Up @@ -695,7 +726,7 @@ class ShellRoute extends ShellRouteBase {
ShellRouteContext shellRouteContext) {
if (builder != null) {
final Widget navigator = shellRouteContext._buildNavigatorForCurrentRoute(
observers, restorationScopeId);
context, observers, mergeObservers, restorationScopeId);
return builder!(context, state, navigator);
}
return null;
Expand All @@ -706,7 +737,7 @@ class ShellRoute extends ShellRouteBase {
ShellRouteContext shellRouteContext) {
if (pageBuilder != null) {
final Widget navigator = shellRouteContext._buildNavigatorForCurrentRoute(
observers, restorationScopeId);
context, observers, mergeObservers, restorationScopeId);
return pageBuilder!(context, state, navigator);
}
return null;
Expand Down Expand Up @@ -822,6 +853,7 @@ class StatefulShellRoute extends ShellRouteBase {
super.redirect,
this.builder,
this.pageBuilder,
super.mergeObservers,
required this.navigatorContainerBuilder,
super.parentNavigatorKey,
this.restorationScopeId,
Expand All @@ -848,6 +880,7 @@ class StatefulShellRoute extends ShellRouteBase {
/// for a complete runnable example using StatefulShellRoute.indexedStack.
StatefulShellRoute.indexedStack({
required List<StatefulShellBranch> branches,
bool mergeObservers = true,
GoRouterRedirect? redirect,
StatefulShellRouteBuilder? builder,
GlobalKey<NavigatorState>? parentNavigatorKey,
Expand All @@ -859,6 +892,7 @@ class StatefulShellRoute extends ShellRouteBase {
redirect: redirect,
builder: builder,
pageBuilder: pageBuilder,
mergeObservers: mergeObservers,
parentNavigatorKey: parentNavigatorKey,
restorationScopeId: restorationScopeId,
navigatorContainerBuilder: _indexedStackContainerBuilder,
Expand Down Expand Up @@ -955,6 +989,7 @@ class StatefulShellRoute extends ShellRouteBase {
StatefulNavigationShell(
shellRouteContext: shellRouteContext,
router: GoRouter.of(context),
mergeObservers: mergeObservers,
containerBuilder: navigatorContainerBuilder);

static Widget _indexedStackContainerBuilder(BuildContext context,
Expand Down Expand Up @@ -1121,6 +1156,7 @@ class StatefulNavigationShell extends StatefulWidget {
required this.shellRouteContext,
required GoRouter router,
required this.containerBuilder,
this.mergeObservers = true,
}) : assert(shellRouteContext.route is StatefulShellRoute),
_router = router,
currentIndex = _indexOfBranchNavigatorKey(
Expand All @@ -1144,6 +1180,9 @@ class StatefulNavigationShell extends StatefulWidget {
/// Corresponds to the index in the branches field of [StatefulShellRoute].
final int currentIndex;

/// Same to [ShellRoute.mergeObservers].
final bool mergeObservers;

/// The associated [StatefulShellRoute].
StatefulShellRoute get route => shellRouteContext.route as StatefulShellRoute;

Expand Down Expand Up @@ -1318,7 +1357,7 @@ class StatefulNavigationShellState extends State<StatefulNavigationShell>
previousBranchLocation != currentBranchLocation;
if (locationChanged || !hasExistingNavigator) {
branchState.navigator = shellRouteContext._buildNavigatorForCurrentRoute(
branch.observers, branch.restorationScopeId);
context, branch.observers, false, branch.restorationScopeId);
}

_cleanUpObsoleteBranches();
Expand Down
14 changes: 14 additions & 0 deletions packages/go_router/lib/src/route_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ abstract class ShellRouteData extends RouteData {
GlobalKey<NavigatorState>? navigatorKey,
GlobalKey<NavigatorState>? parentNavigatorKey,
List<RouteBase> routes = const <RouteBase>[],
bool mergeObservers = true,
List<NavigatorObserver>? observers,
String? restorationScopeId,
}) {
Expand Down Expand Up @@ -239,6 +240,7 @@ abstract class ShellRouteData extends RouteData {
parentNavigatorKey: parentNavigatorKey,
routes: routes,
navigatorKey: navigatorKey,
mergeObservers: mergeObservers,
observers: observers,
restorationScopeId: restorationScopeId,
redirect: redirect,
Expand Down Expand Up @@ -435,9 +437,15 @@ class TypedGoRoute<T extends GoRouteData> extends TypedRoute<T> {
class TypedShellRoute<T extends ShellRouteData> extends TypedRoute<T> {
/// Default const constructor
const TypedShellRoute({
this.mergeObservers = true,
this.routes = const <TypedRoute<RouteData>>[],
});

/// Determines whether the observers should be merged.
///
/// See [ShellRouteBase.mergeObservers].
final bool mergeObservers;

/// Child route definitions.
///
/// See [RouteBase.routes].
Expand All @@ -450,9 +458,15 @@ class TypedStatefulShellRoute<T extends StatefulShellRouteData>
extends TypedRoute<T> {
/// Default const constructor
const TypedStatefulShellRoute({
this.mergeObservers = true,
this.branches = const <TypedStatefulShellBranch<StatefulShellBranchData>>[],
});

/// Determines whether the observers should be merged.
///
/// See [ShellRouteBase.mergeObservers].
final bool mergeObservers;

/// Child route definitions.
///
/// See [RouteBase.routes].
Expand Down
5 changes: 4 additions & 1 deletion packages/go_router/lib/src/router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,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,
Expand Down Expand Up @@ -302,6 +302,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));
Expand Down
2 changes: 1 addition & 1 deletion packages/go_router/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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.2.0
version: 15.3.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

Expand Down
35 changes: 35 additions & 0 deletions packages/go_router/test/shell_route_observers_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/foundation.dart';
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(
Expand All @@ -25,4 +28,36 @@ void main() {

expect(shell.observers!.length, 1);
});

testWidgets('observers should be merged', (WidgetTester tester) async {
final HeroController observer = HeroController();
final List<NavigatorObserver> observers = <NavigatorObserver>[observer];
addTearDown(observer.dispose);

final GlobalKey<NavigatorState> navKey = GlobalKey<NavigatorState>();
await createRouter(
<RouteBase>[
ShellRoute(
navigatorKey: navKey,
builder: (_, __, Widget child) => child,
routes: <RouteBase>[
GoRoute(
path: '/',
parentNavigatorKey: navKey,
builder: (_, __) => const Text('Home'),
),
],
),
],
tester,
observers: observers,
);
await tester.pumpAndSettle();

final List<NavigatorObserver> shellRouteObservers =
navKey.currentState!.widget.observers;
final MergedNavigatorObserver mergedObservers =
shellRouteObservers.single as MergedNavigatorObserver;
expect(listEquals(observers, mergedObservers.observers), isTrue);
});
}
2 changes: 2 additions & 0 deletions packages/go_router/test/test_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ Future<GoRouter> createRouter(
GoExceptionHandler? onException,
bool requestFocus = true,
bool overridePlatformDefaultLocation = false,
List<NavigatorObserver>? observers,
}) async {
final GoRouter goRouter = GoRouter(
routes: routes,
Expand All @@ -191,6 +192,7 @@ Future<GoRouter> createRouter(
restorationScopeId: restorationScopeId,
requestFocus: requestFocus,
overridePlatformDefaultLocation: overridePlatformDefaultLocation,
observers: observers,
);
addTearDown(goRouter.dispose);
await tester.pumpWidget(
Expand Down