Skip to content

Commit

Permalink
[go_router]refactor runtime check to assert (#1362)
Browse files Browse the repository at this point in the history
  • Loading branch information
chunhtai authored Apr 5, 2022
1 parent ee4f492 commit dff39d7
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 128 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.0.7

- Refactors runtime checks to assertions.

## 3.0.6

- Exports inherited_go_router.dart file.
Expand Down
67 changes: 27 additions & 40 deletions packages/go_router/lib/src/go_route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,51 +21,38 @@ class GoRoute {
this.builder = _invalidBuilder,
this.routes = const <GoRoute>[],
this.redirect = _noRedirection,
}) {
if (path.isEmpty) {
throw Exception('GoRoute path cannot be empty');
}

if (name != null && name!.isEmpty) {
throw Exception('GoRoute name cannot be empty');
}

if (pageBuilder == null &&
builder == _invalidBuilder &&
redirect == _noRedirection) {
throw Exception(
'GoRoute builder parameter not set\n'
'See gorouter.dev/redirection#considerations for details',
);
}

}) : assert(path.isNotEmpty, 'GoRoute path cannot be empty'),
assert(name == null || name.isNotEmpty, 'GoRoute name cannot be empty'),
assert(
pageBuilder != null ||
builder != _invalidBuilder ||
redirect != _noRedirection,
'GoRoute builder parameter not set\nSee gorouter.dev/redirection#considerations for details') {
// cache the path regexp and parameters
_pathRE = patternToRegExp(path, _pathParams);

// check path params
final Map<String, List<String>> groupedParams =
_pathParams.groupListsBy<String>((String p) => p);
final Map<String, List<String>> dupParams =
Map<String, List<String>>.fromEntries(
groupedParams.entries
.where((MapEntry<String, List<String>> e) => e.value.length > 1),
);
if (dupParams.isNotEmpty) {
throw Exception(
'duplicate path params: ${dupParams.keys.join(', ')}',
assert(() {
// check path params
final Map<String, List<String>> groupedParams =
_pathParams.groupListsBy<String>((String p) => p);
final Map<String, List<String>> dupParams =
Map<String, List<String>>.fromEntries(
groupedParams.entries
.where((MapEntry<String, List<String>> e) => e.value.length > 1),
);
}

// check sub-routes
for (final GoRoute route in routes) {
// check paths
if (route.path != '/' &&
(route.path.startsWith('/') || route.path.endsWith('/'))) {
throw Exception(
'sub-route path may not start or end with /: ${route.path}',
);
assert(dupParams.isEmpty,
'duplicate path params: ${dupParams.keys.join(', ')}');

// check sub-routes
for (final GoRoute route in routes) {
// check paths
assert(
route.path == '/' ||
(!route.path.startsWith('/') && !route.path.endsWith('/')),
'sub-route path may not start or end with /: ${route.path}');
}
}
return true;
}());
}

final List<String> _pathParams = <String>[];
Expand Down
41 changes: 20 additions & 21 deletions packages/go_router/lib/src/go_route_match.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ class GoRouteMatch {
}) : assert(subloc.startsWith('/')),
assert(Uri.parse(subloc).queryParameters.isEmpty),
assert(fullpath.startsWith('/')),
assert(Uri.parse(fullpath).queryParameters.isEmpty) {
if (kDebugMode) {
for (final MapEntry<String, String> p in encodedParams.entries) {
assert(p.value == Uri.encodeComponent(Uri.decodeComponent(p.value)),
'encodedParams[${p.key}] is not encoded properly: "${p.value}"');
}
}
}
assert(Uri.parse(fullpath).queryParameters.isEmpty),
assert(() {
for (final MapEntry<String, String> p in encodedParams.entries) {
assert(p.value == Uri.encodeComponent(Uri.decodeComponent(p.value)),
'encodedParams[${p.key}] is not encoded properly: "${p.value}"');
}
return true;
}());

// ignore: public_member_api_docs
factory GoRouteMatch.matchNamed({
Expand All @@ -45,22 +45,21 @@ class GoRouteMatch {
}) {
assert(route.name != null);
assert(route.name!.toLowerCase() == name.toLowerCase());

// check that we have all the params we need
final List<String> paramNames = <String>[];
patternToRegExp(fullpath, paramNames);
for (final String paramName in paramNames) {
if (!params.containsKey(paramName)) {
throw Exception('missing param "$paramName" for $fullpath');
assert(() {
// check that we have all the params we need
final List<String> paramNames = <String>[];
patternToRegExp(fullpath, paramNames);
for (final String paramName in paramNames) {
assert(params.containsKey(paramName),
'missing param "$paramName" for $fullpath');
}
}

// check that we have don't have extra params
for (final String key in params.keys) {
if (!paramNames.contains(key)) {
throw Exception('unknown param "$key" for $fullpath');
// check that we have don't have extra params
for (final String key in params.keys) {
assert(paramNames.contains(key), 'unknown param "$key" for $fullpath');
}
}
return true;
}());

final Map<String, String> encodedParams = <String, String>{
for (final MapEntry<String, String> param in params.entries)
Expand Down
96 changes: 42 additions & 54 deletions packages/go_router/lib/src/go_router_delegate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ class GoRouterDelegate extends RouterDelegate<Uri>
required this.debugLogDiagnostics,
required this.routerNeglect,
this.restorationScopeId,
}) {
// check top-level route paths are valid
for (final GoRoute route in routes) {
if (!route.path.startsWith('/')) {
throw Exception('top-level path must start with "/": ${route.path}');
}
}

}) : assert(() {
// check top-level route paths are valid
for (final GoRoute route in routes) {
assert(route.path.startsWith('/'),
'top-level path must start with "/": ${route.path}');
}
return true;
}()) {
// cache the set of named routes for fast lookup
_cacheNamedRoutes(routes, '', _namedMatches);

Expand Down Expand Up @@ -110,10 +110,8 @@ class GoRouterDelegate extends RouterDelegate<Uri>

if (route.name != null) {
final String name = route.name!.toLowerCase();
if (namedFullpaths.containsKey(name)) {
throw Exception('duplication fullpaths for name "$name":'
'${namedFullpaths[name]!.fullpath}, $fullpath');
}
assert(!namedFullpaths.containsKey(name),
'duplication fullpaths for name "$name":${namedFullpaths[name]!.fullpath}, $fullpath');

// we only have a partial match until we have a location;
// we're really only caching the route and fullpath at this point
Expand Down Expand Up @@ -154,12 +152,9 @@ class GoRouterDelegate extends RouterDelegate<Uri>
params: params,
queryParams: queryParams,
);
if (match == null) {
throw Exception('unknown route name: $name');
}

assert(identical(match.queryParams, queryParams));
return _addQueryParams(match.subloc, queryParams);
assert(match != null, 'unknown route name: $name');
assert(identical(match!.queryParams, queryParams));
return _addQueryParams(match!.subloc, queryParams);
}

/// Navigate to the given location.
Expand All @@ -179,12 +174,8 @@ class GoRouterDelegate extends RouterDelegate<Uri>
/// Pop the top page off the GoRouter's page stack.
void pop() {
_matches.remove(_matches.last);
if (_matches.isEmpty) {
throw Exception(
'have popped the last page off of the stack; '
'there are no pages left to show',
);
}
assert(_matches.isNotEmpty,
'have popped the last page off of the stack; there are no pages left to show');
notifyListeners();
}

Expand Down Expand Up @@ -301,29 +292,28 @@ class GoRouterDelegate extends RouterDelegate<Uri>
return false;
}

if (Uri.tryParse(redir) == null) {
throw Exception('invalid redirect: $redir');
}

if (redirects.contains(redir)) {
redirects.add(redir);
final String msg =
'redirect loop detected: ${redirects.join(' => ')}';
throw Exception(msg);
}
assert(Uri.tryParse(redir) != null, 'invalid redirect: $redir');

assert(
!redirects.contains(redir),
'redirect loop detected: ${<String>[
...redirects,
redir
].join(' => ')}');
assert(
redirects.length < redirectLimit,
'too many redirects: ${<String>[
...redirects,
redir
].join(' => ')}');

redirects.add(redir);
if (redirects.length - 1 > redirectLimit) {
final String msg = 'too many redirects: ${redirects.join(' => ')}';
throw Exception(msg);
}

log.info('redirecting to $redir');
return true;
}

// keep looping till we're done redirecting
for (;;) {
while (true) {
final String loc = redirects.last;

// check for top-level redirect
Expand Down Expand Up @@ -449,23 +439,20 @@ class GoRouterDelegate extends RouterDelegate<Uri>
extra: extra,
).toList();

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

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(' => ')}');
}

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

throw Exception(sb.toString());
}

if (kDebugMode) {
assert(matchStacks.length == 1);
final GoRouteMatch match = matchStacks.first.last;
final String loc1 = _addQueryParams(match.subloc, match.queryParams);
Expand All @@ -475,7 +462,8 @@ class GoRouterDelegate extends RouterDelegate<Uri>
// 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;
}());

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.0.6
version: 3.0.7
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
6 changes: 5 additions & 1 deletion packages/go_router/test/go_route_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import 'package:go_router/go_router.dart';

void main() {
test('throws when a builder is not set', () {
expect(() => GoRoute(path: '/'), throwsException);
expect(() => GoRoute(path: '/'), throwsA(isAssertionError));
});

test('throws when a path is empty', () {
expect(() => GoRoute(path: ''), throwsA(isAssertionError));
});
}
2 changes: 1 addition & 1 deletion packages/go_router/test/go_router_delegate_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void main() {
() => delegate
..pop()
..pop(),
throwsException,
throwsA(isAssertionError),
);
});
});
Expand Down
Loading

0 comments on commit dff39d7

Please sign in to comment.