It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

As mentioned in the other PR, please let me know how you want me to refactor the GoRouterShellGenerator to avoid duplicated code and I'll write a test based on it.
Cheers.

Yes, it will be better if we can refactor GoRouterGenerator to prevent code duplication. I imagine we will add more shellroute subclass in the future

I've refactored the generator in 76b9b81, let me know what you think.

For the tests, I'm not sure what is the most pertinent. It seems most of the code is already covered by packages/go_router_builder/test/builder_test.dart or packages/go_router_builder/example/test/ensure_build_test.dart. Let me know what use case you want to test.
Thanks!

I just learned from @johnpryan that annotation has to be const. Let me think a bit on what else we can do here...

Yes, annotations must have a const constructor, so all of the fields must be final and all of the parameters must be const too. Since Keys in Flutter aren't const, I'm not sure how we're going to support navigatorKey or parentNavigatorKey.

Is that a fair assessment of where this is at, at the moment?

That's correct. Right now I'm waiting for @chunhtai or @johnpryan 's advice on how to manage the navigatorKey and the parentNavigatorKey.

We cannot pass them to TypedShellRoute / TypedGoRoute because GlobalKey<NavigatorState>() is not a const. So either we can either:

1. declare them as a static member called $navigatorKey in the typed route and the generator will pick them up (but it's not obvious for users of this library)
2. use the workaround described in [go_router_builder] Add parentNavigatorKey to generated routes. flutter#119294 (comment) that clones each routes after the code generation to add the keys before passing them to GoRouter.

I will give a try to this PR since I built around everything with Typed. Thank you for the workaround.

@GP4cK

After some thought, I think we can have a getter in the GoRouteData and ShellRouteData

for example:

abstract class GoRouteData extends RouteData {
 ...
 GlobalKey<NavigatorKey> get $parentNavigatorKey => null;
 ...
}

abstract class ShellRouteData extends RouteData {
 ...
 GlobalKey<NavigatorKey> get $navigatorKey => null;
 ...
}

and user can override and provide their navigator key in the subclass if they want. I was hoping this would be more clear than using the static field. At least people can see the getter in the abstract class and notice such thing exists.

WDYT @johnpryan @GP4cK

I'm good with that and it's not too far from the initial implementation.

EDIT: actually the problem is that if the keys are non-static getters, I don't know how to invoke the getter or get the name of the variable returned by it in the generated code.

For example, in this code:

final GlobalKey<NavigatorState> goRouteKey = GlobalKey<NavigatorState>();

class GoRouteWithKey extends GoRouteData {
  const GoRouteWithKey();

  @override
  GlobalKey<NavigatorState>? get navigatorKey => goRouteKey;
}

In the _decodeKey() method, we can find the navigatorKey with

classElement.fields
  .where(whereKeyName)
  .where((FieldElement element) {
    final DartType type = element.type;
    if (type is! ParameterizedType) {
      return false;
    }
    final List<DartType> typeArguments = type.typeArguments;
    if (typeArguments.length != 1) {
      return false;
    }
    final DartType typeArgument = typeArguments.single;
    if (typeArgument.getDisplayString(withNullability: false) ==
        'NavigatorState') {
      return true;
    }
    return false;
  })

But once we have the ref to the navigatorKey, how to know that the return value is goRouteKey?

I have a question regarding @TypedShellRoute. Let's say I have two screens: SplashView & HomeView. The first screen shown is SplashView, generated by @TypedGoRouter, after the native splash screen. After SplashView, it will directly transition to HomeView which is generated by @TypedShellGoRouter.

Apparently, to run this flow, I should put HomeView as a childRoute element. If I do not, it does not generate the code.

final GlobalKey<NavigatorState> _navigatorKey = GlobalKey<NavigatorState>(); 
final GlobalKey<NavigatorState> _shellNavigatorKey = GlobalKey<NavigatorState>();

final router = GoRouter(
  navigatorKey: _navigatorKey,
  initialLocation: AppRoutes.SPLASH,
  observers: [NavigatorObservers()],
  routes: $appRoutes,
);

/// Splash Route

@immutable
@TypedGoRoute<SplashViewRoute>(path: AppRoutes.SPLASH, routes: [])
class SplashViewRoute extends GoRouteData {
  const SplashViewRoute();

  @override
  Widget build(BuildContext _, GoRouterState state) => const SplashView();
}

///

/// Home Route
@TypedShellRoute(
  routes: [
    TypedGoRoute<SubViewRoute>(path: AppRoutes.HOME),
  ],
)
class HomeViewRoute extends ShellRouteData {
  const HomeViewRoute();

  @override
  Widget builder(BuildContext context, GoRouterState state, Widget navigator) => const SizedBox.shrink();

  @override
  GlobalKey<NavigatorState>? get navigatorKey => _shellNavigatorKey;
}

class SubViewRoute extends GoRouteData {
  const SubViewRoute();

  @override
  Widget build(BuildContext _, GoRouterState state) => const SizedBox.shrink();

  @override
  GlobalKey<NavigatorState>? get navigatorKey => _shellNavigatorKey;
}

If I turned HomeView to @TypedGoRoute, I can navigate without declaring it as a childRoute.
Am I doing something wrong? Should not I at least navigate to SubView?

I am getting this error when I tried to generate a code:

The @TypedGoRoute annotation must have a type parameter that matches the annotated element.
package:routes/routes.dart:38:7
   ╷
38 │ class HomeViewRoute extends ShellRouteData 

edit: pubspec.yaml

environment:
  sdk: ">=2.19.5 <3.0.0"
  flutter: 3.7.9

dependencies:
  flutter:
    sdk: flutter

  cupertino_icons: ^1.0.2
  # Routing
  go_router: 6.5.2

dev_dependencies:
  flutter_test:
    sdk: flutter

  flutter_lints: ^2.0.0

  # Code Generation
  analyzer: 5.10.0
  build_runner: 2.3.3
  build_verify: 3.1.0

  # Builders
  # TODO: do not forget to change this to upstream when its merged
  go_router_builder:
    git:
      url: [email protected]:GP4cK/packages.git
      path: packages/go_router_builder
      ref: feature/shell-route-go-router-builder

flutter:
  uses-material-design: true

@GP4cK @chunhtai

I have a question [...]

@kaankoken, I copy/pasted your code in the example, ran the generator and it did generate some code. The only line I modified was

/// Home Route
-@TypedShellRoute(
+@TypedShellRoute<HomeViewRoute>(

Also, I see that you're overriding the navigatorKey but with the current state of this PR, this will do nothing. You should replace it like this:

-@override
-GlobalKey<NavigatorState>? get navigatorKey => _shellNavigatorKey;
+static final GlobalKey<NavigatorState> $navigatorKey = _shellNavigatorKey;

yeah I was confused myself how GoRouteData.$route was called. it looks like using static field may be cleanest way to approach this. Sorry for all the detours, let's go with the original proposal.