From 49f39ad5511e9edcb805c9b18832c0fe32074062 Mon Sep 17 00:00:00 2001 From: Hrishikesh Kadam Date: Wed, 11 Oct 2023 21:29:59 +0530 Subject: [PATCH 1/3] [go_router] Fixes deep-link with no path on cold start Fixes flutter/flutter#133928 --- packages/go_router/CHANGELOG.md | 4 ++++ packages/go_router/lib/src/configuration.dart | 4 ++-- packages/go_router/pubspec.yaml | 2 +- packages/go_router/test/matching_test.dart | 24 +++++++++++++++++++ packages/go_router/test/path_utils_test.dart | 2 ++ 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 3baca0cbbb2c..5f54cec55197 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 11.1.4 + +- Fixes deep-link with no path on cold start. + ## 11.1.3 - Fixes missing state.extra in onException(). diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index 24c16e342b8a..a2b44b1654fa 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -297,8 +297,8 @@ class RouteConfiguration { List? _getLocRouteMatches( Uri uri, Map pathParameters) { final List? result = _getLocRouteRecursively( - location: uri.path, - remainingLocation: uri.path, + location: uri.path.isEmpty ? '/' : uri.path, + remainingLocation: uri.path.isEmpty ? '/' : uri.path, matchedLocation: '', pathParameters: pathParameters, routes: routes, diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 9fb123c99ffd..3b3da6c85f0e 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: 11.1.3 +version: 11.1.4 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/matching_test.dart b/packages/go_router/test/matching_test.dart index b6337f47c810..3d7ef1d54882 100644 --- a/packages/go_router/test/matching_test.dart +++ b/packages/go_router/test/matching_test.dart @@ -105,4 +105,28 @@ void main() { expect(decoded, isNotNull); expect(decoded, equals(list1)); }); + + test('RouteMatchList for deep-link Uri with scheme, authority', () { + final RouteConfiguration configuration = RouteConfiguration( + routes: [ + GoRoute( + path: '/', + builder: (BuildContext context, GoRouterState state) => + const Placeholder(), + ), + ], + redirectLimit: 0, + navigatorKey: GlobalKey(), + topRedirect: (_, __) => null, + ); + + final RouteMatchList matchList1 = configuration.findMatch('/'); + expect(matchList1.matches[0].matchedLocation, '/'); + final RouteMatchList matchList2 = + configuration.findMatch('https://domain.com'); + expect(matchList2.matches[0].matchedLocation, '/'); + final RouteMatchList matchList3 = + configuration.findMatch('https://domain.com/'); + expect(matchList3.matches[0].matchedLocation, '/'); + }); } diff --git a/packages/go_router/test/path_utils_test.dart b/packages/go_router/test/path_utils_test.dart index 1c883c451559..b2446405e3e3 100644 --- a/packages/go_router/test/path_utils_test.dart +++ b/packages/go_router/test/path_utils_test.dart @@ -100,6 +100,8 @@ void main() { verify('/a/', '/a'); verify('/', '/'); verify('/a/b/', '/a/b'); + verify('scheme://authority', 'scheme://authority'); + verify('scheme://authority/', 'scheme://authority'); expect(() => canonicalUri('::::'), throwsA(isA())); expect(() => canonicalUri(''), throwsA(anything)); From 6cc2fc4f2b7a4ed0801739b4b6fbde77757c96df Mon Sep 17 00:00:00 2001 From: Hrishikesh Kadam Date: Thu, 26 Oct 2023 00:12:57 +0530 Subject: [PATCH 2/3] [go_router] Resolves https://github.com/flutter/packages/pull/5113#discussion_r1369297013 --- packages/go_router/lib/src/configuration.dart | 4 +- packages/go_router/lib/src/router.dart | 4 +- packages/go_router/test/go_router_test.dart | 43 ++++++++++++++++++- packages/go_router/test/matching_test.dart | 24 ----------- packages/go_router/test/path_utils_test.dart | 2 - 5 files changed, 46 insertions(+), 31 deletions(-) diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index ea1ff6d5ac53..8e6393503a5b 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -315,8 +315,8 @@ class RouteConfiguration { List? _getLocRouteMatches( Uri uri, Map pathParameters) { final List? result = _getLocRouteRecursively( - location: uri.path.isEmpty ? '/' : uri.path, - remainingLocation: uri.path.isEmpty ? '/' : uri.path, + location: uri.path, + remainingLocation: uri.path, matchedLocation: '', matchedPath: '', pathParameters: pathParameters, diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 2e90474be89b..f7f8e2b0f3aa 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -526,8 +526,10 @@ class GoRouter implements RouterConfig { // verified by assert() during the initialization. return initialLocation!; } + final Uri platformDefaultUri = + Uri.parse(WidgetsBinding.instance.platformDispatcher.defaultRouteName); final String platformDefault = - WidgetsBinding.instance.platformDispatcher.defaultRouteName; + platformDefaultUri.path.isEmpty ? '/' : platformDefaultUri.path; if (initialLocation == null) { return platformDefault; } else if (platformDefault == '/') { diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index e486129b201f..d2b3e91a0025 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -2445,8 +2445,8 @@ void main() { // https://github.com/flutter/flutter/issues/124045. // ignore: deprecated_member_use expect(router.routeInformationProvider.value.location, '/dummy'); - TestWidgetsFlutterBinding - .instance.platformDispatcher.defaultRouteNameTestValue = '/'; + TestWidgetsFlutterBinding.instance.platformDispatcher + .clearDefaultRouteNameTestValue(); }); test('throws assertion if initialExtra is set w/o initialLocation', () { @@ -2466,6 +2466,45 @@ void main() { }); }); + group('_effectiveInitialLocation()', () { + final List routes = [ + GoRoute( + path: '/', + builder: (BuildContext context, GoRouterState state) => + const HomeScreen(), + ), + ]; + + testWidgets( + 'When platformDispatcher.defaultRouteName is deep-link Uri with ' + 'scheme, authority, no path', (WidgetTester tester) async { + TestWidgetsFlutterBinding.instance.platformDispatcher + .defaultRouteNameTestValue = 'https://domain.com'; + final GoRouter router = await createRouter( + routes, + tester, + ); + expect(router.routeInformationProvider.value.uri.path, '/'); + TestWidgetsFlutterBinding.instance.platformDispatcher + .clearDefaultRouteNameTestValue(); + }); + + testWidgets( + 'When platformDispatcher.defaultRouteName is deep-link Uri with ' + 'scheme, authority, no path, but trailing slash', + (WidgetTester tester) async { + TestWidgetsFlutterBinding.instance.platformDispatcher + .defaultRouteNameTestValue = 'https://domain.com/'; + final GoRouter router = await createRouter( + routes, + tester, + ); + expect(router.routeInformationProvider.value.uri.path, '/'); + TestWidgetsFlutterBinding.instance.platformDispatcher + .clearDefaultRouteNameTestValue(); + }); + }); + group('params', () { testWidgets('preserve path param case', (WidgetTester tester) async { final List routes = [ diff --git a/packages/go_router/test/matching_test.dart b/packages/go_router/test/matching_test.dart index 5aa746aa0abf..c53ca13bf722 100644 --- a/packages/go_router/test/matching_test.dart +++ b/packages/go_router/test/matching_test.dart @@ -107,28 +107,4 @@ void main() { expect(decoded, isNotNull); expect(decoded, equals(list1)); }); - - test('RouteMatchList for deep-link Uri with scheme, authority', () { - final RouteConfiguration configuration = RouteConfiguration( - routes: [ - GoRoute( - path: '/', - builder: (BuildContext context, GoRouterState state) => - const Placeholder(), - ), - ], - redirectLimit: 0, - navigatorKey: GlobalKey(), - topRedirect: (_, __) => null, - ); - - final RouteMatchList matchList1 = configuration.findMatch('/'); - expect(matchList1.matches[0].matchedLocation, '/'); - final RouteMatchList matchList2 = - configuration.findMatch('https://domain.com'); - expect(matchList2.matches[0].matchedLocation, '/'); - final RouteMatchList matchList3 = - configuration.findMatch('https://domain.com/'); - expect(matchList3.matches[0].matchedLocation, '/'); - }); } diff --git a/packages/go_router/test/path_utils_test.dart b/packages/go_router/test/path_utils_test.dart index b2446405e3e3..1c883c451559 100644 --- a/packages/go_router/test/path_utils_test.dart +++ b/packages/go_router/test/path_utils_test.dart @@ -100,8 +100,6 @@ void main() { verify('/a/', '/a'); verify('/', '/'); verify('/a/b/', '/a/b'); - verify('scheme://authority', 'scheme://authority'); - verify('scheme://authority/', 'scheme://authority'); expect(() => canonicalUri('::::'), throwsA(isA())); expect(() => canonicalUri(''), throwsA(anything)); From e32b56e9fe0c3fe6cbd5a1cc8cf5f614d3e5fc67 Mon Sep 17 00:00:00 2001 From: Hrishikesh Kadam Date: Sat, 28 Oct 2023 00:05:59 +0530 Subject: [PATCH 3/3] [go_router] Resolves https://github.com/flutter/packages/pull/5113#discussion_r1374861070 --- packages/go_router/lib/src/parser.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index 27485f088370..dc70baa16b2b 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -83,6 +83,9 @@ class GoRouteInformationParser extends RouteInformationParser { initialMatches = // TODO(chunhtai): remove this ignore and migrate the code // https://github.com/flutter/flutter/issues/124045. + // TODO(chunhtai): After the migration from routeInformation's location + // to uri, empty path check might be required here; see + // https://github.com/flutter/packages/pull/5113#discussion_r1374861070 // ignore: deprecated_member_use, unnecessary_non_null_assertion configuration.findMatch(routeInformation.location!, extra: state.extra); if (initialMatches.isError) {