[go_router] Improvement of StatefulShellRoute API - deprecating goBranch

[tolo]

This PR introduces refactoring around the API of StatefulShellRoute and ShellRouteBase, to make routing with StatefulShellRoute less of a special case. The primary reason of this PR is to move away from the use of the goBranch method and also the need to expose the class StatefulNavigationShell.

To summarise, the basic motivations of this PR are:

• 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.
• Reduce the direct dependencies on the somewhat special-case implementation of StatefulShellRoute and instead depend only on the abstract base class ShellRouteBase.

The main changes in this PR are:

• Introduction of new GoRouteState subclass: ShellRouteState. This class takes over the responsibility from StatefulNavigationShell when it comes to getting the active navigation state (i.e. branch index and branch locations).
• Builder functions of ShellRoute and StatefulShellRoute updated to accept a ShellRouteState. StatefulShellRoute now uses same signature of the functions as ShellRoute.

Basically, instead of using goBranch, you now do this:

  /// The state of the shell route.
  final ShellRouteState shellState;

  @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'),
          ],
          // NEW API:
          currentIndex: shellState.navigatorIndex,
          onTap: (int index) {
            // NEW API:
            context.restore(shellState.navigatorLocation(index));
          }),
    );
  }

See the stateful_shell_route.dart example for a runnable demo of this change.

---

This PR addresses:

• [go_router] avoid exposing goBranch from StatefulNavigationShell flutter#128262

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• 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.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• 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.

[chunhtai]

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

[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