-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[go_router] Nested stateful navigation with ShellRoute #2650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
fa4c542
76b17ae
7612a55
62574bd
850f41a
1c04588
1db6bba
6028c90
5f2e995
461efd9
0315e1a
510ec34
e82647a
e38fa32
f38b9bf
89f1dd7
3a00a54
b0a6264
b729d64
3b3909f
f0b2a1b
eb6d4d3
016be76
4ae26d7
4a9889a
1f3f01a
3b38641
d701ab5
bb6240d
245d47b
bda571b
da1ea0b
59c19e7
723de76
4c0a91d
7556965
ee9bde9
e9a7029
59a6b05
06ab1e9
1b877c8
e542a05
2215a51
b180653
8fdfb82
9240ea4
93bce8e
20dc0c6
376e80f
2b2ff91
5c9fe04
81e1296
59e3b66
8636bf8
1c509f1
42c7b7d
62e7fc1
9a7069a
ae48ede
141fdc1
b6b289f
4d05d99
d4edd47
7b9de47
703815c
9f88928
5ca533d
f5f0ecb
4a2eac3
fc7bd54
a65f9df
816acb2
ee2a845
89b82c5
90b9d62
7392264
c0253f6
2c4afa1
845c052
193a267
37c4969
c562482
38b5772
db22bac
d8d1641
4c4b7b0
f08f548
6f1b047
07ee030
9ffff5c
e9d40ee
0ea48cb
4abcaa9
ce23558
351ceb2
4cb0f1e
e6a4f71
873bf34
5b668f8
565c3cd
5ee1a8f
358551f
f9a2608
87211aa
257a272
08148e5
c18941f
b1ce762
75e43d0
9f54b4e
1226f44
881be86
6bbbd29
fd5412f
a5234a6
b9428cb
ee047c8
ddc71f0
5041bea
01cce04
9315373
89341ef
2c298cd
4c22b53
a7d419f
7285788
9983887
ad43837
3a83007
ec6722c
9bb0da9
a1c7a8f
477b47c
7f25f0c
6f35636
bb5c08f
16c095c
a18a5a5
abbd01b
0f36623
316cf8c
79a0c63
3696448
763b136
2f48831
709ee7a
bb7fd9e
9eaf1af
527a806
86a72e1
57b761f
85fa0b6
d75c5e2
bd1c018
c13cd71
c7b1295
0f842ee
80c396f
04b549a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Removed some obsolete code.
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,10 @@ class RouteBuilder { | |
| final Map<GlobalKey<NavigatorState>, HeroController> _goHeroCache = | ||
| <GlobalKey<NavigatorState>, HeroController>{}; | ||
|
|
||
| /// State for any active stateful shell routes | ||
| final Map<ShellRouteBase, Object?> _shellRouteState = | ||
|
||
| <ShellRouteBase, Object?>{}; | ||
|
|
||
| /// Builds the top-level Navigator for the given [RouteMatchList]. | ||
| Widget build( | ||
| BuildContext context, | ||
|
|
@@ -154,43 +158,19 @@ class RouteBuilder { | |
| /// Clean up previous cache to prevent memory leak. | ||
| _goHeroCache.removeWhere( | ||
| (GlobalKey<NavigatorState> key, _) => !keyToPage.keys.contains(key)); | ||
| } | ||
| } | ||
|
|
||
| /// Builds a preloaded nested [Navigator], containing a sub-tree (beginning | ||
| /// at startIndex) of the provided route match list. | ||
| Widget buildPreloadedNestedNavigator( | ||
| BuildContext context, | ||
| RouteMatchList matchList, | ||
| int startIndex, | ||
| bool routerNeglect, | ||
| GlobalKey<NavigatorState> navigatorKey, | ||
| {List<NavigatorObserver>? observers, | ||
| String? restorationScopeId}) { | ||
| final Map<GlobalKey<NavigatorState>, List<Page<Object?>>> keyToPage = | ||
| <GlobalKey<NavigatorState>, List<Page<Object?>>>{}; | ||
| try { | ||
| final _PagePopContext pagePopContext = _PagePopContext._(onPopPage); | ||
| _buildRecursive( | ||
| context, | ||
| matchList, | ||
| startIndex, | ||
| pagePopContext, | ||
| routerNeglect, | ||
| keyToPage, | ||
| navigatorKey, <Page<Object?>, GoRouterState>{}); | ||
|
|
||
| return _buildNavigator( | ||
| pagePopContext.onPopPage, | ||
| keyToPage[navigatorKey]!, | ||
| navigatorKey, | ||
| observers: observers, | ||
| restorationScopeId: restorationScopeId, | ||
| heroController: _getHeroController(context), | ||
| ); | ||
| } on _RouteBuilderError catch (e) { | ||
| return _buildErrorNavigator( | ||
| context, e, matchList, onPopPage, configuration.navigatorKey); | ||
| /// Clean up cache of shell route states, but keep states for shell routes | ||
| /// in the current match list, as well as in any nested stateful shell | ||
| /// routes | ||
| if (_shellRouteState.isNotEmpty) { | ||
| Iterable<ShellRouteBase> shellRoutes = matchList.matches | ||
| .map((RouteMatch e) => e.route) | ||
| .whereType<ShellRouteBase>(); | ||
| shellRoutes = RouteConfiguration.routesRecursively(shellRoutes) | ||
| .whereType<ShellRouteBase>(); | ||
| _shellRouteState | ||
| .removeWhere((ShellRouteBase key, _) => !shellRoutes.contains(key)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -273,12 +253,20 @@ class RouteBuilder { | |
| ); | ||
| } | ||
|
|
||
| final ShellRouteContext shellRouteContext = ShellRouteContext( | ||
| ShellRouteContext shellRouteContext = ShellRouteContext( | ||
| subRoute: subRoute, | ||
| routeMatchList: matchList, | ||
| shellRouteState: _shellRouteState[route], | ||
| navigatorBuilder: buildShellNavigator, | ||
| ); | ||
|
|
||
| // Call the ShellRouteBase to create/update the shell route state | ||
| final Object? shellRouteState = | ||
| route.updateShellState(context, state, shellRouteContext); | ||
| _shellRouteState[route] = shellRouteState; | ||
| shellRouteContext = | ||
| shellRouteContext.copyWith(shellRouteState: shellRouteState); | ||
|
|
||
| // Build the Page for this route | ||
| final Page<Object?> page = _buildPageForRoute( | ||
| context, state, match, pagePopContext, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,8 +56,12 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> { | |
| ) { | ||
| late final RouteMatchList initialMatches; | ||
| try { | ||
| initialMatches = matcher.findMatch(routeInformation.location!, | ||
| extra: routeInformation.state); | ||
| if (routeInformation is PreParsedRouteInformation) { | ||
| initialMatches = routeInformation.matchlist; | ||
| } else { | ||
| initialMatches = matcher.findMatch(routeInformation.location!, | ||
| extra: routeInformation.state); | ||
| } | ||
| } on MatcherError { | ||
| log.info('No initial matches: ${routeInformation.location}'); | ||
|
|
||
|
|
@@ -124,3 +128,13 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> { | |
| ); | ||
| } | ||
| } | ||
|
|
||
| /// Pre-parsed [RouteInformation] that contains a [RouteMatchList]. | ||
|
||
| class PreParsedRouteInformation extends RouteInformation { | ||
| /// Creates a [PreParsedRouteInformation]. | ||
| PreParsedRouteInformation( | ||
| {super.location, super.state, required this.matchlist}); | ||
|
||
|
|
||
| /// The pre-parsed [RouteMatchList]. | ||
| final RouteMatchList matchlist; | ||
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this only take a single RouteBase? I can see the behavior difference with a single RouteBase vs multiple RouteBases when you switch to a different uri. but multiple RouteBases may make route parsing a bit complex. Also it is hard to keep track if you may have duplicated GoRoute in different branch.
Is that a use case that require multiple RouteBases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think the main reason was to better support redirection and the possibility to have different root routes for a branch, depending on the application state.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that can be a valid use case. Does branch has priority? for example, if a string location would match two different branch, does one take priority over the other. If so, how the priorty is design. We should definitely document the behavior.
If the branch should not have priority, we should add an assert that the route can't overlap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure any priority is needed, but I guess an overlap check could be good. But perhaps such a check should be added for all routes, not just for StatefulShellRoute branches, or is there something preventing such a check (apart from parametrised routes making such check a bit harder)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is ok for subroutes to overlaps for example this is valid
routes: [
GoRoute(path: '/page/myspecialpage'),
GoRoute(path: '/page/*'),
]
in this case you can do special handling for a specific match in a wild card match, but I am not sure about branches if there are multiple root route though... maybe that is ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Late response to this (possible answered elsewhere), but this should be fine - overlapping routes should work the same as in other places. The only difference is that StatefulShellRoute needs to determine which Navigator a sub-route should be placed on (handled in
navigatorKeyForSubRoute), but overlapping routes shouldn't affect this.A related thing that I realised though, is that there probably needs to be an assertion to check that the
defaultRouteof a branch doesn't use parameters. In such case, theinitialLocationmust be used instead.