[go_router] Add ShellRoute

[johnpryan]

This adds ShellRoute, a new route type that builds an inner Navigator

resolves flutter/flutter#108141
resolves flutter/flutter#99126
resolves flutter/flutter#109479

[chunhtai]

Only half way through, will continue review next week

[loic-sharma]

About this example:

Example...

  final GoRouter _router = GoRouter(
    navigatorKey: _rootNavigatorKey,
    routes: <RouteBase>[
      /// Application shell
      ShellRoute(
        path: '/',
        ...
        routes: <RouteBase>[
          ...
          /// Displayed when the second item in the the bottom navigation bar is selected.
          GoRoute(
            path: 'b',
            ...
            routes: <RouteBase>[
              /// Same as "/a/details", but displayed on the root Navigator.
              GoRoute(
                path: 'details',
                // Display on the root Navigator
                navigatorKey: _rootNavigatorKey,
                builder: (BuildContext context, GoRouterState state) {
                  return const DetailsScreen(label: 'B');
                },
              ),
            ],
          ),
        ],
      ),
    ],
  );

Some comments:

1. Why is the ShellRoute wrapped in a GoRoute? Could we make it be the "top" route?
2. It looks like navigatorKey can be used for multiple purposes. From my understanding, the top GoRoute uses navigatorKey to set its underlying navigator's key and the /b/details GoRoute uses navigatorKey to choose which navigator to display on. Why is that? These seem like separate concerns, should they have separate parameters?

[johnpryan]

Why is the ShellRoute wrapped in a GoRoute? Could we make it be the "top" route?

It's not, it's wrapped in a GoRouter.

It looks like navigatorKey can be used for multiple purposes. From my understanding, the top GoRoute uses navigatorKey to set its underlying navigator's key and the /b/details GoRoute uses navigatorKey to choose which navigator to display on. Why is that? These seem like separate concerns, should they have separate parameters?

The GoRouter's navigatorKey specifies the key for the top-level Navigator. Since 87c16ad renames shellNavigatorKey and navigatorKey to navigatorKey and parentNavigatorKey respectively, this should be more clear.

[loic-sharma]

It's not, it's wrapped in a GoRouter.

Whoops my bad 🤦

[loic-sharma]

I'm worried that folks will see this example and think you need to define a root navigator key to use ShellRoute. I wonder if we should have two separate shell route examples:

1. The simplest shell route example possible
2. This more advanced shell route example that shows you how to "break out" of the shell route and display on top of the root navigator.

[johnpryan]

SGTM - I'll break it out into two samples

[sigis151]

Hello, I am experimenting and trying to implement tabs with ShellRoute, but I was wondering if it is (or will be) possible to persist the navigation stack in the selected tab? e.g. when navigating from /a to /a/details then to /b and going back to /a would open /a/details page. I would like to implement similar navigation as the Youtube app has.

[miaosun009]

@johnpryan
ShellRoute is used on the root route, and the result of calling context.canPop() under the sub-route of the current ShellRoute returns true. At this time, calling context.pop() will return a blank page.
The expected result should be context.canPop() =fase

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      routerDelegate: router.routerDelegate,
      routeInformationParser: router.routeInformationParser,
      routeInformationProvider: router.routeInformationProvider,
    );
  }

  final router = GoRouter(
    routes: [
      ShellRoute(
        path: '/',
        defaultRoute: 'home',
        builder: (_, __, child) {
          return AppScaffold(
            child: child,
          );
        },
        routes: [
          GoRoute(
            path: 'home',
            builder: (_, __) => HomePage(),
          ),
          GoRoute(
            path: 'me',
            builder: (_, __) => MePage(),
          ),
        ],
      ),
    ],
  );
}

class HomePage extends StatelessWidget {
  const HomePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text("Home Page"),
      ),
      body: Column(
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          const Center(
            child: Text("Home"),
          ),
          Center(
            child: MaterialButton(
              child: const Text("pop"),
              onPressed: () {
                /// Why is canPop true
                if (context.canPop()) {
                  context.pop();
                }
              },
            ),
          ),
        ],
      ),
    );
  }
}

class MePage extends StatelessWidget {
  const MePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text("Me Page"),
      ),
      body: Column(
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          const Center(
            child: Text("Me"),
          ),
          Center(
            child: MaterialButton(
              child: const Text("pop"),
              onPressed: () {
                context.pop();
              },
            ),
          ),
        ],
      ),
    );
  }
}

class AppScaffold extends StatelessWidget {
  final Widget child;

  const AppScaffold({
    Key? key,
    required this.child,
  }) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Column(
        children: [
          Expanded(
            child: child,
          ),
          Row(
            children: [
              Expanded(
                child: MaterialButton(
                  color: Colors.green,
                  onPressed: () {
                    context.go('/home');
                  },
                  child: const Text("Home"),
                ),
              ),
              Expanded(
                child: MaterialButton(
                  color: Colors.blue,
                  onPressed: () {
                    context.go('/me');
                  },
                  child: const Text("Me"),
                ),
              )
            ],
          )
        ],
      ),
    );
  }
}

[johnpryan]

Hello, I am experimenting and trying to implement tabs with ShellRoute, but I was wondering if it is (or will be) possible to persist the navigation stack in the selected tab?

@sigis151 this won't be included in this initial version of ShellRoute, but you can follow this feature here: flutter/flutter#99124

[johnpryan]

@miaosun009 thanks for bringing that up and providing a snippet - as this is currently built, ShellRoutes can be pushed and popped like normal GoRoutes, but we might need to figure out what canPop should do in this situation. For example, what if I'm building an app that needs to push a ShellRoute with a tab bar? Should canPop return false in that case?

[chunhtai]

I didn't look at the test yet

[miaosun009]

@johnpryan
As my example shows, my purpose is to build an interface with a bottom tab bar, each tab corresponds to a sub-route, I want the current route to be the top-level route, its canPop should be false, I also know Now why canPop is true, because the current routing stack is like this [/,/home], so when I call canPop in /home, matches.length is > 1

Maybe it's not a ShellRoute problem, but to achieve what I want, what do I need to do

[johnpryan]

@miaosun009 after talking about this PR with @chunhtai some more I think we are going to remove the path parameter from ShellRoute. This will make upgrading from navigatorBuilder easier and also make your snippet work as expected, since there will no longer be a RouteMatch for /.

Edit: We are removing the path parameter

[chunhtai]

everything else looks good

[chunhtai]

This will fail but it should pass

test(
      'Does not throw with valid parentNavigatorKey configuration',
      () {
        final GlobalKey<NavigatorState> root =
            GlobalKey<NavigatorState>(debugLabel: 'root');
        final GlobalKey<NavigatorState> shell =
            GlobalKey<NavigatorState>(debugLabel: 'shell');
        final GlobalKey<NavigatorState> shell2 =
            GlobalKey<NavigatorState>(debugLabel: 'shell2');
        RouteConfiguration(
          navigatorKey: root,
          routes: <RouteBase>[
            ShellRoute(
              navigatorKey: shell,
              routes: <RouteBase>[
                GoRoute(
                  path: '/',
                  builder: _mockScreenBuilder,
                  routes: <RouteBase>[
                    GoRoute(
                      path: 'a',
                      builder: _mockScreenBuilder,
                      parentNavigatorKey: root,
                      routes: <RouteBase>[
                        ShellRoute(
                          navigatorKey: shell2,
                          routes: <RouteBase>[
                            GoRoute(
                              path: 'b',
                              builder: _mockScreenBuilder,
                              routes: <RouteBase>[
                                GoRoute(
                                  path: 'b',
                                  builder: _mockScreenBuilder,
                                  parentNavigatorKey: shell2,
                                ),
                              ],
                            ),
                          ],
                        ),
                      ],
                    ),
                  ],
                ),
              ],
            ),
          ],
          redirectLimit: 10,
          topRedirect: (GoRouterState state) {
            return null;
          },
        );
      },
    );

[johnpryan]

@chunhtai good catch, 9d92bbe should do the trick.

[johnpryan]

@chunhtai I forgot to add your test case: fb36859

[chunhtai]

LGTM

[flodaniel]

@miaosun009 thanks for bringing that up and providing a snippet - as this is currently built, ShellRoutes can be pushed and popped like normal GoRoutes, but we might need to figure out what canPop should do in this situation. For example, what if I'm building an app that needs to push a ShellRoute with a tab bar? Should canPop return false in that case?

@johnpryan is this still true in the final merged version? When pushing a GoRoute that is inside a ShellRoute, the builder function is not executed. However when using context.go it works. I am trying to add some bloc dependencies in the ShellRoute builder, hoping it would run for go and push alike.
Thank you for this great PR, have been following it a lot in the last 2 weeks!

[johnpryan]

@flodaniel no, ShellRoutes can't be pushed and popped. Do you have a snippet that shows the issue you are seeing? Feel free to file an issue and cc me.

[flodaniel]

Oh. I am not sure that this justifies as an issue right now. I am posting this here but can open an issue if you think it makes sense.
In the first snippet you can see how I currently handle it. I have two pages that both need access to the same bloc instance (to share a state). I wanted to use a parent ShellRoute to provide the bloc (using a BlocProvider widget) as a dependency to both pages (second snipped).

Current solution:

/// this is where i now store the bloc instance to be reused
var EnterAddressBloc? _bloc;

/// these are my routes
return [
     GoRoute(
       path: prefix + RoutesAddress.overview.name,
       builder: (context, state) => BlocProvider(
         create: (context) {
           _bloc = EnterAddressBloc();
           return _bloc!;
         },
         child: EnterAddressPage(),
       ),
     ),
     GoRoute(
       path: prefix + RoutesAddress.selectAddress.name,
       builder: (context, state) => BlocProvider.value(
         value: _bloc!,
         child: SelectAddressPage(),
       ),
     ),
];

This is how it works when i go to the first child route of the ShellRoute, but not if I push it.

ShellRoute(
        builder: (context, state, child) {
          return BlocProvider(
            create: (context) => EnterAddressBloc(),
            child: child,
          );
        },
        routes: [
          GoRoute(
            path: prefix + RoutesAddress.overview.name,
            builder: (context, state) => EnterAddressPage(),
          ),
          GoRoute(
            path: prefix + RoutesAddress.selectAddress.name,
            builder: (context, state) => SelectAddressPage(),
          ),
        ],
      )

The solution with using the ShellRoute is shorter, cleaner and allows to clearly scope dependencies to a number of (sub)-pages. I am migrating our project from flow_builder to go_router and the major downside is that we have to move all our dependencies injections further up the widget tree without an easy solution to scope them to a number of sub-pages, which we really hoped we could do with ShellRoutes :)

[johnpryan]

@flodaniel I think we need to ensure that the BuildContext is correct for all builders, this will be addressed with relative routing I think.

[flodaniel]

Thanks a lot @johnpryan. I will follow this ticket and hope for a swift merge. You rock!