[go_router] Fixes the GoRouter.goBranch so that it doesn't reset extr… (

flutter#4723) â�¦a to null if extra is not serializable. This was a mistake I made when I refactor the code, the goBranch uses GoRouter.restore which will go through unnecessary serialize/deserialize. Fixing this will unfortunately introduce a breaking change, though I don't think a lot of people will be impacted by this. I will write a migration guide soon fixes flutter/flutter#129347
moneybird · Sep 21, 2023 · 55b7293 · 55b7293
1 parent 7eee49a
commit 55b7293
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 9 deletions.
diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
@@ -1,3 +1,10 @@
+## 11.0.0
+
+- Fixes the GoRouter.goBranch so that it doesn't reset extra to null if extra is not serializable.
+- **BREAKING CHANGE**:
+  - Updates the function signature of `GoRouteInformationProvider.restore`.
+  - Adds `NavigationType.restore` to `NavigationType` enum.
+
 ## 10.2.0
 
 - Adds `onExit` to GoRoute.

diff --git a/packages/go_router/README.md b/packages/go_router/README.md
@@ -37,6 +37,7 @@ See the API documentation for details on the following topics:
 - [Error handling](https://pub.dev/documentation/go_router/latest/topics/Error%20handling-topic.html)
 
 ## Migration Guides
+- [Migrating to 11.0.0](https://flutter.dev/go/go-router-v11-breaking-changes).
 - [Migrating to 10.0.0](https://flutter.dev/go/go-router-v10-breaking-changes).
 - [Migrating to 9.0.0](https://flutter.dev/go/go-router-v9-breaking-changes).
 - [Migrating to 8.0.0](https://flutter.dev/go/go-router-v8-breaking-changes).

diff --git a/packages/go_router/lib/src/information_provider.dart b/packages/go_router/lib/src/information_provider.dart
@@ -33,6 +33,10 @@ enum NavigatingType {
 
   /// Replace the entire [RouteMatchList] with the new location.
   go,
+
+  /// Restore the current match list with
+  /// [RouteInformationState.baseRouteMatchList].
+  restore,
 }
 
 /// The data class to be stored in [RouteInformation.state] to be used by
@@ -48,16 +52,18 @@ class RouteInformationState<T> {
     this.completer,
     this.baseRouteMatchList,
     required this.type,
-  }) : assert((type != NavigatingType.go) ==
-            (completer != null && baseRouteMatchList != null));
+  })  : assert((type == NavigatingType.go || type == NavigatingType.restore) ==
+            (completer == null)),
+        assert((type != NavigatingType.go) == (baseRouteMatchList != null));
 
   /// The extra object used when navigating with [GoRouter].
   final Object? extra;
 
   /// The completer that needs to be completed when the newly added route is
   /// popped off the screen.
   ///
-  /// This is only null if [type] is [NavigatingType.go].
+  /// This is only null if [type] is [NavigatingType.go] or
+  /// [NavigatingType.restore].
   final Completer<T?>? completer;
 
   /// The base route match list to push on top to.
@@ -177,11 +183,15 @@ class GoRouteInformationProvider extends RouteInformationProvider
     );
   }
 
-  /// Restores the current route matches with the `encodedMatchList`.
-  void restore(String location, {required Object encodedMatchList}) {
+  /// Restores the current route matches with the `matchList`.
+  void restore(String location, {required RouteMatchList matchList}) {
     _setValue(
-      location,
-      encodedMatchList,
+      matchList.uri.toString(),
+      RouteInformationState<void>(
+        extra: matchList.extra,
+        baseRouteMatchList: matchList,
+        type: NavigatingType.restore,
+      ),
     );
   }
 

diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart
@@ -193,6 +193,11 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
             );
       case NavigatingType.go:
         return newMatchList;
+      case NavigatingType.restore:
+        // Still need to consider redirection.
+        return baseRouteMatchList!.uri.toString() == newMatchList.uri.toString()
+            ? baseRouteMatchList
+            : newMatchList;
     }
   }
 

diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart
@@ -327,7 +327,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
     log.info('restoring ${matchList.uri}');
     routeInformationProvider.restore(
       matchList.uri.toString(),
-      encodedMatchList: RouteMatchListCodec(configuration).encode(matchList),
+      matchList: matchList,
     );
   }
 

diff --git 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: 10.2.0
+version: 11.0.0
 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
@@ -2843,6 +2843,52 @@ void main() {
       expect(matches.pathParameters['pid'], pid);
     });
 
+    testWidgets('StatefulShellRoute preserve extra when switching branch',
+        (WidgetTester tester) async {
+      StatefulNavigationShell? routeState;
+      Object? latestExtra;
+      final List<RouteBase> 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) {
+                      latestExtra = state.extra;
+                      return const DummyScreen();
+                    }),
+              ],
+            ),
+          ],
+        ),
+      ];
+      final Object expectedExtra = Object();
+
+      await createRouter(routes, tester,
+          initialLocation: '/b', initialExtra: expectedExtra);
+      expect(latestExtra, expectedExtra);
+      routeState!.goBranch(0);
+      await tester.pumpAndSettle();
+      routeState!.goBranch(1);
+      await tester.pumpAndSettle();
+      expect(latestExtra, expectedExtra);
+    });
+
     testWidgets('goNames should allow dynamics values for queryParams',
         (WidgetTester tester) async {
       const Map<String, dynamic> queryParametersAll = <String, List<dynamic>>{