[go_router] Allows redirect only GoRoute to be part of RouteMatchList (#4315)

fixes https://github.com/flutter/flutter/issues/114807
This commit is contained in:
chunhtai
2023-07-05 23:08:16 -07:00
committed by GitHub
parent 8f6e326152
commit 38f17151fa
9 changed files with 226 additions and 42 deletions

View File

@ -1,3 +1,7 @@
## 9.0.1
- Allows redirect only GoRoute to be part of RouteMatchList.
## 9.0.0
- **BREAKING CHANGE**:

View File

@ -88,6 +88,8 @@ class RouteBuilder {
// 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 @@ class RouteBuilder {
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 @@ class RouteBuilder {
if (page != null) {
registry[page] = state;
pagePopContext._setRouteMatchForPage(page, match);
} else {
throw GoError('Unsupported route type $route');
}
}
@ -344,36 +348,30 @@ class RouteBuilder {
}
/// 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]

View File

@ -263,8 +263,11 @@ class RouteMatchList {
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.

View File

@ -98,6 +98,14 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
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,

View File

@ -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 @@ import 'typedefs.dart';
/// ///
/// 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 @@ abstract class RouteBase {
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 @@ class GoRoute extends RouteBase {
_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 @@ class GoRoute extends RouteBase {
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 @@ abstract class ShellRouteBase extends RouteBase {
{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 @@ class ShellRoute extends ShellRouteBase {
}) : 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 @@ class ShellRoute extends ShellRouteBase {
@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 @@ class StatefulShellRoute extends ShellRouteBase {
}
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 @@ class StatefulShellBranch {
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.
///

View File

@ -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

View File

@ -155,4 +155,127 @@ void main() {
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);
});
});
}

View File

@ -22,9 +22,8 @@ void main() {
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'));

View File

@ -189,9 +189,7 @@ void main() {
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 @@ void main() {
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);