Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 60 additions & 1 deletion packages/go_router/lib/src/route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1476,8 +1476,67 @@ class StatefulNavigationShellState extends State<StatefulNavigationShell> with R
if (matchList != null && matchList.isNotEmpty) {
_router.restore(matchList);
} else {
_router.go(widget._effectiveInitialBranchLocation(index));
final RouteMatchList? initialMatchList = _initialMatchListForBranch(index);
if (initialMatchList != null) {
_router.restore(initialMatchList);
} else {
_router.go(widget._effectiveInitialBranchLocation(index));
}
}
}

/// Builds the match list for the initial location of the branch at [index],
/// keeping the parts of the current match list that lie outside this shell
/// route.
///
/// Switching to a branch without preserved state must not drop pages the
/// parent Navigators have below the shell route. This grafts the shell match
/// for the initial branch location into the current match list instead of
/// navigating from scratch. See
/// https://github.com/flutter/flutter/issues/188295.
RouteMatchList? _initialMatchListForBranch(int index) {
final RouteMatchList initialMatchList = _router.configuration.findMatch(
Uri.parse(widget._effectiveInitialBranchLocation(index)),
);
ShellRouteMatch? newShellMatch;
initialMatchList.visitRouteMatches((RouteMatchBase match) {
if (match is ShellRouteMatch && match.route == route) {
newShellMatch = match;
return false;
}
return true;
});
Comment on lines +1501 to +1508

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current logic for finding newShellMatch and returning the loop control boolean can be simplified to improve readability and reduce cognitive load. Using an explicit if statement with an early return of false is more idiomatic and easier to understand at a glance.

Suggested change
ShellRouteMatch? newShellMatch;
initialMatchList.visitRouteMatches((RouteMatchBase match) {
newShellMatch = match is ShellRouteMatch && match.route == route ? match : newShellMatch;
return newShellMatch == null;
});
ShellRouteMatch? newShellMatch;
initialMatchList.visitRouteMatches((RouteMatchBase match) {
if (match is ShellRouteMatch && match.route == route) {
newShellMatch = match;
return false;
}
return true;
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The old return inferred the stop from newShellMatch instead of saying it at the match.

if (newShellMatch == null) {
return null;
}

List<RouteMatchBase> replaceShellMatch(List<RouteMatchBase> matches) {
return matches.map((RouteMatchBase match) {
if (match is ShellRouteMatch) {
if (match.route == route) {
return newShellMatch!;
}
return match.copyWith(matches: replaceShellMatch(match.matches));
}
return match;
}).toList();
}

final RouteMatchList currentMatchList = _scopedMatchList(
widget.shellRouteContext.routeMatchList,
);
final List<RouteMatchBase> matches = replaceShellMatch(currentMatchList.matches);
return RouteMatchList(
matches: matches,
uri: initialMatchList.uri,
// The branch is navigated to as if by [GoRouter.go], which carries no
// extra. The object the outer location was given must not leak into it.
extra: initialMatchList.extra,
pathParameters: <String, String>{
...currentMatchList.pathParameters,
...initialMatchList.pathParameters,
},
);
}

@override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changelog: |
- Fixes pages on parent navigators being dropped when switching to a not yet loaded branch of a `StatefulShellRoute`.
version: patch
106 changes: 106 additions & 0 deletions packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3211,6 +3211,112 @@ void main() {
expect(matches.pathParameters['pid'], pid);
});

testWidgets('StatefulShellRoute keeps parent navigator pages when switching '
'to an unloaded branch', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/188295
StatefulNavigationShell? routeState;
final routes = <RouteBase>[
GoRoute(
path: '/',
builder: (BuildContext context, GoRouterState state) => const Text('Home'),
),
StatefulShellRoute.indexedStack(
builder:
(BuildContext context, GoRouterState state, StatefulNavigationShell navigationShell) {
routeState = navigationShell;
return navigationShell;
},
branches: <StatefulShellBranch>[
StatefulShellBranch(
routes: <RouteBase>[
GoRoute(
path: '/a',
builder: (BuildContext context, GoRouterState state) => const Text('Screen A'),
),
],
),
StatefulShellBranch(
routes: <RouteBase>[
GoRoute(
path: '/b',
builder: (BuildContext context, GoRouterState state) => const Text('Screen B'),
),
],
),
],
),
];

final GoRouter router = await createRouter(routes, tester);
expect(find.text('Home'), findsOneWidget);

router.push('/a');
await tester.pumpAndSettle();
expect(find.text('Screen A'), findsOneWidget);
expect(router.canPop(), isTrue);

routeState!.goBranch(1);
await tester.pumpAndSettle();
expect(find.text('Screen B'), findsOneWidget);
expect(router.routerDelegate.currentConfiguration.uri.toString(), '/b');
expect(router.canPop(), isTrue);

router.pop();
await tester.pumpAndSettle();
expect(find.text('Home'), findsOneWidget);
});

testWidgets('StatefulShellRoute does not carry extra into an unloaded branch', (
WidgetTester tester,
) async {
StatefulNavigationShell? routeState;
Object? extraOnB;
final routes = <RouteBase>[
StatefulShellRoute.indexedStack(
builder:
(BuildContext context, GoRouterState state, StatefulNavigationShell navigationShell) {
routeState = navigationShell;
return navigationShell;
},
branches: <StatefulShellBranch>[
StatefulShellBranch(
routes: <RouteBase>[
GoRoute(
path: '/a',
builder: (BuildContext context, GoRouterState state) => const Text('Screen A'),
),
],
),
StatefulShellBranch(
routes: <RouteBase>[
GoRoute(
path: '/b',
builder: (BuildContext context, GoRouterState state) {
extraOnB = state.extra;
return const Text('Screen B');
},
),
],
),
],
),
];

final GoRouter router = await createRouter(
routes,
tester,
initialLocation: '/a',
initialExtra: Object(),
);
expect(find.text('Screen A'), findsOneWidget);

routeState!.goBranch(1);
await tester.pumpAndSettle();
expect(find.text('Screen B'), findsOneWidget);
expect(extraOnB, isNull);
expect(router.routerDelegate.currentConfiguration.uri.toString(), '/b');
});

testWidgets('StatefulShellRoute preserve extra when switching branch', (
WidgetTester tester,
) async {
Expand Down