Skip to content

Commit dff39d7

Browse files
chunhtaichunhtai
authored
[go_router]refactor runtime check to assert (#1362)
1 parent ee4f492 commit dff39d7

8 files changed

+110
-128
lines changed

‎packages/go_router/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 3.0.7
2+
3+
- Refactors runtime checks to assertions.
4+
15
## 3.0.6
26

37
- Exports inherited_go_router.dart file.

‎packages/go_router/lib/src/go_route.dart

+27-40
Original file line numberDiff line numberDiff line change
@@ -21,51 +21,38 @@ class GoRoute {
2121
this.builder = _invalidBuilder,
2222
this.routes = const <GoRoute>[],
2323
this.redirect = _noRedirection,
24-
}) {
25-
if (path.isEmpty) {
26-
throw Exception('GoRoute path cannot be empty');
27-
}
28-
29-
if (name != null && name!.isEmpty) {
30-
throw Exception('GoRoute name cannot be empty');
31-
}
32-
33-
if (pageBuilder == null &&
34-
builder == _invalidBuilder &&
35-
redirect == _noRedirection) {
36-
throw Exception(
37-
'GoRoute builder parameter not set\n'
38-
'See gorouter.dev/redirection#considerations for details',
39-
);
40-
}
41-
24+
}) : assert(path.isNotEmpty, 'GoRoute path cannot be empty'),
25+
assert(name == null || name.isNotEmpty, 'GoRoute name cannot be empty'),
26+
assert(
27+
pageBuilder != null ||
28+
builder != _invalidBuilder ||
29+
redirect != _noRedirection,
30+
'GoRoute builder parameter not set\nSee gorouter.dev/redirection#considerations for details') {
4231
// cache the path regexp and parameters
4332
_pathRE = patternToRegExp(path, _pathParams);
4433

45-
// check path params
46-
final Map<String, List<String>> groupedParams =
47-
_pathParams.groupListsBy<String>((String p) => p);
48-
final Map<String, List<String>> dupParams =
49-
Map<String, List<String>>.fromEntries(
50-
groupedParams.entries
51-
.where((MapEntry<String, List<String>> e) => e.value.length > 1),
52-
);
53-
if (dupParams.isNotEmpty) {
54-
throw Exception(
55-
'duplicate path params: ${dupParams.keys.join(', ')}',
34+
assert(() {
35+
// check path params
36+
final Map<String, List<String>> groupedParams =
37+
_pathParams.groupListsBy<String>((String p) => p);
38+
final Map<String, List<String>> dupParams =
39+
Map<String, List<String>>.fromEntries(
40+
groupedParams.entries
41+
.where((MapEntry<String, List<String>> e) => e.value.length > 1),
5642
);
57-
}
58-
59-
// check sub-routes
60-
for (final GoRoute route in routes) {
61-
// check paths
62-
if (route.path != '/' &&
63-
(route.path.startsWith('/') || route.path.endsWith('/'))) {
64-
throw Exception(
65-
'sub-route path may not start or end with /: ${route.path}',
66-
);
43+
assert(dupParams.isEmpty,
44+
'duplicate path params: ${dupParams.keys.join(', ')}');
45+
46+
// check sub-routes
47+
for (final GoRoute route in routes) {
48+
// check paths
49+
assert(
50+
route.path == '/' ||
51+
(!route.path.startsWith('/') && !route.path.endsWith('/')),
52+
'sub-route path may not start or end with /: ${route.path}');
6753
}
68-
}
54+
return true;
55+
}());
6956
}
7057

7158
final List<String> _pathParams = <String>[];

‎packages/go_router/lib/src/go_route_match.dart

+20-21
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@ class GoRouteMatch {
2525
}) : assert(subloc.startsWith('/')),
2626
assert(Uri.parse(subloc).queryParameters.isEmpty),
2727
assert(fullpath.startsWith('/')),
28-
assert(Uri.parse(fullpath).queryParameters.isEmpty) {
29-
if (kDebugMode) {
30-
for (final MapEntry<String, String> p in encodedParams.entries) {
31-
assert(p.value == Uri.encodeComponent(Uri.decodeComponent(p.value)),
32-
'encodedParams[${p.key}] is not encoded properly: "${p.value}"');
33-
}
34-
}
35-
}
28+
assert(Uri.parse(fullpath).queryParameters.isEmpty),
29+
assert(() {
30+
for (final MapEntry<String, String> p in encodedParams.entries) {
31+
assert(p.value == Uri.encodeComponent(Uri.decodeComponent(p.value)),
32+
'encodedParams[${p.key}] is not encoded properly: "${p.value}"');
33+
}
34+
return true;
35+
}());
3636

3737
// ignore: public_member_api_docs
3838
factory GoRouteMatch.matchNamed({
@@ -45,22 +45,21 @@ class GoRouteMatch {
4545
}) {
4646
assert(route.name != null);
4747
assert(route.name!.toLowerCase() == name.toLowerCase());
48-
49-
// check that we have all the params we need
50-
final List<String> paramNames = <String>[];
51-
patternToRegExp(fullpath, paramNames);
52-
for (final String paramName in paramNames) {
53-
if (!params.containsKey(paramName)) {
54-
throw Exception('missing param "$paramName" for $fullpath');
48+
assert(() {
49+
// check that we have all the params we need
50+
final List<String> paramNames = <String>[];
51+
patternToRegExp(fullpath, paramNames);
52+
for (final String paramName in paramNames) {
53+
assert(params.containsKey(paramName),
54+
'missing param "$paramName" for $fullpath');
5555
}
56-
}
5756

58-
// check that we have don't have extra params
59-
for (final String key in params.keys) {
60-
if (!paramNames.contains(key)) {
61-
throw Exception('unknown param "$key" for $fullpath');
57+
// check that we have don't have extra params
58+
for (final String key in params.keys) {
59+
assert(paramNames.contains(key), 'unknown param "$key" for $fullpath');
6260
}
63-
}
61+
return true;
62+
}());
6463

6564
final Map<String, String> encodedParams = <String, String>{
6665
for (final MapEntry<String, String> param in params.entries)

‎packages/go_router/lib/src/go_router_delegate.dart

+42-54
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ class GoRouterDelegate extends RouterDelegate<Uri>
3838
required this.debugLogDiagnostics,
3939
required this.routerNeglect,
4040
this.restorationScopeId,
41-
}) {
42-
// check top-level route paths are valid
43-
for (final GoRoute route in routes) {
44-
if (!route.path.startsWith('/')) {
45-
throw Exception('top-level path must start with "/": ${route.path}');
46-
}
47-
}
48-
41+
}) : assert(() {
42+
// check top-level route paths are valid
43+
for (final GoRoute route in routes) {
44+
assert(route.path.startsWith('/'),
45+
'top-level path must start with "/": ${route.path}');
46+
}
47+
return true;
48+
}()) {
4949
// cache the set of named routes for fast lookup
5050
_cacheNamedRoutes(routes, '', _namedMatches);
5151

@@ -110,10 +110,8 @@ class GoRouterDelegate extends RouterDelegate<Uri>
110110

111111
if (route.name != null) {
112112
final String name = route.name!.toLowerCase();
113-
if (namedFullpaths.containsKey(name)) {
114-
throw Exception('duplication fullpaths for name "$name":'
115-
'${namedFullpaths[name]!.fullpath}, $fullpath');
116-
}
113+
assert(!namedFullpaths.containsKey(name),
114+
'duplication fullpaths for name "$name":${namedFullpaths[name]!.fullpath}, $fullpath');
117115

118116
// we only have a partial match until we have a location;
119117
// we're really only caching the route and fullpath at this point
@@ -154,12 +152,9 @@ class GoRouterDelegate extends RouterDelegate<Uri>
154152
params: params,
155153
queryParams: queryParams,
156154
);
157-
if (match == null) {
158-
throw Exception('unknown route name: $name');
159-
}
160-
161-
assert(identical(match.queryParams, queryParams));
162-
return _addQueryParams(match.subloc, queryParams);
155+
assert(match != null, 'unknown route name: $name');
156+
assert(identical(match!.queryParams, queryParams));
157+
return _addQueryParams(match!.subloc, queryParams);
163158
}
164159

165160
/// Navigate to the given location.
@@ -179,12 +174,8 @@ class GoRouterDelegate extends RouterDelegate<Uri>
179174
/// Pop the top page off the GoRouter's page stack.
180175
void pop() {
181176
_matches.remove(_matches.last);
182-
if (_matches.isEmpty) {
183-
throw Exception(
184-
'have popped the last page off of the stack; '
185-
'there are no pages left to show',
186-
);
187-
}
177+
assert(_matches.isNotEmpty,
178+
'have popped the last page off of the stack; there are no pages left to show');
188179
notifyListeners();
189180
}
190181

@@ -301,29 +292,28 @@ class GoRouterDelegate extends RouterDelegate<Uri>
301292
return false;
302293
}
303294

304-
if (Uri.tryParse(redir) == null) {
305-
throw Exception('invalid redirect: $redir');
306-
}
307-
308-
if (redirects.contains(redir)) {
309-
redirects.add(redir);
310-
final String msg =
311-
'redirect loop detected: ${redirects.join(' => ')}';
312-
throw Exception(msg);
313-
}
295+
assert(Uri.tryParse(redir) != null, 'invalid redirect: $redir');
296+
297+
assert(
298+
!redirects.contains(redir),
299+
'redirect loop detected: ${<String>[
300+
...redirects,
301+
redir
302+
].join(' => ')}');
303+
assert(
304+
redirects.length < redirectLimit,
305+
'too many redirects: ${<String>[
306+
...redirects,
307+
redir
308+
].join(' => ')}');
314309

315310
redirects.add(redir);
316-
if (redirects.length - 1 > redirectLimit) {
317-
final String msg = 'too many redirects: ${redirects.join(' => ')}';
318-
throw Exception(msg);
319-
}
320-
321311
log.info('redirecting to $redir');
322312
return true;
323313
}
324314

325315
// keep looping till we're done redirecting
326-
for (;;) {
316+
while (true) {
327317
final String loc = redirects.last;
328318

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

452-
if (matchStacks.isEmpty) {
453-
throw Exception('no routes for location: $location');
454-
}
442+
assert(matchStacks.isNotEmpty, 'no routes for location: $location');
443+
assert(() {
444+
if (matchStacks.length > 1) {
445+
final StringBuffer sb = StringBuffer()
446+
..writeln('too many routes for location: $location');
455447

456-
if (matchStacks.length > 1) {
457-
final StringBuffer sb = StringBuffer()
458-
..writeln('too many routes for location: $location');
448+
for (final List<GoRouteMatch> stack in matchStacks) {
449+
sb.writeln(
450+
'\t${stack.map((GoRouteMatch m) => m.route.path).join(' => ')}');
451+
}
459452

460-
for (final List<GoRouteMatch> stack in matchStacks) {
461-
sb.writeln(
462-
'\t${stack.map((GoRouteMatch m) => m.route.path).join(' => ')}');
453+
assert(false, sb.toString());
463454
}
464455

465-
throw Exception(sb.toString());
466-
}
467-
468-
if (kDebugMode) {
469456
assert(matchStacks.length == 1);
470457
final GoRouteMatch match = matchStacks.first.last;
471458
final String loc1 = _addQueryParams(match.subloc, match.queryParams);
@@ -475,7 +462,8 @@ class GoRouterDelegate extends RouterDelegate<Uri>
475462
// NOTE: match the lower case, since subloc is canonicalized to match the
476463
// path case whereas the location can be any case
477464
assert(loc1.toLowerCase() == loc2.toLowerCase(), '$loc1 != $loc2');
478-
}
465+
return true;
466+
}());
479467

480468
return matchStacks.first;
481469
}

‎packages/go_router/pubspec.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: go_router
22
description: A declarative router for Flutter based on Navigation 2 supporting
33
deep linking, data-driven routes and more
4-
version: 3.0.6
4+
version: 3.0.7
55
repository: https://github.com/flutter/packages/tree/main/packages/go_router
66
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
77

‎packages/go_router/test/go_route_test.dart

+5-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import 'package:go_router/go_router.dart';
77

88
void main() {
99
test('throws when a builder is not set', () {
10-
expect(() => GoRoute(path: '/'), throwsException);
10+
expect(() => GoRoute(path: '/'), throwsA(isAssertionError));
11+
});
12+
13+
test('throws when a path is empty', () {
14+
expect(() => GoRoute(path: ''), throwsA(isAssertionError));
1115
});
1216
}

‎packages/go_router/test/go_router_delegate_test.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void main() {
4545
() => delegate
4646
..pop()
4747
..pop(),
48-
throwsException,
48+
throwsA(isAssertionError),
4949
);
5050
});
5151
});

0 commit comments

Comments
 (0)