From eff98e6d030e7ca31e62fd07ad1f30f75d964854 Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 22:35:24 +0200 Subject: [PATCH 01/20] [Feature]: implemented helpers for ShellRoute --- packages/go_router/lib/src/route_data.dart | 149 +++++++++++++++++++-- 1 file changed, 135 insertions(+), 14 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index f3a7f32a2eec..22b1160bd92f 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -11,12 +11,29 @@ import 'package:meta/meta_meta.dart'; import 'route.dart'; import 'state.dart'; +/// {@template route_data} +/// A superclass for each route data +/// {@endtemplate} +abstract class RouteData { + /// {@macro route_data} + const RouteData(); + + /// [navigatorKey] is used to point to a certain navigator + /// or pass it into the shell route + /// + /// In case of [ShellRoute] it will instantiate a new navigator + /// with the given key + /// + /// In case of [GoRoute] it will use the given key to find the navigator + GlobalKey? get navigatorKey => null; +} + /// Baseclass for supporting /// [typed routing](https://gorouter.dev/typed-routing). /// /// Subclasses must override one of [build], [buildPageWithState], or /// [redirect]. -abstract class GoRouteData { +abstract class GoRouteData extends RouteData { /// Allows subclasses to have `const` constructors. /// /// [GoRouteData] is abstract and cannot be instantiated directly. @@ -90,7 +107,8 @@ abstract class GoRouteData { static GoRoute $route({ required String path, required T Function(GoRouterState) factory, - List routes = const [], + GlobalKey? parentNavigatorKey, + List routes = const [], }) { T factoryImpl(GoRouterState state) { final Object? extra = state.extra; @@ -119,6 +137,7 @@ abstract class GoRouteData { pageBuilder: pageBuilder, redirect: redirect, routes: routes, + parentNavigatorKey: parentNavigatorKey, ); } @@ -129,24 +148,126 @@ abstract class GoRouteData { ); } -/// Annotation for types that support typed routing. +/// {@template shell_route_data} +/// Baseclass for supporting +/// [nested navigation](https://pub.dev/packages/go_router#nested-navigation) +/// {@endtemplate} +abstract class ShellRouteData extends RouteData { + /// {@macro shell_route_data} + const ShellRouteData(); + + /// [pageBuilder] is used to build the page + Page pageBuilder( + BuildContext context, + GoRouterState state, + Widget navigator, + ) => + const NoOpPage(); + + /// [pageBuilder] is used to build the page + Widget builder( + BuildContext context, + GoRouterState state, + Widget navigator, + ) => + throw UnimplementedError( + 'One of `builder` or `pageBuilder` must be implemented.', + ); + + /// A helper function used by generated code. + /// + /// Should not be used directly. + static ShellRoute $route({ + required String path, + required T Function(GoRouterState) factory, + GlobalKey? navigatorKey, + List routes = const [], + }) { + T factoryImpl(GoRouterState state) { + final Object? extra = state.extra; + + // If the "extra" value is of type `T` then we know it's the source + // instance of `GoRouteData`, so it doesn't need to be recreated. + if (extra is T) { + return extra; + } + + return (_stateObjectExpando[state] ??= factory(state)) as T; + } + + Widget builder( + BuildContext context, + GoRouterState state, + Widget navigator, + ) => + factoryImpl(state).builder( + context, + state, + navigator, + ); + + Page pageBuilder( + BuildContext context, + GoRouterState state, + Widget navigator, + ) => + factoryImpl(state).pageBuilder( + context, + state, + navigator, + ); + + return ShellRoute( + builder: builder, + pageBuilder: pageBuilder, + routes: routes, + navigatorKey: navigatorKey, + ); + } + + /// Used to cache [ShellRouteData] that corresponds to a given [GoRouterState] + /// to minimize the number of times it has to be deserialized. + static final Expando _stateObjectExpando = + Expando( + 'GoRouteState to ShellRouteData expando', + ); +} + +/// {@template typed_route} +/// A superclass for each typed route descendant +/// {@endtemplate} @Target({TargetKind.library, TargetKind.classType}) -class TypedGoRoute { - /// Instantiates a new instance of [TypedGoRoute]. - const TypedGoRoute({ - required this.path, - this.routes = const >[], +class TypedRoute { + /// {@macro typed_route} + const TypedRoute._({ + this.routes = const >[], + this.path, }); - /// The path that corresponds to this rout. - /// - /// See [GoRoute.path]. - final String path; + /// Instantiate a [TypedRoute] with a [path] and [routes]. + factory TypedRoute.go({ + required String path, + List> routes = const >[], + }) => + TypedRoute._(routes: routes, path: path); + + /// Instantiate a [TypedRoute] with [routes]. + factory TypedRoute.shell({ + List> routes = const >[], + }) => + TypedRoute._(routes: routes); /// Child route definitions. /// - /// See [GoRoute.routes]. - final List> routes; + /// See [RouteBase.routes]. + final List> routes; + + /// The path that corresponds to this route. + /// + /// See [GoRoute.path]. + /// + /// + final String? path; } /// Internal class used to signal that the default page behavior should be used. From 8302cbb16c6026c8c5ea0ab45a96034e42c0740d Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 23:12:21 +0200 Subject: [PATCH 02/20] [bugfix]: change export --- packages/go_router/lib/go_router.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/lib/go_router.dart b/packages/go_router/lib/go_router.dart index 09bfda977b04..4f6d2822b5b9 100644 --- a/packages/go_router/lib/go_router.dart +++ b/packages/go_router/lib/go_router.dart @@ -11,6 +11,6 @@ export 'src/configuration.dart' export 'src/misc/extensions.dart'; export 'src/misc/inherited_router.dart'; export 'src/pages/custom_transition_page.dart'; -export 'src/route_data.dart' show GoRouteData, TypedGoRoute; +export 'src/route_data.dart' show GoRouteData, TypedRoute; export 'src/router.dart'; export 'src/typedefs.dart' show GoRouterPageBuilder, GoRouterRedirect; From f1ad34786536022ff1b2edac6a55f347eeba68e2 Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 23:16:45 +0200 Subject: [PATCH 03/20] Removed named private constructor for TypedRoute --- packages/go_router/lib/src/route_data.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index 22b1160bd92f..10d191f21b0a 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -239,7 +239,7 @@ abstract class ShellRouteData extends RouteData { @Target({TargetKind.library, TargetKind.classType}) class TypedRoute { /// {@macro typed_route} - const TypedRoute._({ + const TypedRoute({ this.routes = const >[], this.path, }); @@ -249,13 +249,13 @@ class TypedRoute { required String path, List> routes = const >[], }) => - TypedRoute._(routes: routes, path: path); + TypedRoute(routes: routes, path: path); /// Instantiate a [TypedRoute] with [routes]. factory TypedRoute.shell({ List> routes = const >[], }) => - TypedRoute._(routes: routes); + TypedRoute(routes: routes); /// Child route definitions. /// From beeab6225cbf49e7c38ff6d53c17b470e5a0511f Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 23:20:58 +0200 Subject: [PATCH 04/20] Update CHANGELOG.md --- packages/go_router/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 4d326cd2dab8..d5686e4a6ab1 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 5.1.2 +- Adds helpers for go_router_builder for ShellRoute support - Update README - Removes dynamic calls in examples. From b432c307f77afbd1b75bd7297cffe83ace2f77eb Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 23:21:19 +0200 Subject: [PATCH 05/20] Update pubspec.yaml --- packages/go_router/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index f2c47050bf40..c3151361c9f3 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.1 +version: 5.1.2 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 From efe47d08a3719e14a895955fb72303c101ed7b71 Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 23:38:12 +0200 Subject: [PATCH 06/20] [Feature]: split routes per type --- packages/go_router/lib/src/route_data.dart | 38 +++++++++++++--------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index 10d191f21b0a..54da5448fbed 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -236,38 +236,44 @@ abstract class ShellRouteData extends RouteData { /// {@template typed_route} /// A superclass for each typed route descendant /// {@endtemplate} -@Target({TargetKind.library, TargetKind.classType}) class TypedRoute { /// {@macro typed_route} const TypedRoute({ this.routes = const >[], - this.path, }); - /// Instantiate a [TypedRoute] with a [path] and [routes]. - factory TypedRoute.go({ - required String path, - List> routes = const >[], - }) => - TypedRoute(routes: routes, path: path); - - /// Instantiate a [TypedRoute] with [routes]. - factory TypedRoute.shell({ - List> routes = const >[], - }) => - TypedRoute(routes: routes); - /// Child route definitions. /// /// See [RouteBase.routes]. final List> routes; +} + +/// {@template typed_go_route} +/// A superclass for each typed go route descendant +/// {@endtemplate} +@Target({TargetKind.library, TargetKind.classType}) +class TypedGoRoute extends TypedRoute { + /// {@macro typed_go_route} + const TypedGoRoute({ + required this.path, + super.routes, + }); /// The path that corresponds to this route. /// /// See [GoRoute.path]. /// /// - final String? path; + final String path; +} + +/// {@template typed_shell_route} +/// A superclass for each typed shell route descendant +/// {@endtemplate} +@Target({TargetKind.library, TargetKind.classType}) +class TypedShellRoute extends TypedRoute { + /// {@macro typed_shell_route} + const TypedShellRoute({super.routes}); } /// Internal class used to signal that the default page behavior should be used. From 751cd2025bf81cc4e751a06ccc383a48ce872b75 Mon Sep 17 00:00:00 2001 From: Mihail Date: Sun, 23 Oct 2022 23:41:26 +0200 Subject: [PATCH 07/20] [Feature]: add new exports --- packages/go_router/lib/go_router.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/lib/go_router.dart b/packages/go_router/lib/go_router.dart index 4f6d2822b5b9..72fe6ea73b25 100644 --- a/packages/go_router/lib/go_router.dart +++ b/packages/go_router/lib/go_router.dart @@ -11,6 +11,6 @@ export 'src/configuration.dart' export 'src/misc/extensions.dart'; export 'src/misc/inherited_router.dart'; export 'src/pages/custom_transition_page.dart'; -export 'src/route_data.dart' show GoRouteData, TypedRoute; +export 'src/route_data.dart' show GoRouteData, TypedGoRoute, TypedShellRoute; export 'src/router.dart'; export 'src/typedefs.dart' show GoRouterPageBuilder, GoRouterRedirect; From b5eac46d075fce951f807fbfaab23a5feeda3e47 Mon Sep 17 00:00:00 2001 From: Mihail Date: Thu, 27 Oct 2022 22:21:50 +0200 Subject: [PATCH 08/20] Tests, refactor routes --- packages/go_router/lib/src/route_data.dart | 25 ++- packages/go_router/test/route_data_test.dart | 193 ++++++++++++++----- 2 files changed, 155 insertions(+), 63 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index 54da5448fbed..bfcd526d29c6 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -238,14 +238,7 @@ abstract class ShellRouteData extends RouteData { /// {@endtemplate} class TypedRoute { /// {@macro typed_route} - const TypedRoute({ - this.routes = const >[], - }); - - /// Child route definitions. - /// - /// See [RouteBase.routes]. - final List> routes; + const TypedRoute(); } /// {@template typed_go_route} @@ -256,7 +249,7 @@ class TypedGoRoute extends TypedRoute { /// {@macro typed_go_route} const TypedGoRoute({ required this.path, - super.routes, + this.routes = const >[], }); /// The path that corresponds to this route. @@ -265,6 +258,11 @@ class TypedGoRoute extends TypedRoute { /// /// final String path; + + /// Child route definitions. + /// + /// See [RouteBase.routes]. + final List> routes; } /// {@template typed_shell_route} @@ -273,7 +271,14 @@ class TypedGoRoute extends TypedRoute { @Target({TargetKind.library, TargetKind.classType}) class TypedShellRoute extends TypedRoute { /// {@macro typed_shell_route} - const TypedShellRoute({super.routes}); + const TypedShellRoute({ + this.routes = const >[], + }); + + /// Child route definitions. + /// + /// See [RouteBase.routes]. + final List> routes; } /// Internal class used to signal that the default page behavior should be used. diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart index 4ae12f6dfb10..7e4a408a2f7e 100644 --- a/packages/go_router/test/route_data_test.dart +++ b/packages/go_router/test/route_data_test.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; +import 'package:go_router/src/route_data.dart'; class _GoRouteDataBuild extends GoRouteData { const _GoRouteDataBuild(); @@ -12,11 +13,32 @@ class _GoRouteDataBuild extends GoRouteData { Widget build(BuildContext context) => const SizedBox(key: Key('build')); } +class _ShellRouteDataBuilder extends ShellRouteData { + const _ShellRouteDataBuilder(); + + @override + Widget builder( + BuildContext context, + GoRouterState state, + Widget navigator, + ) => + SizedBox( + key: const Key('builder'), + child: navigator, + ); +} + final GoRoute _goRouteDataBuild = GoRouteData.$route( path: '/build', factory: (GoRouterState state) => const _GoRouteDataBuild(), ); +final ShellRoute _shellRouteDataBuilder = ShellRouteData.$route( + path: '/builder', + factory: (GoRouterState state) => const _ShellRouteDataBuilder(), + routes: _routes, +); + class _GoRouteDataBuildPage extends GoRouteData { const _GoRouteDataBuildPage(); @override @@ -25,11 +47,34 @@ class _GoRouteDataBuildPage extends GoRouteData { ); } +class _ShellRouteDataPageBuilder extends ShellRouteData { + const _ShellRouteDataPageBuilder(); + + @override + Page pageBuilder( + BuildContext context, + GoRouterState state, + Widget navigator, + ) => + MaterialPage( + child: SizedBox( + key: const Key('page-builder'), + child: navigator, + ), + ); +} + final GoRoute _goRouteDataBuildPage = GoRouteData.$route( path: '/build-page', factory: (GoRouterState state) => const _GoRouteDataBuildPage(), ); +final ShellRoute _shellRouteDataPageBuilder = ShellRouteData.$route( + path: '/page-builder', + factory: (GoRouterState state) => const _ShellRouteDataPageBuilder(), + routes: _routes, +); + class _GoRouteDataBuildPageWithState extends GoRouteData { const _GoRouteDataBuildPageWithState(); @override @@ -51,57 +96,99 @@ final List _routes = [ ]; void main() { - testWidgets( - 'It should build the page from the overridden build method', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsOneWidget); - expect(find.byKey(const Key('buildPage')), findsNothing); - expect(find.byKey(const Key('buildPageWithState')), findsNothing); - }, - ); - - testWidgets( - 'It should build the page from the overridden buildPage method', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build-page', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsNothing); - expect(find.byKey(const Key('buildPage')), findsOneWidget); - expect(find.byKey(const Key('buildPageWithState')), findsNothing); - }, - ); - - testWidgets( - 'It should build the page from the overridden buildPageWithState method', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build-page-with-state', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsNothing); - expect(find.byKey(const Key('buildPage')), findsNothing); - expect(find.byKey(const Key('buildPageWithState')), findsOneWidget); - }, - ); + group('GoRouteData >', () { + testWidgets( + 'build', + (WidgetTester tester) async { + final GoRouter goRouter = GoRouter( + initialLocation: '/build', + routes: _routes, + ); + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + expect(find.byKey(const Key('build')), findsOneWidget); + expect(find.byKey(const Key('buildPage')), findsNothing); + expect(find.byKey(const Key('buildPageWithState')), findsNothing); + }, + ); + + testWidgets( + 'buildPage', + (WidgetTester tester) async { + final GoRouter goRouter = GoRouter( + initialLocation: '/build-page', + routes: _routes, + ); + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + expect(find.byKey(const Key('build')), findsNothing); + expect(find.byKey(const Key('buildPage')), findsOneWidget); + expect(find.byKey(const Key('buildPageWithState')), findsNothing); + }, + ); + + testWidgets( + 'buildPageWithState', + (WidgetTester tester) async { + final GoRouter goRouter = GoRouter( + initialLocation: '/build-page-with-state', + routes: _routes, + ); + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + expect(find.byKey(const Key('build')), findsNothing); + expect(find.byKey(const Key('buildPage')), findsNothing); + expect(find.byKey(const Key('buildPageWithState')), findsOneWidget); + }, + ); + }); + + group('ShellRouteData >', () { + final List shellRoutes = [ + _shellRouteDataBuilder, + _shellRouteDataPageBuilder, + ]; + testWidgets( + 'builder', + (WidgetTester tester) async { + final GoRouter goRouter = GoRouter( + initialLocation: '/builder', + routes: shellRoutes, + ); + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + expect(find.byKey(const Key('builder')), findsOneWidget); + expect(find.byKey(const Key('page-builder')), findsNothing); + }, + ); + + testWidgets( + 'pageBuilder', + (WidgetTester tester) async { + final GoRouter goRouter = GoRouter( + initialLocation: '/page-builder', + routes: shellRoutes, + ); + await tester.pumpWidget(MaterialApp.router( + routeInformationProvider: goRouter.routeInformationProvider, + routeInformationParser: goRouter.routeInformationParser, + routerDelegate: goRouter.routerDelegate, + )); + expect(find.byKey(const Key('builder')), findsNothing); + expect(find.byKey(const Key('page-builder')), findsOneWidget); + }, + ); + }); } From 0ac454de0a4e9b4ed58c63e9426582089850fcc5 Mon Sep 17 00:00:00 2001 From: Michael Lazebnyi <62852417+hawkkiller@users.noreply.github.com> Date: Tue, 15 Nov 2022 17:55:02 +0200 Subject: [PATCH 09/20] remove line in changelog --- packages/go_router/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index dc1d49112675..5185470cf6fd 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -24,7 +24,6 @@ - Adds GoRouterState to context. - Fixes GoRouter notification. - Updates README. - - Removes dynamic calls in examples. - **BREAKING CHANGE** - Remove NavigatorObserver mixin from GoRouter From 9f06bd86f41fe9638467ee8f9e22f74bf9140525 Mon Sep 17 00:00:00 2001 From: Mihail Date: Fri, 2 Dec 2022 18:12:02 +0100 Subject: [PATCH 10/20] Add navigatorKey for each route --- packages/go_router/lib/src/route_data.dart | 41 ++++++++++------------ 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index b59dd5755cba..a5d9b3ef08da 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -17,15 +17,6 @@ import 'state.dart'; abstract class RouteData { /// {@macro route_data} const RouteData(); - - /// [navigatorKey] is used to point to a certain navigator - /// or pass it into the shell route - /// - /// In case of [ShellRoute] it will instantiate a new navigator - /// with the given key - /// - /// In case of [GoRoute] it will use the given key to find the navigator - GlobalKey? get navigatorKey => null; } /// Baseclass for supporting @@ -94,14 +85,13 @@ abstract class GoRouteData extends RouteData { /// A helper function used by generated code. /// /// Should not be used directly. - static String $location(String path, {Map? queryParams}) => - Uri.parse(path) - .replace( - queryParameters: - // Avoid `?` in generated location if `queryParams` is empty - queryParams?.isNotEmpty ?? false ? queryParams : null, - ) - .toString(); + static String $location(String path, {Map? queryParams}) => Uri.parse(path) + .replace( + queryParameters: + // Avoid `?` in generated location if `queryParams` is empty + queryParams?.isNotEmpty ?? false ? queryParams : null, + ) + .toString(); /// A helper function used by generated code. /// @@ -124,8 +114,7 @@ abstract class GoRouteData extends RouteData { return (_stateObjectExpando[state] ??= factory(state)) as T; } - Widget builder(BuildContext context, GoRouterState state) => - factoryImpl(state).build(context); + Widget builder(BuildContext context, GoRouterState state) => factoryImpl(state).build(context); Page pageBuilder(BuildContext context, GoRouterState state) => factoryImpl(state).buildPageWithState(context, state); @@ -148,6 +137,11 @@ abstract class GoRouteData extends RouteData { static final Expando _stateObjectExpando = Expando( 'GoRouteState to GoRouteData expando', ); + + /// [navigatorKey] is used to point to a certain navigator + /// + /// It will use the given key to find the right navigator for [GoRoute] + GlobalKey? get navigatorKey => null; } /// {@template shell_route_data} @@ -229,10 +223,12 @@ abstract class ShellRouteData extends RouteData { /// Used to cache [ShellRouteData] that corresponds to a given [GoRouterState] /// to minimize the number of times it has to be deserialized. - static final Expando _stateObjectExpando = - Expando( + static final Expando _stateObjectExpando = Expando( 'GoRouteState to ShellRouteData expando', ); + + /// It will be used to instantiate [Navigator] with the given key + GlobalKey? get navigatorKey => null; } /// {@template typed_route} @@ -290,6 +286,5 @@ class NoOpPage extends Page { const NoOpPage(); @override - Route createRoute(BuildContext context) => - throw UnsupportedError('Should never be called'); + Route createRoute(BuildContext context) => throw UnsupportedError('Should never be called'); } From 3edfe0b8f2c3adf64d196f5157e714a6d3ed6233 Mon Sep 17 00:00:00 2001 From: Mihail Date: Fri, 2 Dec 2022 18:48:28 +0100 Subject: [PATCH 11/20] Fixed tests --- packages/go_router/lib/src/route_data.dart | 2 -- packages/go_router/test/route_data_test.dart | 32 ++++++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index a5d9b3ef08da..b00cc99534f9 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -26,7 +26,6 @@ abstract class RouteData { /// [redirect]. /// {@category Type-safe routes} abstract class GoRouteData extends RouteData { - /// Allows subclasses to have `const` constructors. /// /// [GoRouteData] is abstract and cannot be instantiated directly. @@ -174,7 +173,6 @@ abstract class ShellRouteData extends RouteData { /// /// Should not be used directly. static ShellRoute $route({ - required String path, required T Function(GoRouterState) factory, GlobalKey? navigatorKey, List routes = const [], diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart index 7e4a408a2f7e..f9d12915ecda 100644 --- a/packages/go_router/test/route_data_test.dart +++ b/packages/go_router/test/route_data_test.dart @@ -34,9 +34,13 @@ final GoRoute _goRouteDataBuild = GoRouteData.$route( ); final ShellRoute _shellRouteDataBuilder = ShellRouteData.$route( - path: '/builder', factory: (GoRouterState state) => const _ShellRouteDataBuilder(), - routes: _routes, + routes: [ + GoRouteData.$route( + path: '/child', + factory: (GoRouterState state) => const _GoRouteDataBuild(), + ), + ], ); class _GoRouteDataBuildPage extends GoRouteData { @@ -70,9 +74,13 @@ final GoRoute _goRouteDataBuildPage = GoRouteData.$route( ); final ShellRoute _shellRouteDataPageBuilder = ShellRouteData.$route( - path: '/page-builder', factory: (GoRouterState state) => const _ShellRouteDataPageBuilder(), - routes: _routes, + routes: [ + GoRouteData.$route( + path: '/child', + factory: (GoRouterState state) => const _GoRouteDataBuild(), + ), + ], ); class _GoRouteDataBuildPageWithState extends GoRouteData { @@ -153,16 +161,14 @@ void main() { }); group('ShellRouteData >', () { - final List shellRoutes = [ - _shellRouteDataBuilder, - _shellRouteDataPageBuilder, - ]; testWidgets( 'builder', (WidgetTester tester) async { final GoRouter goRouter = GoRouter( - initialLocation: '/builder', - routes: shellRoutes, + initialLocation: '/child', + routes: [ + _shellRouteDataBuilder, + ], ); await tester.pumpWidget(MaterialApp.router( routeInformationProvider: goRouter.routeInformationProvider, @@ -178,8 +184,10 @@ void main() { 'pageBuilder', (WidgetTester tester) async { final GoRouter goRouter = GoRouter( - initialLocation: '/page-builder', - routes: shellRoutes, + initialLocation: '/child', + routes: [ + _shellRouteDataPageBuilder, + ], ); await tester.pumpWidget(MaterialApp.router( routeInformationProvider: goRouter.routeInformationProvider, From a8a467019c812c0a7fff6dc3446edabd328f19ab Mon Sep 17 00:00:00 2001 From: Mihail Date: Fri, 2 Dec 2022 18:54:41 +0100 Subject: [PATCH 12/20] Format --- packages/go_router/lib/src/route_data.dart | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index b00cc99534f9..ffccbdbd5989 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -84,13 +84,14 @@ abstract class GoRouteData extends RouteData { /// A helper function used by generated code. /// /// Should not be used directly. - static String $location(String path, {Map? queryParams}) => Uri.parse(path) - .replace( - queryParameters: - // Avoid `?` in generated location if `queryParams` is empty - queryParams?.isNotEmpty ?? false ? queryParams : null, - ) - .toString(); + static String $location(String path, {Map? queryParams}) => + Uri.parse(path) + .replace( + queryParameters: + // Avoid `?` in generated location if `queryParams` is empty + queryParams?.isNotEmpty ?? false ? queryParams : null, + ) + .toString(); /// A helper function used by generated code. /// @@ -113,7 +114,8 @@ abstract class GoRouteData extends RouteData { return (_stateObjectExpando[state] ??= factory(state)) as T; } - Widget builder(BuildContext context, GoRouterState state) => factoryImpl(state).build(context); + Widget builder(BuildContext context, GoRouterState state) => + factoryImpl(state).build(context); Page pageBuilder(BuildContext context, GoRouterState state) => factoryImpl(state).buildPageWithState(context, state); @@ -221,7 +223,8 @@ abstract class ShellRouteData extends RouteData { /// Used to cache [ShellRouteData] that corresponds to a given [GoRouterState] /// to minimize the number of times it has to be deserialized. - static final Expando _stateObjectExpando = Expando( + static final Expando _stateObjectExpando = + Expando( 'GoRouteState to ShellRouteData expando', ); @@ -284,5 +287,6 @@ class NoOpPage extends Page { const NoOpPage(); @override - Route createRoute(BuildContext context) => throw UnsupportedError('Should never be called'); + Route createRoute(BuildContext context) => + throw UnsupportedError('Should never be called'); } From 078c46fb3db9a3223c050a2d10d936242a391ddb Mon Sep 17 00:00:00 2001 From: Mihail Date: Fri, 2 Dec 2022 20:38:34 +0100 Subject: [PATCH 13/20] Bump version in pubspec.yaml --- packages/go_router/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 64eb598047a1..9fbfc28022b0 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.2.1 +version: 5.2.2 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 From c3ba9bacd38b7f23f3a5c614ecab4d952c51487b Mon Sep 17 00:00:00 2001 From: Michael Lazebnyi <62852417+hawkkiller@users.noreply.github.com> Date: Tue, 13 Dec 2022 23:13:50 +0200 Subject: [PATCH 14/20] Update packages/go_router/lib/src/route_data.dart Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com> --- packages/go_router/lib/src/route_data.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index ffccbdbd5989..8ccbb8cf6922 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -244,7 +244,7 @@ class TypedRoute { /// A superclass for each typed go route descendant /// {@endtemplate} @Target({TargetKind.library, TargetKind.classType}) -class TypedGoRoute extends TypedRoute { +class TypedGoRoute extends TypedRoute { /// {@macro typed_go_route} const TypedGoRoute({ required this.path, From ff1ee611e47962a6f65c43666e36459dd8efc0f1 Mon Sep 17 00:00:00 2001 From: Michael Lazebnyi <62852417+hawkkiller@users.noreply.github.com> Date: Tue, 13 Dec 2022 23:13:58 +0200 Subject: [PATCH 15/20] Update packages/go_router/lib/src/route_data.dart Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com> --- packages/go_router/lib/src/route_data.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index 8ccbb8cf6922..cab8be9965bf 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -268,7 +268,7 @@ class TypedGoRoute extends TypedRoute { /// A superclass for each typed shell route descendant /// {@endtemplate} @Target({TargetKind.library, TargetKind.classType}) -class TypedShellRoute extends TypedRoute { +class TypedShellRoute extends TypedRoute { /// {@macro typed_shell_route} const TypedShellRoute({ this.routes = const >[], From 24d7a364fea1adae89f9d5991fb79d4962db0fb4 Mon Sep 17 00:00:00 2001 From: Mihail Date: Mon, 30 Jan 2023 13:43:50 +0100 Subject: [PATCH 16/20] Removed buildPageWithState tests --- packages/go_router/test/route_data_test.dart | 36 -------------------- 1 file changed, 36 deletions(-) diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart index 6668bd71a3c0..986eccd3af09 100644 --- a/packages/go_router/test/route_data_test.dart +++ b/packages/go_router/test/route_data_test.dart @@ -121,7 +121,6 @@ void main() { )); expect(find.byKey(const Key('build')), findsOneWidget); expect(find.byKey(const Key('buildPage')), findsNothing); - expect(find.byKey(const Key('buildPageWithState')), findsNothing); }, ); @@ -139,25 +138,6 @@ void main() { )); expect(find.byKey(const Key('build')), findsNothing); expect(find.byKey(const Key('buildPage')), findsOneWidget); - expect(find.byKey(const Key('buildPageWithState')), findsNothing); - }, - ); - - testWidgets( - 'buildPageWithState', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build-page-with-state', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsNothing); - expect(find.byKey(const Key('buildPage')), findsNothing); - expect(find.byKey(const Key('buildPageWithState')), findsOneWidget); }, ); }); @@ -235,22 +215,6 @@ void main() { }, ); - testWidgets( - 'It should build the page from the overridden buildPage method', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build-page-with-state', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsNothing); - expect(find.byKey(const Key('buildPage')), findsNothing); - }, - ); testWidgets( 'It should redirect using the overridden redirect method', (WidgetTester tester) async { From 9cd060ddd50b499e5450479eaf24749478c05db8 Mon Sep 17 00:00:00 2001 From: Mihail Date: Tue, 31 Jan 2023 10:03:44 +0100 Subject: [PATCH 17/20] Bump version in pubspec --- packages/go_router/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index ade738b44c91..e29a5f0c89fc 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: 6.0.2 +version: 6.0.3 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 From 3246f3e450d42c28a190c265d29570a388bb2151 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Wed, 22 Feb 2023 10:50:28 -0800 Subject: [PATCH 18/20] Address comments from code review --- packages/go_router/CHANGELOG.md | 1 + packages/go_router/lib/src/route_data.dart | 22 +++------- packages/go_router/test/route_data_test.dart | 45 +++----------------- 3 files changed, 13 insertions(+), 55 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index f4802552e750..e7346b2a7379 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -32,6 +32,7 @@ - Makes `CustomTransitionPage.barrierDismissible` work ## 6.0.2 + - Fixes missing result on pop in go_router extension. ## 6.0.1 diff --git a/packages/go_router/lib/src/route_data.dart b/packages/go_router/lib/src/route_data.dart index e7fd664c5696..3530ca250fc4 100644 --- a/packages/go_router/lib/src/route_data.dart +++ b/packages/go_router/lib/src/route_data.dart @@ -11,11 +11,9 @@ import 'package:meta/meta_meta.dart'; import 'route.dart'; import 'state.dart'; -/// {@template route_data} /// A superclass for each route data -/// {@endtemplate} abstract class RouteData { - /// {@macro route_data} + /// Default const constructor const RouteData(); } @@ -128,12 +126,10 @@ abstract class GoRouteData extends RouteData { GlobalKey? get navigatorKey => null; } -/// {@template shell_route_data} -/// Baseclass for supporting +/// Base class for supporting /// [nested navigation](https://pub.dev/packages/go_router#nested-navigation) -/// {@endtemplate} abstract class ShellRouteData extends RouteData { - /// {@macro shell_route_data} + /// Default const constructor const ShellRouteData(); /// [pageBuilder] is used to build the page @@ -215,20 +211,16 @@ abstract class ShellRouteData extends RouteData { GlobalKey? get navigatorKey => null; } -/// {@template typed_route} /// A superclass for each typed route descendant -/// {@endtemplate} class TypedRoute { - /// {@macro typed_route} + /// Default const constructor const TypedRoute(); } -/// {@template typed_go_route} /// A superclass for each typed go route descendant -/// {@endtemplate} @Target({TargetKind.library, TargetKind.classType}) class TypedGoRoute extends TypedRoute { - /// {@macro typed_go_route} + /// Default const constructor const TypedGoRoute({ required this.path, this.routes = const >[], @@ -247,12 +239,10 @@ class TypedGoRoute extends TypedRoute { final List> routes; } -/// {@template typed_shell_route} /// A superclass for each typed shell route descendant -/// {@endtemplate} @Target({TargetKind.library, TargetKind.classType}) class TypedShellRoute extends TypedRoute { - /// {@macro typed_shell_route} + /// Default const constructor const TypedShellRoute({ this.routes = const >[], }); diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart index 986eccd3af09..558d5f5d60dd 100644 --- a/packages/go_router/test/route_data_test.dart +++ b/packages/go_router/test/route_data_test.dart @@ -106,9 +106,9 @@ final List _routes = [ ]; void main() { - group('GoRouteData >', () { + group('GoRouteData', () { testWidgets( - 'build', + 'It should build the page from the overridden build method', (WidgetTester tester) async { final GoRouter goRouter = GoRouter( initialLocation: '/build', @@ -125,7 +125,7 @@ void main() { ); testWidgets( - 'buildPage', + 'It should build the page from the overridden buildPage method', (WidgetTester tester) async { final GoRouter goRouter = GoRouter( initialLocation: '/build-page', @@ -142,9 +142,9 @@ void main() { ); }); - group('ShellRouteData >', () { + group('ShellRouteData', () { testWidgets( - 'builder', + 'It should build the page from the overridden build method', (WidgetTester tester) async { final GoRouter goRouter = GoRouter( initialLocation: '/child', @@ -163,7 +163,7 @@ void main() { ); testWidgets( - 'pageBuilder', + 'It should build the page from the overridden buildPage method', (WidgetTester tester) async { final GoRouter goRouter = GoRouter( initialLocation: '/child', @@ -181,39 +181,6 @@ void main() { }, ); }); - testWidgets( - 'It should build the page from the overridden build method', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsOneWidget); - expect(find.byKey(const Key('buildPage')), findsNothing); - }, - ); - - testWidgets( - 'It should build the page from the overridden buildPage method', - (WidgetTester tester) async { - final GoRouter goRouter = GoRouter( - initialLocation: '/build-page', - routes: _routes, - ); - await tester.pumpWidget(MaterialApp.router( - routeInformationProvider: goRouter.routeInformationProvider, - routeInformationParser: goRouter.routeInformationParser, - routerDelegate: goRouter.routerDelegate, - )); - expect(find.byKey(const Key('build')), findsNothing); - expect(find.byKey(const Key('buildPage')), findsOneWidget); - }, - ); testWidgets( 'It should redirect using the overridden redirect method', From 298a6017b4e57022dbbf669e4a92ba57ba5427d0 Mon Sep 17 00:00:00 2001 From: John Ryan Date: Wed, 22 Feb 2023 11:33:45 -0800 Subject: [PATCH 19/20] Export ShellRouteData --- packages/go_router/lib/go_router.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/go_router/lib/go_router.dart b/packages/go_router/lib/go_router.dart index be00adca1f4a..b60ae60115be 100644 --- a/packages/go_router/lib/go_router.dart +++ b/packages/go_router/lib/go_router.dart @@ -11,7 +11,8 @@ export 'src/configuration.dart' export 'src/misc/extensions.dart'; export 'src/misc/inherited_router.dart'; export 'src/pages/custom_transition_page.dart'; -export 'src/route_data.dart' show GoRouteData, TypedGoRoute, TypedShellRoute; +export 'src/route_data.dart' + show GoRouteData, TypedGoRoute, TypedShellRoute, ShellRouteData; export 'src/router.dart'; export 'src/typedefs.dart' show GoRouterPageBuilder, GoRouterRedirect, GoRouterWidgetBuilder; From c2d1f48b03ce9cf7f7750917db12ea243ea50c9d Mon Sep 17 00:00:00 2001 From: John Ryan Date: Wed, 22 Feb 2023 11:37:51 -0800 Subject: [PATCH 20/20] Remove unnecessary import --- packages/go_router/test/route_data_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart index 558d5f5d60dd..15419406f645 100644 --- a/packages/go_router/test/route_data_test.dart +++ b/packages/go_router/test/route_data_test.dart @@ -7,7 +7,6 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; -import 'package:go_router/src/route_data.dart'; class _GoRouteDataBuild extends GoRouteData { const _GoRouteDataBuild();