From 1c538b969b16cbf9bae814b63ae59e5d93b3d995 Mon Sep 17 00:00:00 2001 From: Kravchenko Igor Date: Sat, 28 Feb 2026 22:56:12 +0100 Subject: [PATCH 1/2] [go_router] prevent ShellRoute key collisions when sibling shells coexist --- packages/go_router/CHANGELOG.md | 5 + packages/go_router/lib/src/builder.dart | 4 +- packages/go_router/lib/src/match.dart | 12 +- packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/go_router_test.dart | 2 +- .../go_router/test/imperative_api_test.dart | 123 ++++++++++++++++++ packages/go_router/test/match_test.dart | 18 ++- 7 files changed, 155 insertions(+), 11 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index f1d95f90c9fc..367f25ecfaf2 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,8 @@ +## 17.1.1 + +- Fixes `ShellRoute` key collisions when `push()` keeps multiple shell branches in + the widget tree at the same time. + ## 17.1.0 - Adds `TypedQueryParameter` annotation to override parameter names in `TypedGoRoute` constructors. diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index 4eb036ea190b..a1dc26b43471 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -108,7 +108,7 @@ class RouteBuilder { context, _CustomNavigator( // The state needs to persist across rebuild. - key: GlobalObjectKey(configuration.navigatorKey.hashCode), + key: GlobalObjectKey(configuration.navigatorKey), navigatorKey: configuration.navigatorKey, observers: observers, navigatorRestorationId: restorationScopeId, @@ -300,7 +300,7 @@ class _CustomNavigatorState extends State<_CustomNavigator> { canPop: match.matches.length == 1, child: _CustomNavigator( // The state needs to persist across rebuild. - key: GlobalObjectKey(navigatorKey.hashCode), + key: GlobalObjectKey(navigatorKey), navigatorRestorationId: restorationScopeId, navigatorKey: navigatorKey, matches: match.matches, diff --git a/packages/go_router/lib/src/match.dart b/packages/go_router/lib/src/match.dart index 87f588a3eeef..c20014171983 100644 --- a/packages/go_router/lib/src/match.dart +++ b/packages/go_router/lib/src/match.dart @@ -175,14 +175,22 @@ abstract class RouteMatchBase with Diagnosticable { if (subRouteMatches?.isEmpty ?? true) { return _empty; } + final matchNavigatorKey = route is ShellRoute + ? GlobalKey(debugLabel: navigatorKeyUsed.toString()) + : navigatorKeyUsed; + final pageKey = route is ShellRoute + ? ValueKey( + '${route.hashCode}-${identityHashCode(matchNavigatorKey)}', + ) + : ValueKey(route.hashCode.toString()); final RouteMatchBase result = ShellRouteMatch( route: route, // The RouteConfiguration should have asserted the subRouteMatches must // have at least one match for this ShellRouteBase. matches: subRouteMatches!.remove(null)!, matchedLocation: remainingLocation, - pageKey: ValueKey(route.hashCode.toString()), - navigatorKey: navigatorKeyUsed, + pageKey: pageKey, + navigatorKey: matchNavigatorKey, ); subRouteMatches .putIfAbsent(parentKey, () => []) diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 55ad56b1058c..91402a841cb7 100644 --- a/packages/go_router/pubspec.yaml +++ b/packages/go_router/pubspec.yaml @@ -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: 17.1.0 +version: 17.1.1 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 diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index d47fee16665a..3b9232bf7ca6 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -5579,7 +5579,7 @@ void main() { return Scaffold( body: TextButton( onPressed: () async { - shellNavigatorKey.currentState!.push( + Navigator.of(context).push( MaterialPageRoute( builder: (BuildContext context) { return const Scaffold( diff --git a/packages/go_router/test/imperative_api_test.dart b/packages/go_router/test/imperative_api_test.dart index b56d0eefa565..ad4be89ab8d1 100644 --- a/packages/go_router/test/imperative_api_test.dart +++ b/packages/go_router/test/imperative_api_test.dart @@ -192,6 +192,73 @@ void main() { expect(find.byKey(b), findsOneWidget); }); + testWidgets( + 'push to a sibling shell route under the same parent shell route', + (WidgetTester tester) async { + const appleNavigatorKey = _CollidingNavigatorKey('apple'); + const googleNavigatorKey = _CollidingNavigatorKey('google'); + final routes = [ + ShellRoute( + builder: (_, __, Widget child) => + _ShellScaffold(label: 'project-shell', child: child), + routes: [ + ShellRoute( + navigatorKey: appleNavigatorKey, + builder: (_, __, Widget child) => + _ShellScaffold(label: 'apple-shell', child: child), + routes: [ + GoRoute( + path: '/apple', + builder: (_, __) => const _CounterPage(label: 'apple count'), + ), + ], + ), + ShellRoute( + navigatorKey: googleNavigatorKey, + builder: (_, __, Widget child) => + _ShellScaffold(label: 'google-shell', child: child), + routes: [ + GoRoute( + path: '/google', + builder: (_, __) => const Text('google page'), + ), + ], + ), + ], + ), + ]; + final GoRouter router = await createRouter( + routes, + tester, + initialLocation: '/apple', + ); + + expect(find.text('project-shell'), findsOneWidget); + expect(find.text('apple-shell'), findsOneWidget); + expect(find.text('apple count: 0'), findsOneWidget); + + await tester.tap(find.text('apple count: 0')); + await tester.pump(); + expect(find.text('apple count: 1'), findsOneWidget); + + router.push('/google'); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + expect(find.text('project-shell'), findsOneWidget); + expect(find.text('apple-shell'), findsNothing); + expect(find.text('google-shell'), findsOneWidget); + expect(find.text('google page'), findsOneWidget); + + router.pop(); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + expect(find.text('project-shell'), findsOneWidget); + expect(find.text('google-shell'), findsNothing); + expect(find.text('apple-shell'), findsOneWidget); + expect(find.text('apple count: 1'), findsOneWidget); + }, + ); + testWidgets('push inside or outside shell route', ( WidgetTester tester, ) async { @@ -317,3 +384,59 @@ void main() { expect(find.byKey(e), findsOneWidget); }); } + +class _ShellScaffold extends StatelessWidget { + const _ShellScaffold({required this.label, required this.child}); + + final String label; + final Widget child; + + @override + Widget build(BuildContext context) { + return Material( + child: Column( + children: [ + Text(label), + Expanded(child: child), + ], + ), + ); + } +} + +class _CounterPage extends StatefulWidget { + const _CounterPage({required this.label}); + + final String label; + + @override + State<_CounterPage> createState() => _CounterPageState(); +} + +class _CounterPageState extends State<_CounterPage> { + int _count = 0; + + @override + Widget build(BuildContext context) { + return Center( + child: TextButton( + onPressed: () { + setState(() { + _count++; + }); + }, + child: Text('${widget.label}: $_count'), + ), + ); + } +} + +class _CollidingNavigatorKey extends GlobalObjectKey { + const _CollidingNavigatorKey(super.value); + + @override + int get hashCode => 0; + + @override + bool operator ==(Object other) => identical(this, other); +} diff --git a/packages/go_router/test/match_test.dart b/packages/go_router/test/match_test.dart index 1b12d2e71d7c..1ac1cce0d9a7 100644 --- a/packages/go_router/test/match_test.dart +++ b/packages/go_router/test/match_test.dart @@ -43,27 +43,35 @@ void main() { expect(matches.first.pageKey, isNotNull); }); - test('ShellRoute Match has stable unique key', () { + test('ShellRoute Match has unique keys and preserves them on copy', () { final route = ShellRoute( builder: _shellBuilder, routes: [GoRoute(path: '/users/:userId', builder: _builder)], ); - final pathParameters = {}; final List matches1 = RouteMatchBase.match( route: route, - pathParameters: pathParameters, + pathParameters: {}, uri: Uri.parse('/users/123'), rootNavigatorKey: GlobalKey(), ); final List matches2 = RouteMatchBase.match( route: route, - pathParameters: pathParameters, + pathParameters: {}, uri: Uri.parse('/users/1234'), rootNavigatorKey: GlobalKey(), ); expect(matches1.length, 1); expect(matches2.length, 1); - expect(matches1.first.pageKey, matches2.first.pageKey); + final match1 = matches1.first as ShellRouteMatch; + final match2 = matches2.first as ShellRouteMatch; + expect(match1.pageKey, isNot(equals(match2.pageKey))); + expect(match1.navigatorKey, isNot(same(match2.navigatorKey))); + + final ShellRouteMatch copiedMatch = match1.copyWith( + matches: match1.matches, + ); + expect(copiedMatch.pageKey, match1.pageKey); + expect(copiedMatch.navigatorKey, same(match1.navigatorKey)); }); test('GoRoute Match has stable unique key', () { From b0df4550921715817b50761fb14b34d815d77783 Mon Sep 17 00:00:00 2001 From: Kravchenko Igor Date: Sat, 28 Feb 2026 23:26:26 +0100 Subject: [PATCH 2/2] [go_router] Update packages/go_router/test/imperative_api_test.dart --- .../go_router/test/imperative_api_test.dart | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/go_router/test/imperative_api_test.dart b/packages/go_router/test/imperative_api_test.dart index ad4be89ab8d1..c81906205329 100644 --- a/packages/go_router/test/imperative_api_test.dart +++ b/packages/go_router/test/imperative_api_test.dart @@ -195,32 +195,32 @@ void main() { testWidgets( 'push to a sibling shell route under the same parent shell route', (WidgetTester tester) async { - const appleNavigatorKey = _CollidingNavigatorKey('apple'); - const googleNavigatorKey = _CollidingNavigatorKey('google'); + const firstNavigatorKey = _CollidingNavigatorKey('first'); + const secondNavigatorKey = _CollidingNavigatorKey('second'); final routes = [ ShellRoute( builder: (_, __, Widget child) => _ShellScaffold(label: 'project-shell', child: child), routes: [ ShellRoute( - navigatorKey: appleNavigatorKey, + navigatorKey: firstNavigatorKey, builder: (_, __, Widget child) => - _ShellScaffold(label: 'apple-shell', child: child), + _ShellScaffold(label: 'first-shell', child: child), routes: [ GoRoute( - path: '/apple', - builder: (_, __) => const _CounterPage(label: 'apple count'), + path: '/first', + builder: (_, __) => const _CounterPage(label: 'first count'), ), ], ), ShellRoute( - navigatorKey: googleNavigatorKey, + navigatorKey: secondNavigatorKey, builder: (_, __, Widget child) => - _ShellScaffold(label: 'google-shell', child: child), + _ShellScaffold(label: 'second-shell', child: child), routes: [ GoRoute( - path: '/google', - builder: (_, __) => const Text('google page'), + path: '/second', + builder: (_, __) => const Text('second page'), ), ], ), @@ -230,32 +230,32 @@ void main() { final GoRouter router = await createRouter( routes, tester, - initialLocation: '/apple', + initialLocation: '/first', ); expect(find.text('project-shell'), findsOneWidget); - expect(find.text('apple-shell'), findsOneWidget); - expect(find.text('apple count: 0'), findsOneWidget); + expect(find.text('first-shell'), findsOneWidget); + expect(find.text('first count: 0'), findsOneWidget); - await tester.tap(find.text('apple count: 0')); + await tester.tap(find.text('first count: 0')); await tester.pump(); - expect(find.text('apple count: 1'), findsOneWidget); + expect(find.text('first count: 1'), findsOneWidget); - router.push('/google'); + router.push('/second'); await tester.pumpAndSettle(); expect(tester.takeException(), isNull); expect(find.text('project-shell'), findsOneWidget); - expect(find.text('apple-shell'), findsNothing); - expect(find.text('google-shell'), findsOneWidget); - expect(find.text('google page'), findsOneWidget); + expect(find.text('first-shell'), findsNothing); + expect(find.text('second-shell'), findsOneWidget); + expect(find.text('second page'), findsOneWidget); router.pop(); await tester.pumpAndSettle(); expect(tester.takeException(), isNull); expect(find.text('project-shell'), findsOneWidget); - expect(find.text('google-shell'), findsNothing); - expect(find.text('apple-shell'), findsOneWidget); - expect(find.text('apple count: 1'), findsOneWidget); + expect(find.text('second-shell'), findsNothing); + expect(find.text('first-shell'), findsOneWidget); + expect(find.text('first count: 1'), findsOneWidget); }, ); @@ -431,12 +431,15 @@ class _CounterPageState extends State<_CounterPage> { } } -class _CollidingNavigatorKey extends GlobalObjectKey { - const _CollidingNavigatorKey(super.value); +class _CollidingNavigatorKey extends GlobalKey { + const _CollidingNavigatorKey(this._label) : super.constructor(); + + final String _label; @override int get hashCode => 0; @override - bool operator ==(Object other) => identical(this, other); + bool operator ==(Object other) => + other is _CollidingNavigatorKey && other._label == _label; }