Skip to content

Commit 92c60ee

Browse files
chunhtaichunhtai
authored
[go_router]Fixes GoRouterState.location and GoRouterState.param to return correct value (#2786)
* Fixes GoRouterState.location and GoRouterState.param to return correct value * update * format
1 parent e5c16e0 commit 92c60ee

18 files changed

+531
-579
lines changed

‎packages/go_router/CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 5.2.0
2+
3+
- Fixes `GoRouterState.location` and `GoRouterState.param` to return correct value.
4+
- Cleans up `RouteMatch` and `RouteMatchList` API.
5+
16
## 5.1.10
27

38
- Fixes link of ShellRoute in README.

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

+17-23
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'package:flutter/widgets.dart';
66

77
import 'configuration.dart';
8+
import 'delegate.dart';
89
import 'logging.dart';
910
import 'match.dart';
1011
import 'matching.dart';
@@ -75,11 +76,7 @@ class RouteBuilder {
7576
registry: _registry, child: result);
7677
} on _RouteBuilderError catch (e) {
7778
return _buildErrorNavigator(
78-
context,
79-
e,
80-
Uri.parse(matchList.location.toString()),
81-
pop,
82-
configuration.navigatorKey);
79+
context, e, matchList.uri, pop, configuration.navigatorKey);
8380
}
8481
},
8582
),
@@ -124,13 +121,12 @@ class RouteBuilder {
124121
try {
125122
final Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPage =
126123
<GlobalKey<NavigatorState>, List<Page<Object?>>>{};
127-
final Map<String, String> params = <String, String>{};
128124
_buildRecursive(context, matchList, 0, onPop, routerNeglect, keyToPage,
129-
params, navigatorKey, registry);
125+
navigatorKey, registry);
130126
return keyToPage[navigatorKey]!;
131127
} on _RouteBuilderError catch (e) {
132128
return <Page<Object?>>[
133-
_buildErrorPage(context, e, matchList.location),
129+
_buildErrorPage(context, e, matchList.uri),
134130
];
135131
}
136132
}
@@ -142,7 +138,6 @@ class RouteBuilder {
142138
VoidCallback pop,
143139
bool routerNeglect,
144140
Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPages,
145-
Map<String, String> params,
146141
GlobalKey<NavigatorState> navigatorKey,
147142
Map<Page<Object?>, GoRouterState> registry,
148143
) {
@@ -157,11 +152,7 @@ class RouteBuilder {
157152
}
158153

159154
final RouteBase route = match.route;
160-
final Map<String, String> newParams = <String, String>{
161-
...params,
162-
...match.decodedParams
163-
};
164-
final GoRouterState state = buildState(match, newParams);
155+
final GoRouterState state = buildState(matchList, match);
165156
if (route is GoRoute) {
166157
final Page<Object?> page = _buildPageForRoute(context, state, match);
167158
registry[page] = state;
@@ -173,7 +164,7 @@ class RouteBuilder {
173164
keyToPages.putIfAbsent(goRouteNavKey, () => <Page<Object?>>[]).add(page);
174165

175166
_buildRecursive(context, matchList, startIndex + 1, pop, routerNeglect,
176-
keyToPages, newParams, navigatorKey, registry);
167+
keyToPages, navigatorKey, registry);
177168
} else if (route is ShellRoute) {
178169
// The key for the Navigator that will display this ShellRoute's page.
179170
final GlobalKey<NavigatorState> parentNavigatorKey = navigatorKey;
@@ -194,7 +185,7 @@ class RouteBuilder {
194185

195186
// Build the remaining pages
196187
_buildRecursive(context, matchList, startIndex + 1, pop, routerNeglect,
197-
keyToPages, newParams, shellNavigatorKey, registry);
188+
keyToPages, shellNavigatorKey, registry);
198189

199190
// Build the Navigator
200191
final Widget child = _buildNavigator(
@@ -235,25 +226,27 @@ class RouteBuilder {
235226
/// Helper method that builds a [GoRouterState] object for the given [match]
236227
/// and [params].
237228
@visibleForTesting
238-
GoRouterState buildState(RouteMatch match, Map<String, String> params) {
229+
GoRouterState buildState(RouteMatchList matchList, RouteMatch match) {
239230
final RouteBase route = match.route;
240-
String? name = '';
231+
String? name;
241232
String path = '';
242233
if (route is GoRoute) {
243234
name = route.name;
244235
path = route.path;
245236
}
237+
final RouteMatchList effectiveMatchList =
238+
match is ImperativeRouteMatch ? match.matches : matchList;
246239
return GoRouterState(
247240
configuration,
248-
location: match.fullUriString,
241+
location: effectiveMatchList.uri.toString(),
249242
subloc: match.subloc,
250243
name: name,
251244
path: path,
252-
fullpath: match.fullpath,
253-
params: params,
245+
fullpath: effectiveMatchList.fullpath,
246+
params: effectiveMatchList.pathParameters,
254247
error: match.error,
255-
queryParams: match.queryParams,
256-
queryParametersAll: match.queryParametersAll,
248+
queryParams: effectiveMatchList.uri.queryParameters,
249+
queryParametersAll: effectiveMatchList.uri.queryParametersAll,
257250
extra: match.extra,
258251
pageKey: match.pageKey,
259252
);
@@ -425,6 +418,7 @@ class RouteBuilder {
425418
queryParams: uri.queryParameters,
426419
queryParametersAll: uri.queryParametersAll,
427420
error: Exception(error),
421+
pageKey: const ValueKey<String>('error'),
428422
);
429423

430424
// If the error page builder is provided, use that, otherwise, if the error

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

+80-59
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import 'package:flutter/widgets.dart';
66

77
import 'configuration.dart';
88
import 'logging.dart';
9+
import 'misc/errors.dart';
910
import 'path_utils.dart';
1011
import 'typedefs.dart';
11-
1212
export 'route.dart';
1313
export 'state.dart';
1414

@@ -20,74 +20,95 @@ class RouteConfiguration {
2020
required this.redirectLimit,
2121
required this.topRedirect,
2222
required this.navigatorKey,
23-
}) {
23+
}) : assert(_debugCheckPath(routes, true)),
24+
assert(
25+
_debugVerifyNoDuplicatePathParameter(routes, <String, GoRoute>{})),
26+
assert(_debugCheckParentNavigatorKeys(
27+
routes, <GlobalKey<NavigatorState>>[navigatorKey])) {
2428
_cacheNameToPath('', routes);
25-
2629
log.info(_debugKnownRoutes());
30+
}
2731

28-
assert(() {
29-
for (final RouteBase route in routes) {
30-
if (route is GoRoute && !route.path.startsWith('/')) {
32+
static bool _debugCheckPath(List<RouteBase> routes, bool isTopLevel) {
33+
for (final RouteBase route in routes) {
34+
late bool subRouteIsTopLevel;
35+
if (route is GoRoute) {
36+
if (isTopLevel) {
3137
assert(route.path.startsWith('/'),
32-
'top-level path must start with "/": ${route.path}');
33-
} else if (route is ShellRoute) {
34-
for (final RouteBase route in routes) {
35-
if (route is GoRoute) {
36-
assert(route.path.startsWith('/'),
37-
'top-level path must start with "/": ${route.path}');
38-
}
39-
}
38+
'top-level path must start with "/": $route');
39+
} else {
40+
assert(!route.path.startsWith('/') && !route.path.endsWith('/'),
41+
'sub-route path may not start or end with /: $route');
4042
}
43+
subRouteIsTopLevel = false;
44+
} else if (route is ShellRoute) {
45+
subRouteIsTopLevel = isTopLevel;
4146
}
47+
_debugCheckPath(route.routes, subRouteIsTopLevel);
48+
}
49+
return true;
50+
}
4251

43-
// Check that each parentNavigatorKey refers to either a ShellRoute's
44-
// navigatorKey or the root navigator key.
45-
void checkParentNavigatorKeys(
46-
List<RouteBase> routes, List<GlobalKey<NavigatorState>> allowedKeys) {
47-
for (final RouteBase route in routes) {
48-
if (route is GoRoute) {
49-
final GlobalKey<NavigatorState>? parentKey =
50-
route.parentNavigatorKey;
51-
if (parentKey != null) {
52-
// Verify that the root navigator or a ShellRoute ancestor has a
53-
// matching navigator key.
54-
assert(
55-
allowedKeys.contains(parentKey),
56-
'parentNavigatorKey $parentKey must refer to'
57-
" an ancestor ShellRoute's navigatorKey or GoRouter's"
58-
' navigatorKey');
59-
60-
checkParentNavigatorKeys(
61-
route.routes,
62-
<GlobalKey<NavigatorState>>[
63-
// Once a parentNavigatorKey is used, only that navigator key
64-
// or keys above it can be used.
65-
...allowedKeys.sublist(0, allowedKeys.indexOf(parentKey) + 1),
66-
],
67-
);
68-
} else {
69-
checkParentNavigatorKeys(
70-
route.routes,
71-
<GlobalKey<NavigatorState>>[
72-
...allowedKeys,
73-
],
74-
);
75-
}
76-
} else if (route is ShellRoute && route.navigatorKey != null) {
77-
checkParentNavigatorKeys(
78-
route.routes,
79-
<GlobalKey<NavigatorState>>[
80-
...allowedKeys..add(route.navigatorKey)
81-
],
82-
);
83-
}
52+
// Check that each parentNavigatorKey refers to either a ShellRoute's
53+
// navigatorKey or the root navigator key.
54+
static bool _debugCheckParentNavigatorKeys(
55+
List<RouteBase> routes, List<GlobalKey<NavigatorState>> allowedKeys) {
56+
for (final RouteBase route in routes) {
57+
if (route is GoRoute) {
58+
final GlobalKey<NavigatorState>? parentKey = route.parentNavigatorKey;
59+
if (parentKey != null) {
60+
// Verify that the root navigator or a ShellRoute ancestor has a
61+
// matching navigator key.
62+
assert(
63+
allowedKeys.contains(parentKey),
64+
'parentNavigatorKey $parentKey must refer to'
65+
" an ancestor ShellRoute's navigatorKey or GoRouter's"
66+
' navigatorKey');
67+
68+
_debugCheckParentNavigatorKeys(
69+
route.routes,
70+
<GlobalKey<NavigatorState>>[
71+
// Once a parentNavigatorKey is used, only that navigator key
72+
// or keys above it can be used.
73+
...allowedKeys.sublist(0, allowedKeys.indexOf(parentKey) + 1),
74+
],
75+
);
76+
} else {
77+
_debugCheckParentNavigatorKeys(
78+
route.routes,
79+
<GlobalKey<NavigatorState>>[
80+
...allowedKeys,
81+
],
82+
);
8483
}
84+
} else if (route is ShellRoute && route.navigatorKey != null) {
85+
_debugCheckParentNavigatorKeys(
86+
route.routes,
87+
<GlobalKey<NavigatorState>>[...allowedKeys..add(route.navigatorKey)],
88+
);
8589
}
90+
}
91+
return true;
92+
}
8693

87-
checkParentNavigatorKeys(
88-
routes, <GlobalKey<NavigatorState>>[navigatorKey]);
89-
return true;
90-
}());
94+
static bool _debugVerifyNoDuplicatePathParameter(
95+
List<RouteBase> routes, Map<String, GoRoute> usedPathParams) {
96+
for (final RouteBase route in routes) {
97+
if (route is! GoRoute) {
98+
continue;
99+
}
100+
for (final String pathParam in route.pathParams) {
101+
if (usedPathParams.containsKey(pathParam)) {
102+
final bool sameRoute = usedPathParams[pathParam] == route;
103+
throw GoError(
104+
"duplicate path parameter, '$pathParam' found in ${sameRoute ? '$route' : '${usedPathParams[pathParam]}, and $route'}");
105+
}
106+
usedPathParams[pathParam] = route;
107+
}
108+
_debugVerifyNoDuplicatePathParameter(route.routes, usedPathParams);
109+
route.pathParams.forEach(usedPathParams.remove);
110+
}
111+
return true;
91112
}
92113

93114
/// The list of top level routes used by [GoRouterDelegate].

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

+32-21
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import 'builder.dart';
1111
import 'configuration.dart';
1212
import 'match.dart';
1313
import 'matching.dart';
14-
import 'misc/errors.dart';
1514
import 'typedefs.dart';
1615

1716
/// GoRouter implementation of [RouterDelegate].
@@ -44,7 +43,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
4443
/// Set to true to disable creating history entries on the web.
4544
final bool routerNeglect;
4645

47-
RouteMatchList _matchList = RouteMatchList.empty();
46+
RouteMatchList _matchList = RouteMatchList.empty;
4847

4948
/// Stores the number of times each route route has been pushed.
5049
///
@@ -95,26 +94,21 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
9594
}
9695

9796
/// Pushes the given location onto the page stack
98-
void push(RouteMatch match) {
99-
if (match.route is ShellRoute) {
100-
throw GoError('ShellRoutes cannot be pushed');
101-
}
97+
void push(RouteMatchList matches) {
98+
assert(matches.last.route is! ShellRoute);
10299

103100
// Remap the pageKey to allow any number of the same page on the stack
104-
final String fullPath = match.fullpath;
105-
final int count = (_pushCounts[fullPath] ?? 0) + 1;
106-
_pushCounts[fullPath] = count;
107-
final ValueKey<String> pageKey = ValueKey<String>('$fullPath-p$count');
108-
final RouteMatch newPageKeyMatch = RouteMatch(
109-
route: match.route,
110-
subloc: match.subloc,
111-
fullpath: match.fullpath,
112-
encodedParams: match.encodedParams,
113-
queryParams: match.queryParams,
114-
queryParametersAll: match.queryParametersAll,
115-
extra: match.extra,
116-
error: match.error,
101+
final int count = (_pushCounts[matches.fullpath] ?? 0) + 1;
102+
_pushCounts[matches.fullpath] = count;
103+
final ValueKey<String> pageKey =
104+
ValueKey<String>('${matches.fullpath}-p$count');
105+
final ImperativeRouteMatch newPageKeyMatch = ImperativeRouteMatch(
106+
route: matches.last.route,
107+
subloc: matches.last.subloc,
108+
extra: matches.last.extra,
109+
error: matches.last.error,
117110
pageKey: pageKey,
111+
matches: matches,
118112
);
119113

120114
_matchList.push(newPageKeyMatch);
@@ -170,9 +164,9 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
170164
///
171165
/// See also:
172166
/// * [push] which pushes the given location onto the page stack.
173-
void replace(RouteMatch match) {
167+
void replace(RouteMatchList matches) {
174168
_matchList.pop();
175-
push(match); // [push] will notify the listeners.
169+
push(matches); // [push] will notify the listeners.
176170
}
177171

178172
/// For internal use; visible for testing only.
@@ -209,3 +203,20 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
209203
return SynchronousFuture<void>(null);
210204
}
211205
}
206+
207+
/// The route match that represent route pushed through [GoRouter.push].
208+
// TODO(chunhtai): Removes this once imperative API no longer insert route match.
209+
class ImperativeRouteMatch extends RouteMatch {
210+
/// Constructor for [ImperativeRouteMatch].
211+
ImperativeRouteMatch({
212+
required super.route,
213+
required super.subloc,
214+
required super.extra,
215+
required super.error,
216+
required super.pageKey,
217+
required this.matches,
218+
});
219+
220+
/// The matches that produces this route match.
221+
final RouteMatchList matches;
222+
}

0 commit comments

Comments
 (0)