From 6a03dd47ec86bc69bafae904a99777e8ea952724 Mon Sep 17 00:00:00 2001 From: David Miguel Lozano Date: Mon, 14 Nov 2022 18:55:56 +0100 Subject: [PATCH] [go_router] Fix parentNavigatorKey not working with multiple nested Routes in ShellRoute (#2784) * [go_router] Fix parentNavigatorKey with multiple nested Routes in ShellRoute (#111961) * Update packages/go_router/CHANGELOG.md Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com> Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com> --- packages/go_router/CHANGELOG.md | 4 + packages/go_router/lib/src/configuration.dart | 6 +- packages/go_router/pubspec.yaml | 2 +- .../go_router/test/configuration_test.dart | 177 +++++++++++------- 4 files changed, 121 insertions(+), 68 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 186fabb6a4..146368bdac 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 5.1.6 + +- Fixes crashes when multiple `GoRoute`s use the same `parentNavigatorKey` in a route subtree. + ## 5.1.5 - Adds migration guide for 5.1.2 to readme. diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index fa7261b470..8f97a7c317 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -60,9 +60,9 @@ class RouteConfiguration { checkParentNavigatorKeys( route.routes, >[ - // Once a parentNavigatorKey is used, only the navigator keys - // above it can be used. - ...allowedKeys.sublist(0, allowedKeys.indexOf(parentKey)), + // Once a parentNavigatorKey is used, only that navigator key + // or keys above it can be used. + ...allowedKeys.sublist(0, allowedKeys.indexOf(parentKey) + 1), ], ); } else { diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 3973a2a9ea..94df1c96dd 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: 5.1.5 +version: 5.1.6 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/configuration_test.dart b/packages/go_router/test/configuration_test.dart index 61f769f4c5..353653140f 100644 --- a/packages/go_router/test/configuration_test.dart +++ b/packages/go_router/test/configuration_test.dart @@ -175,6 +175,55 @@ void main() { }, ); + test( + 'Does not throw with multiple nested GoRoutes using parentNavigatorKey in ShellRoute', + () { + final GlobalKey root = + GlobalKey(debugLabel: 'root'); + final GlobalKey shell = + GlobalKey(debugLabel: 'shell'); + RouteConfiguration( + navigatorKey: root, + routes: [ + ShellRoute( + navigatorKey: shell, + routes: [ + GoRoute( + path: '/', + builder: _mockScreenBuilder, + routes: [ + GoRoute( + path: 'a', + builder: _mockScreenBuilder, + parentNavigatorKey: root, + routes: [ + GoRoute( + path: 'b', + builder: _mockScreenBuilder, + parentNavigatorKey: root, + routes: [ + GoRoute( + path: 'c', + builder: _mockScreenBuilder, + parentNavigatorKey: root, + ), + ], + ), + ], + ), + ], + ), + ], + ), + ], + redirectLimit: 10, + topRedirect: (BuildContext context, GoRouterState state) { + return null; + }, + ); + }, + ); + test( 'Throws when parentNavigatorKeys are overlapping', () { @@ -369,78 +418,50 @@ void main() { navigatorKey: root, ); }); - }); - test( - 'Does not throw with valid parentNavigatorKey configuration', - () { - final GlobalKey root = - GlobalKey(debugLabel: 'root'); - final GlobalKey shell = - GlobalKey(debugLabel: 'shell'); - final GlobalKey shell2 = - GlobalKey(debugLabel: 'shell2'); - RouteConfiguration( - navigatorKey: root, - routes: [ - ShellRoute( - navigatorKey: shell, - routes: [ - GoRoute( - path: '/', - builder: _mockScreenBuilder, - routes: [ - GoRoute( - path: 'a', - builder: _mockScreenBuilder, - parentNavigatorKey: root, - routes: [ - ShellRoute( - navigatorKey: shell2, - routes: [ - GoRoute( - path: 'b', - builder: _mockScreenBuilder, - routes: [ - GoRoute( - path: 'b', - builder: _mockScreenBuilder, - parentNavigatorKey: shell2, - ), - ], - ), - ], - ), - ], - ), - ], - ), - ], - ), - ], - redirectLimit: 10, - topRedirect: (BuildContext context, GoRouterState state) { - return null; - }, - ); - }, - ); - - test('throws when ShellRoute contains a GoRoute with a parentNavigatorKey', - () { - final GlobalKey root = - GlobalKey(debugLabel: 'root'); - expect( + test( + 'Does not throw with valid parentNavigatorKey configuration', () { + final GlobalKey root = + GlobalKey(debugLabel: 'root'); + final GlobalKey shell = + GlobalKey(debugLabel: 'shell'); + final GlobalKey shell2 = + GlobalKey(debugLabel: 'shell2'); RouteConfiguration( navigatorKey: root, routes: [ ShellRoute( + navigatorKey: shell, routes: [ GoRoute( - path: '/a', + path: '/', builder: _mockScreenBuilder, - parentNavigatorKey: root, + routes: [ + GoRoute( + path: 'a', + builder: _mockScreenBuilder, + parentNavigatorKey: root, + routes: [ + ShellRoute( + navigatorKey: shell2, + routes: [ + GoRoute( + path: 'b', + builder: _mockScreenBuilder, + routes: [ + GoRoute( + path: 'b', + builder: _mockScreenBuilder, + parentNavigatorKey: shell2, + ), + ], + ), + ], + ), + ], + ), + ], ), ], ), @@ -451,8 +472,36 @@ void main() { }, ); }, - throwsAssertionError, ); + + test('throws when ShellRoute contains a GoRoute with a parentNavigatorKey', + () { + final GlobalKey root = + GlobalKey(debugLabel: 'root'); + expect( + () { + RouteConfiguration( + navigatorKey: root, + routes: [ + ShellRoute( + routes: [ + GoRoute( + path: '/a', + builder: _mockScreenBuilder, + parentNavigatorKey: root, + ), + ], + ), + ], + redirectLimit: 10, + topRedirect: (BuildContext context, GoRouterState state) { + return null; + }, + ); + }, + throwsAssertionError, + ); + }); }); }