[go_router] Relax subroute path requirements (allow root and child ro…

…utes to have the same path forms ) (#7647)

Before this change `GoRoute` constructor allowed Schroedinger routes which were neither in a valid state nor an invalid state until they were used. EG: `GoRoute(path: '/some-route')`  was valid as a top route but invalid as a child route. This prevents routes from being moved around and prevents building upon go router. 

To solve this I see two solution:

  1. Breaking change: Require all routes to start with `/` (or none)
  2. Non breaking: Allow all routes to start or not with `/`

Since I did not want to introduce a breaking change I followed option 2, which allows all routes to be of the form `/some-route` or `some-route`. However, breaking changes aside, I believe it would be better to use solution 1 as it's more predictible.

@chunhtai What do you think about those options ? 

Related issue: Fix flutter/flutter#145867

packages/go_router/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 14.2.9
+
+- Relaxes route path requirements. Both root and child routes can now start with or without '/'.
+
 ## 14.2.8
 
 - Updated custom_stateful_shell_route example to better support swiping in TabView as well as demonstration of the use of PageView. 

packages/go_router/lib/src/configuration.dart

@@ -36,12 +36,9 @@ class RouteConfiguration {
     for (final RouteBase route in routes) {
       late bool subRouteIsTopLevel;
       if (route is GoRoute) {
-        if (isTopLevel) {
-          assert(route.path.startsWith('/'),
-              'top-level path must start with "/": $route');
-        } else {
-          assert(!route.path.startsWith('/') && !route.path.endsWith('/'),
-              'sub-route path may not start or end with "/": $route');
+        if (route.path != '/') {
+          assert(!route.path.endsWith('/'),
+              'route path may not end with "/" except for the top "/" route. Found: $route');
         }
         subRouteIsTopLevel = false;
       } else if (route is ShellRouteBase) {

packages/go_router/lib/src/match.dart

@@ -203,6 +203,7 @@ abstract class RouteMatchBase with Diagnosticable {
 
     final RegExpMatch? regExpMatch =
         route.matchPatternAsPrefix(remainingLocation);
+
     if (regExpMatch == null) {
       return _empty;
     }
@@ -555,13 +556,9 @@ class RouteMatchList with Diagnosticable {
   /// [RouteMatchA(), RouteMatchB(), RouteMatchC()]
   /// ```
   static String _generateFullPath(Iterable<RouteMatchBase> matches) {
-    final StringBuffer buffer = StringBuffer();
-    bool addsSlash = false;
+    String fullPath = '';
     for (final RouteMatchBase match in matches
         .where((RouteMatchBase match) => match is! ImperativeRouteMatch)) {
-      if (addsSlash) {
-        buffer.write('/');
-      }
       final String pathSegment;
       if (match is RouteMatch) {
         pathSegment = match.route.path;
@@ -571,10 +568,9 @@ class RouteMatchList with Diagnosticable {
         assert(false, 'Unexpected match type: $match');
         continue;
       }
-      buffer.write(pathSegment);
-      addsSlash = pathSegment.isNotEmpty && (addsSlash || pathSegment != '/');
+      fullPath = concatenatePaths(fullPath, pathSegment);
     }
-    return buffer.toString();
+    return fullPath;
   }
 
   /// Returns true if there are no matches.

packages/go_router/lib/src/path_utils.dart

@@ -102,20 +102,14 @@ Map<String, String> extractPathParameters(
 
 /// Concatenates two paths.
 ///
-/// e.g: pathA = /a, pathB = c/d,  concatenatePaths(pathA, pathB) = /a/c/d.
+/// e.g: pathA = /a, pathB = /c/d, concatenatePaths(pathA, pathB) = /a/c/d.
+/// or: pathA = a, pathB = c/d, concatenatePaths(pathA, pathB) = /a/c/d.
 String concatenatePaths(String parentPath, String childPath) {
-  // at the root, just return the path
-  if (parentPath.isEmpty) {
-    assert(childPath.startsWith('/'));
-    assert(childPath == '/' || !childPath.endsWith('/'));
-    return childPath;
-  }
-
-  // not at the root, so append the parent path
-  assert(childPath.isNotEmpty);
-  assert(!childPath.startsWith('/'));
-  assert(!childPath.endsWith('/'));
-  return '${parentPath == '/' ? '' : parentPath}/$childPath';
+  final Iterable<String> segments = <String>[
+    ...parentPath.split('/'),
+    ...childPath.split('/')
+  ].where((String segment) => segment.isNotEmpty);
+  return '/${segments.join('/')}';
 }
 
 /// Builds an absolute path for the provided route.

packages/go_router/lib/src/route.dart

@@ -432,8 +432,10 @@ class GoRoute extends RouteBase {
 
   // TODO(chunhtai): move all regex related help methods to path_utils.dart.
   /// Match this route against a location.
-  RegExpMatch? matchPatternAsPrefix(String loc) =>
-      _pathRE.matchAsPrefix(loc) as RegExpMatch?;
+  RegExpMatch? matchPatternAsPrefix(String loc) {
+    return _pathRE.matchAsPrefix('/$loc') as RegExpMatch? ??
+        _pathRE.matchAsPrefix(loc) as RegExpMatch?;
+  }
 
   /// Extract the path parameters from a match.
   Map<String, String> extractPathParams(RegExpMatch match) =>

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: 14.2.8
+version: 14.2.9
 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
 

packages/go_router/test/go_router_test.dart

@@ -96,25 +96,6 @@ void main() {
       }, throwsA(isAssertionError));
     });
 
-    test('leading / on sub-route', () {
-      expect(() {
-        GoRouter(
-          routes: <RouteBase>[
-            GoRoute(
-              path: '/',
-              builder: dummy,
-              routes: <GoRoute>[
-                GoRoute(
-                  path: '/foo',
-                  builder: dummy,
-                ),
-              ],
-            ),
-          ],
-        );
-      }, throwsA(isAssertionError));
-    });
-
     test('trailing / on sub-route', () {
       expect(() {
         GoRouter(
@@ -134,16 +115,6 @@ void main() {
       }, throwsA(isAssertionError));
     });
 
-    testWidgets('lack of leading / on top-level route',
-        (WidgetTester tester) async {
-      await expectLater(() async {
-        final List<GoRoute> routes = <GoRoute>[
-          GoRoute(path: 'foo', builder: dummy),
-        ];
-        await createRouter(routes, tester);
-      }, throwsA(isAssertionError));
-    });
-
     testWidgets('match no routes', (WidgetTester tester) async {
       final List<GoRoute> routes = <GoRoute>[
         GoRoute(path: '/', builder: dummy),
@@ -5502,6 +5473,94 @@ void main() {
       ),
     );
   });
+
+  testWidgets('should allow route paths without leading /',
+      (WidgetTester tester) async {
+    final List<GoRoute> routes = <GoRoute>[
+      GoRoute(
+        path: '/', // root cannot be empty (existing assert)
+        builder: (BuildContext context, GoRouterState state) =>
+            const HomeScreen(),
+        routes: <RouteBase>[
+          GoRoute(
+            path: 'child-route',
+            builder: (BuildContext context, GoRouterState state) =>
+                const Text('/child-route'),
+            routes: <RouteBase>[
+              GoRoute(
+                path: 'grand-child-route',
+                builder: (BuildContext context, GoRouterState state) =>
+                    const Text('/grand-child-route'),
+              ),
+              GoRoute(
+                path: 'redirected-grand-child-route',
+                redirect: (BuildContext context, GoRouterState state) =>
+                    '/child-route',
+              ),
+            ],
+          )
+        ],
+      ),
+    ];
+
+    final GoRouter router = await createRouter(routes, tester,
+        initialLocation: '/child-route/grand-child-route');
+    RouteMatchList matches = router.routerDelegate.currentConfiguration;
+    expect(matches.matches, hasLength(3));
+    expect(matches.uri.toString(), '/child-route/grand-child-route');
+    expect(find.text('/grand-child-route'), findsOneWidget);
+
+    router.go('/child-route/redirected-grand-child-route');
+    await tester.pumpAndSettle();
+    matches = router.routerDelegate.currentConfiguration;
+    expect(matches.matches, hasLength(2));
+    expect(matches.uri.toString(), '/child-route');
+    expect(find.text('/child-route'), findsOneWidget);
+  });
+
+  testWidgets('should allow route paths with leading /',
+      (WidgetTester tester) async {
+    final List<GoRoute> routes = <GoRoute>[
+      GoRoute(
+        path: '/',
+        builder: (BuildContext context, GoRouterState state) =>
+            const HomeScreen(),
+        routes: <RouteBase>[
+          GoRoute(
+            path: '/child-route',
+            builder: (BuildContext context, GoRouterState state) =>
+                const Text('/child-route'),
+            routes: <RouteBase>[
+              GoRoute(
+                path: '/grand-child-route',
+                builder: (BuildContext context, GoRouterState state) =>
+                    const Text('/grand-child-route'),
+              ),
+              GoRoute(
+                path: '/redirected-grand-child-route',
+                redirect: (BuildContext context, GoRouterState state) =>
+                    '/child-route',
+              ),
+            ],
+          )
+        ],
+      ),
+    ];
+
+    final GoRouter router = await createRouter(routes, tester,
+        initialLocation: '/child-route/grand-child-route');
+    RouteMatchList matches = router.routerDelegate.currentConfiguration;
+    expect(matches.matches, hasLength(3));
+    expect(matches.uri.toString(), '/child-route/grand-child-route');
+    expect(find.text('/grand-child-route'), findsOneWidget);
+
+    router.go('/child-route/redirected-grand-child-route');
+    await tester.pumpAndSettle();
+    matches = router.routerDelegate.currentConfiguration;
+    expect(matches.matches, hasLength(2));
+    expect(matches.uri.toString(), '/child-route');
+    expect(find.text('/child-route'), findsOneWidget);
+  });
 }
 
 class TestInheritedNotifier extends InheritedNotifier<ValueNotifier<String>> {

packages/go_router/test/path_utils_test.dart

@@ -79,17 +79,11 @@ void main() {
       expect(result, expected);
     }
 
-    void verifyThrows(String pathA, String pathB) {
-      expect(
-          () => concatenatePaths(pathA, pathB), throwsA(isA<AssertionError>()));
-    }
-
     verify('/a', 'b/c', '/a/b/c');
     verify('/', 'b', '/b');
-    verifyThrows('/a', '/b');
-    verifyThrows('/a', '/');
-    verifyThrows('/', '/');
-    verifyThrows('/', '');
-    verifyThrows('', '');
+    verify('/a', '/b/c/', '/a/b/c');
+    verify('/a', 'b/c', '/a/b/c');
+    verify('/', '/', '/');
+    verify('', '', '/');
   });
 }