[go_router] Allows redirect only GoRoute to be part of RouteMatchList (#4315)
fixes https://github.com/flutter/flutter/issues/114807
diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
index bdde3c2..0d1f319 100644
--- a/packages/go_router/CHANGELOG.md
+++ b/packages/go_router/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 9.0.1
+
+- Allows redirect only GoRoute to be part of RouteMatchList.
+
## 9.0.0
- **BREAKING CHANGE**:
diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart
index 26d70ae..7746165 100644
--- a/packages/go_router/lib/src/builder.dart
+++ b/packages/go_router/lib/src/builder.dart
@@ -88,6 +88,8 @@
// empty box until then.
return const SizedBox.shrink();
}
+ assert(
+ matchList.isError || !(matchList.last.route as GoRoute).redirectOnly);
return builderWithNav(
context,
Builder(
@@ -212,8 +214,12 @@
if (route is GoRoute) {
page =
_buildPageForGoRoute(context, state, match, route, pagePopContext);
-
- keyToPages.putIfAbsent(routeNavKey, () => <Page<Object?>>[]).add(page);
+ assert(page != null || route.redirectOnly);
+ if (page != null) {
+ keyToPages
+ .putIfAbsent(routeNavKey, () => <Page<Object?>>[])
+ .add(page);
+ }
_buildRecursive(context, matchList, startIndex + 1, pagePopContext,
routerNeglect, keyToPages, navigatorKey, registry);
@@ -275,8 +281,6 @@
if (page != null) {
registry[page] = state;
pagePopContext._setRouteMatchForPage(page, match);
- } else {
- throw GoError('Unsupported route type $route');
}
}
@@ -344,36 +348,30 @@
}
/// Builds a [Page] for [GoRoute]
- Page<Object?> _buildPageForGoRoute(BuildContext context, GoRouterState state,
+ Page<Object?>? _buildPageForGoRoute(BuildContext context, GoRouterState state,
RouteMatch match, GoRoute route, _PagePopContext pagePopContext) {
- Page<Object?>? page;
-
// Call the pageBuilder if it's non-null
final GoRouterPageBuilder? pageBuilder = route.pageBuilder;
if (pageBuilder != null) {
- page = pageBuilder(context, state);
- if (page is NoOpPage) {
- page = null;
+ final Page<Object?> page = pageBuilder(context, state);
+ if (page is! NoOpPage) {
+ return page;
}
}
-
- // Return the result of the route's builder() or pageBuilder()
- return page ??
- buildPage(context, state, Builder(builder: (BuildContext context) {
- return _callGoRouteBuilder(context, state, route);
- }));
+ return _callGoRouteBuilder(context, state, route);
}
/// Calls the user-provided route builder from the [GoRoute].
- Widget _callGoRouteBuilder(
+ Page<Object?>? _callGoRouteBuilder(
BuildContext context, GoRouterState state, GoRoute route) {
final GoRouterWidgetBuilder? builder = route.builder;
if (builder == null) {
- throw GoError('No routeBuilder provided to GoRoute: $route');
+ return null;
}
-
- return builder(context, state);
+ return buildPage(context, state, Builder(builder: (BuildContext context) {
+ return builder(context, state);
+ }));
}
/// Builds a [Page] for [ShellRouteBase]
diff --git a/packages/go_router/lib/src/match.dart b/packages/go_router/lib/src/match.dart
index 865ab0b..9106b1c 100644
--- a/packages/go_router/lib/src/match.dart
+++ b/packages/go_router/lib/src/match.dart
@@ -263,8 +263,11 @@
assert(index != -1);
newMatches.removeRange(index, newMatches.length);
- // Also pop ShellRoutes when there are no subsequent route matches
- while (newMatches.isNotEmpty && newMatches.last.route is ShellRouteBase) {
+ // Also pop ShellRoutes that have no subsequent route matches and GoRoutes
+ // that only have redirect.
+ while (newMatches.isNotEmpty &&
+ (newMatches.last.route is ShellRouteBase ||
+ (newMatches.last.route as GoRoute).redirectOnly)) {
newMatches.removeLast();
}
// Removing ImperativeRouteMatch should not change uri and pathParameters.
diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart
index 1a9dcca..778d099 100644
--- a/packages/go_router/lib/src/parser.dart
+++ b/packages/go_router/lib/src/parser.dart
@@ -98,6 +98,14 @@
if (matchList.isError && onParserException != null) {
return onParserException!(context, matchList);
}
+
+ assert(() {
+ if (matchList.isNotEmpty) {
+ assert(!(matchList.last.route as GoRoute).redirectOnly,
+ 'A redirect-only route must redirect to location different from itself.\n The offending route: ${matchList.last.route}');
+ }
+ return true;
+ }());
return _updateRouteMatchList(
matchList,
baseRouteMatchList: state.baseRouteMatchList,
diff --git a/packages/go_router/lib/src/route.dart b/packages/go_router/lib/src/route.dart
index 9d33e88..b7ba3af 100644
--- a/packages/go_router/lib/src/route.dart
+++ b/packages/go_router/lib/src/route.dart
@@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'package:collection/collection.dart';
+import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart';
import 'package:meta/meta.dart';
@@ -96,7 +97,7 @@
/// ///
/// See [main.dart](https://github.com/flutter/packages/blob/main/packages/go_router/example/lib/main.dart)
@immutable
-abstract class RouteBase {
+abstract class RouteBase with Diagnosticable {
const RouteBase._({
required this.routes,
required this.parentNavigatorKey,
@@ -118,6 +119,15 @@
return routes.expand(
(RouteBase e) => <RouteBase>[e, ...routesRecursively(e.routes)]);
}
+
+ @override
+ void debugFillProperties(DiagnosticPropertiesBuilder properties) {
+ super.debugFillProperties(properties);
+ if (parentNavigatorKey != null) {
+ properties.add(DiagnosticsProperty<GlobalKey<NavigatorState>>(
+ 'parentNavKey', parentNavigatorKey));
+ }
+ }
}
/// A route that is displayed visually above the matching parent route using the
@@ -157,6 +167,11 @@
_pathRE = patternToRegExp(path, pathParameters);
}
+ /// Whether this [GoRoute] only redirects to another route.
+ ///
+ /// If this is true, this route must redirect location other than itself.
+ bool get redirectOnly => pageBuilder == null && builder == null;
+
/// Optional name of the route.
///
/// If used, a unique string name must be provided and it can not be empty.
@@ -323,8 +338,12 @@
final List<String> pathParameters = <String>[];
@override
- String toString() {
- return 'GoRoute(name: $name, path: $path)';
+ void debugFillProperties(DiagnosticPropertiesBuilder properties) {
+ super.debugFillProperties(properties);
+ properties.add(StringProperty('name', name));
+ properties.add(StringProperty('path', path));
+ properties.add(
+ FlagProperty('redirect', value: redirectOnly, ifTrue: 'Redirect Only'));
}
late final RegExp _pathRE;
@@ -338,6 +357,21 @@
{required super.routes, required super.parentNavigatorKey})
: super._();
+ static void _debugCheckSubRouteParentNavigatorKeys(
+ List<RouteBase> subRoutes, GlobalKey<NavigatorState> navigatorKey) {
+ for (final RouteBase route in subRoutes) {
+ assert(
+ route.parentNavigatorKey == null ||
+ route.parentNavigatorKey == navigatorKey,
+ "sub-route's parent navigator key must either be null or has the same navigator key as parent's key");
+ if (route is GoRoute && route.redirectOnly) {
+ // This route does not produce a page, need to check its sub-routes
+ // instead.
+ _debugCheckSubRouteParentNavigatorKeys(route.routes, navigatorKey);
+ }
+ }
+ }
+
/// Attempts to build the Widget representing this shell route.
///
/// Returns null if this shell route does not build a Widget, but instead uses
@@ -506,12 +540,11 @@
}) : assert(routes.isNotEmpty),
navigatorKey = navigatorKey ?? GlobalKey<NavigatorState>(),
super._() {
- for (final RouteBase route in routes) {
- if (route is GoRoute) {
- assert(route.parentNavigatorKey == null ||
- route.parentNavigatorKey == navigatorKey);
- }
- }
+ assert(() {
+ ShellRouteBase._debugCheckSubRouteParentNavigatorKeys(
+ routes, this.navigatorKey);
+ return true;
+ }());
}
/// The widget builder for a shell route.
@@ -576,6 +609,13 @@
@override
Iterable<GlobalKey<NavigatorState>> get _navigatorKeys =>
<GlobalKey<NavigatorState>>[navigatorKey];
+
+ @override
+ void debugFillProperties(DiagnosticPropertiesBuilder properties) {
+ super.debugFillProperties(properties);
+ properties.add(DiagnosticsProperty<GlobalKey<NavigatorState>>(
+ 'navigatorKey', navigatorKey));
+ }
}
/// A route that displays a UI shell with separate [Navigator]s for its
@@ -828,6 +868,13 @@
}
return true;
}
+
+ @override
+ void debugFillProperties(DiagnosticPropertiesBuilder properties) {
+ super.debugFillProperties(properties);
+ properties.add(DiagnosticsProperty<Iterable<GlobalKey<NavigatorState>>>(
+ 'navigatorKeys', _navigatorKeys));
+ }
}
/// Representation of a separate branch in a stateful navigation tree, used to
@@ -854,7 +901,13 @@
this.initialLocation,
this.restorationScopeId,
this.observers,
- }) : navigatorKey = navigatorKey ?? GlobalKey<NavigatorState>();
+ }) : navigatorKey = navigatorKey ?? GlobalKey<NavigatorState>() {
+ assert(() {
+ ShellRouteBase._debugCheckSubRouteParentNavigatorKeys(
+ routes, this.navigatorKey);
+ return true;
+ }());
+ }
/// The [GlobalKey] to be used by the [Navigator] built for this branch.
///
diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml
index f977eb9..83bebb1 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: 9.0.0
+version: 9.0.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
diff --git a/packages/go_router/test/go_route_test.dart b/packages/go_router/test/go_route_test.dart
index f7ffe70..0ff437c 100644
--- a/packages/go_router/test/go_route_test.dart
+++ b/packages/go_router/test/go_route_test.dart
@@ -155,4 +155,127 @@
expect(find.text('Screen D'), findsOneWidget);
expect(find.text('Screen C'), findsOneWidget);
});
+
+ test('ShellRoute parent navigator key throw if not match', () async {
+ final GlobalKey<NavigatorState> key1 = GlobalKey<NavigatorState>();
+ final GlobalKey<NavigatorState> key2 = GlobalKey<NavigatorState>();
+ bool hasError = false;
+ try {
+ ShellRoute(
+ navigatorKey: key1,
+ builder: (_, __, Widget child) => child,
+ routes: <RouteBase>[
+ ShellRoute(
+ parentNavigatorKey: key2,
+ builder: (_, __, Widget child) => child,
+ routes: <RouteBase>[
+ GoRoute(
+ path: '1',
+ builder: (_, __) => const Text('/route/1'),
+ ),
+ ],
+ ),
+ ],
+ );
+ } on AssertionError catch (_) {
+ hasError = true;
+ }
+ expect(hasError, isTrue);
+ });
+
+ group('Redirect only GoRoute', () {
+ testWidgets('can redirect to subroute', (WidgetTester tester) async {
+ final GoRouter router = await createRouter(
+ <RouteBase>[
+ GoRoute(
+ path: '/',
+ builder: (_, __) => const Text('home'),
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'route',
+ redirect: (_, __) => '/route/1',
+ routes: <RouteBase>[
+ GoRoute(
+ path: '1',
+ builder: (_, __) => const Text('/route/1'),
+ ),
+ ],
+ ),
+ ],
+ ),
+ ],
+ tester,
+ );
+ expect(find.text('home'), findsOneWidget);
+
+ router.go('/route');
+ await tester.pumpAndSettle();
+ // Should redirect to /route/1 without error.
+ expect(find.text('/route/1'), findsOneWidget);
+
+ router.pop();
+ await tester.pumpAndSettle();
+ // Should go back directly to home page.
+ expect(find.text('home'), findsOneWidget);
+ });
+
+ testWidgets('throw if redirect to itself.', (WidgetTester tester) async {
+ final GoRouter router = await createRouter(
+ <RouteBase>[
+ GoRoute(
+ path: '/',
+ builder: (_, __) => const Text('home'),
+ routes: <RouteBase>[
+ GoRoute(
+ path: 'route',
+ redirect: (_, __) => '/route',
+ routes: <RouteBase>[
+ GoRoute(
+ path: '1',
+ builder: (_, __) => const Text('/route/1'),
+ ),
+ ],
+ ),
+ ],
+ ),
+ ],
+ tester,
+ );
+ expect(find.text('home'), findsOneWidget);
+
+ router.go('/route');
+ await tester.pumpAndSettle();
+ // Should redirect to /route/1 without error.
+ expect(tester.takeException(), isAssertionError);
+ });
+
+ testWidgets('throw if sub route does not conform with parent navigator key',
+ (WidgetTester tester) async {
+ final GlobalKey<NavigatorState> key1 = GlobalKey<NavigatorState>();
+ final GlobalKey<NavigatorState> key2 = GlobalKey<NavigatorState>();
+ bool hasError = false;
+ try {
+ ShellRoute(
+ navigatorKey: key1,
+ builder: (_, __, Widget child) => child,
+ routes: <RouteBase>[
+ GoRoute(
+ path: '/',
+ redirect: (_, __) => '/route',
+ routes: <RouteBase>[
+ GoRoute(
+ parentNavigatorKey: key2,
+ path: 'route',
+ builder: (_, __) => const Text('/route/1'),
+ ),
+ ],
+ ),
+ ],
+ );
+ } on AssertionError catch (_) {
+ hasError = true;
+ }
+ expect(hasError, isTrue);
+ });
+ });
}
diff --git a/packages/go_router/test/matching_test.dart b/packages/go_router/test/matching_test.dart
index 7784b85..640a467 100644
--- a/packages/go_router/test/matching_test.dart
+++ b/packages/go_router/test/matching_test.dart
@@ -22,9 +22,8 @@
const Placeholder()),
];
- final GoRouter router = await createRouter(routes, tester);
- router.go('/page-0');
- await tester.pumpAndSettle();
+ final GoRouter router =
+ await createRouter(routes, tester, initialLocation: '/page-0');
final RouteMatchList matches = router.routerDelegate.currentConfiguration;
expect(matches.toString(), contains('/page-0'));
diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart
index 1541940..43299ec 100644
--- a/packages/go_router/test/route_data_test.dart
+++ b/packages/go_router/test/route_data_test.dart
@@ -189,9 +189,7 @@
routes: _routes,
);
await tester.pumpWidget(MaterialApp.router(
- routeInformationProvider: goRouter.routeInformationProvider,
- routeInformationParser: goRouter.routeInformationParser,
- routerDelegate: goRouter.routerDelegate,
+ routerConfig: goRouter,
));
expect(find.byKey(const Key('build')), findsNothing);
expect(find.byKey(const Key('buildPage')), findsOneWidget);
@@ -206,9 +204,7 @@
routes: _routes,
);
await tester.pumpWidget(MaterialApp.router(
- routeInformationProvider: goRouter.routeInformationProvider,
- routeInformationParser: goRouter.routeInformationParser,
- routerDelegate: goRouter.routerDelegate,
+ routerConfig: goRouter,
));
expect(find.byKey(const Key('build')), findsNothing);
expect(find.byKey(const Key('buildPage')), findsNothing);
