Skip to content

Commit

Permalink
[go_router] If there is more than one route to match, use the first m…
Browse files Browse the repository at this point in the history
…atch. (flutter#1995)
  • Loading branch information
GauravCalidig authored May 27, 2022
1 parent 966d0b9 commit 197bb11
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 38 deletions.
4 changes: 4 additions & 0 deletions packages/go_router/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 3.1.1

- Uses first match if there are more than one route to match. [ [#99833](https://github.com/flutter/flutter/issues/99833)

## 3.1.0

- Added `GoRouteData` and `TypedGoRoute` to support `package:go_router_builder`.
Expand Down
24 changes: 24 additions & 0 deletions packages/go_router/lib/src/go_route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,30 @@ class GoRoute {
/// ],
/// );
///
/// If there are multiple routes that match the location, the first match is used.
/// To make predefined routes to take precedence over dynamic routes eg. '/:id'
/// consider adding the dynamic route at the end of the routes
/// For example:
/// ```
/// final GoRouter _router = GoRouter(
/// routes: <GoRoute>[
/// GoRoute(
/// path: '/',
/// redirect: (_) => '/family/${Families.data[0].id}',
/// ),
/// GoRoute(
/// path: '/family',
/// pageBuilder: (BuildContext context, GoRouterState state) => ...,
/// ),
/// GoRoute(
/// path: '/:username',
/// pageBuilder: (BuildContext context, GoRouterState state) => ...,
/// ),
/// ],
/// );
/// ```
/// In the above example, if /family route is matched, it will be used.
/// else /:username route will be used.
final List<GoRoute> routes;

/// An optional redirect function for this route.
Expand Down
26 changes: 3 additions & 23 deletions packages/go_router/lib/src/go_router_delegate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -441,30 +441,10 @@ class GoRouterDelegate extends RouterDelegate<Uri>
).toList();

assert(matchStacks.isNotEmpty, 'no routes for location: $location');
assert(() {
if (matchStacks.length > 1) {
final StringBuffer sb = StringBuffer()
..writeln('too many routes for location: $location');

for (final List<GoRouteMatch> stack in matchStacks) {
sb.writeln(
'\t${stack.map((GoRouteMatch m) => m.route.path).join(' => ')}');
}

assert(false, sb.toString());
}

assert(matchStacks.length == 1);
final GoRouteMatch match = matchStacks.first.last;
final String loc1 = _addQueryParams(match.subloc, match.queryParams);
final Uri uri2 = Uri.parse(location);
final String loc2 = _addQueryParams(uri2.path, uri2.queryParameters);

// NOTE: match the lower case, since subloc is canonicalized to match the
// path case whereas the location can be any case
assert(loc1.toLowerCase() == loc2.toLowerCase(), '$loc1 != $loc2');
return true;
}());
// If there are multiple routes that match the location, returning the first one.
// To make predefined routes to take precedence over dynamic routes eg. '/:id'
// consider adding the dynamic route at the end of the routes

return matchStacks.first;
}
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: 3.1.0
version: 3.1.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

Expand Down
49 changes: 35 additions & 14 deletions packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void main() {
expect(router.screenFor(matches.first).runtimeType, HomeScreen);
});

test('match too many routes', () {
test('If there is more than one route to match, use the first match', () {
final List<GoRoute> routes = <GoRoute>[
GoRoute(path: '/', builder: _dummy),
GoRoute(path: '/', builder: _dummy),
Expand All @@ -50,7 +50,7 @@ void main() {
final List<GoRouteMatch> matches = router.routerDelegate.matches;
expect(matches, hasLength(1));
expect(matches.first.fullpath, '/');
expect(router.screenFor(matches.first).runtimeType, ErrorScreen);
expect(router.screenFor(matches.first).runtimeType, DummyScreen);
});

test('empty path', () {
Expand Down Expand Up @@ -275,23 +275,32 @@ void main() {
}
});

test('match too many sub-routes', () {
test('return first matching route if too many subroutes', () {
final List<GoRoute> routes = <GoRoute>[
GoRoute(
path: '/',
builder: _dummy,
builder: (BuildContext context, GoRouterState state) =>
const HomeScreen(),
routes: <GoRoute>[
GoRoute(
path: 'foo/bar',
builder: _dummy,
builder: (BuildContext context, GoRouterState state) =>
const FamilyScreen(''),
),
GoRoute(
path: 'bar',
builder: (BuildContext context, GoRouterState state) =>
const Page1Screen(),
),
GoRoute(
path: 'foo',
builder: _dummy,
builder: (BuildContext context, GoRouterState state) =>
const Page2Screen(),
routes: <GoRoute>[
GoRoute(
path: 'bar',
builder: _dummy,
builder: (BuildContext context, GoRouterState state) =>
const LoginScreen(),
),
],
),
Expand All @@ -300,10 +309,20 @@ void main() {
];

final GoRouter router = _router(routes);
router.go('/bar');
List<GoRouteMatch> matches = router.routerDelegate.matches;
expect(matches, hasLength(2));
expect(router.screenFor(matches[1]).runtimeType, Page1Screen);

router.go('/foo/bar');
final List<GoRouteMatch> matches = router.routerDelegate.matches;
expect(matches, hasLength(1));
expect(router.screenFor(matches.first).runtimeType, ErrorScreen);
matches = router.routerDelegate.matches;
expect(matches, hasLength(2));
expect(router.screenFor(matches[1]).runtimeType, FamilyScreen);

router.go('/foo');
matches = router.routerDelegate.matches;
expect(matches, hasLength(2));
expect(router.screenFor(matches[1]).runtimeType, Page2Screen);
});

test('router state', () {
Expand Down Expand Up @@ -424,17 +443,19 @@ void main() {
expect(router.screenFor(matches.first).runtimeType, FamilyScreen);
});

test('match too many routes, ignoring case', () {
test('If there is more than one route to match, use the first match.', () {
final List<GoRoute> routes = <GoRoute>[
GoRoute(path: '/', builder: _dummy),
GoRoute(path: '/page1', builder: _dummy),
GoRoute(path: '/PaGe1', builder: _dummy),
GoRoute(path: '/page1', builder: _dummy),
GoRoute(path: '/:ok', builder: _dummy),
];

final GoRouter router = _router(routes);
router.go('/PAGE1');
router.go('/user');
final List<GoRouteMatch> matches = router.routerDelegate.matches;
expect(matches, hasLength(1));
expect(router.screenFor(matches.first).runtimeType, ErrorScreen);
expect(router.screenFor(matches.first).runtimeType, DummyScreen);
});
});

Expand Down

0 comments on commit 197bb11

Please sign in to comment.