[go_router] Path based branch routing for StatefulShellRoute - deprecating goBranch

[tolo]

trafficstars

Support for path based routing (i.e. using GoRouter.go) when switching branches in a StatefulShellRoute, making routing with StatefulShellRoute less of a special case. This also means that the goBranch method becomes obsolete.

This change introduces a new field on StatefulShellRoute to configure a path for branch navigation on the shell route, which under the hood generates a number of redirection routes. These routes allow for path based navigation for switching branch and restoring branch/route state. Effectively, this negates the need to use goBranch, and even the need to expose the class StatefulNavigationShell.

To summarise, the basic motivations of this PR are:

• Introduce path based navigation for StatefulShellRoute, to make more in line with routing in GoRouter in general, and less of a special case.
• Related to the above point: remove the method goBranch, primarily from the Widget StatefulNavigationShell, where it is somewhat misplaced.
• Hide internal implementation details, like StatefulNavigationShell, to make the surface area of the StatefulShellRoute API smaller, and again; more in line with GoRouter in general, and less of a special case.

TL;DR - see the stateful_shell_route.dart example for a runnable demo of this change.

---

Short demonstration of this change You start by configuring your shell route like this:

StatefulShellRoute.indexedStack(
  path: '/myshell', 
  initialBranchIndex: 1, // Optionally set which branch should be default
  ...
  branches: <StatefulShellBranch>[
    StatefulShellBranch(
    name: 'branchA', // Optionally associate a name/alias to a branch
    ...

This enables you to do this:

GoRouter.of(context).go('/myshell/0');
// or this:
GoRouter.of(context).go('/myshell/branchA');

Instead of this:

StatefulNavigationShell.of(context).goBranch(0);

There are also additional redirects available:

// Redirect to the initial state of the first branch: 
GoRouter.of(context).go('/myshell/0/initial'); 
// or:
GoRouter.of(context).go('/myshell/branchA/initial'); 

// Redirect to the current state/branch of the stateful shell route. If no state 
// exists, the redirect will go to the initial location of the branch at 'initialBranchIndex'
GoRouter.of(context).go('/myshell'); 

Update The latest version of this PR also included some refactoring around the StatefulShellRoute API, including the introduction of the GoRouterState subclass ShellRouteState. The goal with this is to hide internal implementation details (primarily the class StatefulNavigationShell) and improve the DX of the API. Example (from stateful_shell_route.dart):

StatefulShellRoute.indexedStackContainer(
  path: '/shell',
  builder: (BuildContext context, ShellRouteState state, Widget child) {
    return ScaffoldWithNavBar(shellState: state, child: child);
   }
   ...,
}

...

class ScaffoldWithNavBar extends StatelessWidget {
  const ScaffoldWithNavBar({
    required this.shellState,
    required this.child,
    Key? key,
  }) : super(key: key ?? const ValueKey<String>('ScaffoldWithNavBar'));

  final ShellRouteState shellState;
  final Widget child;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: child,
      bottomNavigationBar: BottomNavigationBar(
          items: const <BottomNavigationBarItem>[
            BottomNavigationBarItem(icon: Icon(Icons.home), label: 'Section A'),
            BottomNavigationBarItem(icon: Icon(Icons.work), label: 'Section B'),
            BottomNavigationBarItem(icon: Icon(Icons.tab), label: 'Section C'),
          ],
          currentIndex: shellState.navigatorIndex,
          onTap: (int index) {
            // It's possible to simply do this to navigate between branches:
            // return GoRouter.of(context).go('/shell/$index');
            // But since names are configured for the branches, it is also
            // possible to restore a branch by name, like this:
            return switch (index) {
              1 => GoRouter.of(context).goNamed('branchB'), 
              2 => GoRouter.of(context).goNamed('branchC'),
              _ => GoRouter.of(context).goNamed('branchA'),
            };
            // Additionally, this also works: GoRouter.of(context).go('/shell/branchB'),
          }),
    );
  }
}

---

This PR addresses:

• flutter/flutter#128262
• Parts of flutter/flutter#140273

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene page, which explains my responsibilities.
• [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• [x] I signed the CLA.
• [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• [x] I linked to at least one issue that this PR fixes in the description above.
• [ ] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• [ ] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.
• [ ] I updated/added relevant documentation (doc comments with ///).
• [ ] I added new tests to check the change I am making, or this PR is test-exempt.
• [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[tolo]

@chunhtai, would appreciate if you could have a look at this and see what you think, before I contuine fully implementing/documenting/testing this.

The current implementation works (at least as first attempt) and the stateful_shell_route.dart sample is updated to use it.

[chunhtai]

It feels a bit weird that we force the statefulbranch to use numeric path, is there a reason behind this change?

[tolo]

It feels a bit weird that we force the statefulbranch to use numeric path, is there a reason behind this change?

Well, a numeric index is what we're using already, via the programmatic API. So there really is no change in what information you use to switch branch.

But I was actually thinking about re-introducing the concept of giving each branch a name (which did exist at some point in the original StatefulShellRoute PR), and now that I think of it, this may certainly make the navigation feel less weird.

I'll go ahead and add that change.

[chunhtai]

I am more concern about url will be force to use numeric path like /mypath/0, /mypath/1.

Another concern is that we should figure out when two url may point to same page. for example if /mypath/subpath also switch branch to 0, then both /mypath/subpath and /mypath/0 points to same page. In this case should the url for /mypath/0 redirect to /mypath/subpath? we need to update the reportRouteToEngine code to make sure we update the history correctly.

[tolo]

Updated the description with more information about using names instead of indices.

This change uses the standard redirect handling, so I would assume circular redirects etc would be handled already (i.e. redirect limits and supporting reporting back to the engine)?

[tolo]

Ok, so I took it one step further, to more thoroughly adress the issues of flutter/flutter#128262. It may be too much to do in a single step / PR, but I mainly did it to showcase a potential refactoring of the API around StatefulShellRoute, to improve the DX and make the API more in line with the rest of go router (i.e. path based). Would be great if you could take a deeper look at this @chunhtai.

(Edit: updated the original PR description/comment with information about these changes).

[cedvdb]

@tolo I'm not sure I understand the point of this PR. router.go('/branchA'); already works. Why would a number in the URL be desirable ( when you consider the web platform ) ?

This is a response to your comment on the other PR where you link this one.

Maybe it's to make routing work nicely with the usual navigation widgets which work with indexes currently, in which case, what's wrong with:

StatefulNavigationShell.of(context).goBranch(index);

[tolo]

@tolo I'm not sure I understand the point of this PR. router.go('/branchA'); already works. Why would a number in the URL be desirable ( when you consider the web platform ) ?

Basically, the initial intent of this PR was to add support for a path-based way of navigating to the current state of a stateful shell route or a branch thereof, which is not possible today.

The more I worked on this PR though, other related opportunities for improvements also presented themselves. I would now summarise the motivations of this PR like this (will add it to the description):

• Introduce path based navigation for StatefulShellRoute, to make more in line with routing in GoRouter in general, and less of a special case.
• Related to the above point: remove the method goBranch, primarily from the Widget StatefulNavigationShell, where it is somewhat misplaced.
• Hide internal implementation details, like StatefulNavigationShell, to make the surface area of the StatefulShellRoute API smaller, and again; more in line with GoRouter in general, and less of a special case.

[tolo]

instead of using index, have you thought about using name instead? i.e a new name parameter to StatefulShellRouteBranch. and then you can use reuse goName API

Good suggestion. Name was supported though in the latest change, but I've now also added support for goNamed (branch name gets mapped to a new StatefulShellRestoreStateRedirect). Again, see the stateful_shell_route.dart for a working example.

[tolo]

Added some generalization and simplification around both the path based redirects and the API in general. Will update the PR description to reflect these changes after the weekend.

[tolo]

Decided to narrow the focus of this PR to mainly adress the goBranch API as well as introducing improvements around the ShellRoute/StatefulShellRoute API in general. Will update name and description shortly.

[tolo]

@chunhtai / @hannah-hyj, when you have a minute, I'd appreciate you're thoughts around this.

[chunhtai]

I will take a look this week