[go_router] Support for top level `onEnter` callback.

[omar-hanafy]

Description:
This PR introduces top level onEnter callback in RouteConfiguration to allow developers to intercept and conditionally block navigation based on incoming routes. It adds an example (top_level_on_enter.dart) demonstrating its usage, such as handling referral code from /referral.

What This PR Changes:

• Adds the onEnter callback (typedef OnEnter) to intercept route navigation before processing.
• Implements logic for onEnter in GoRouteInformationParser.
• Updates RouteConfiguration to include the onEnter callback.
• Adds a new example, top_level_on_enter.dart, to demonstrate the feature.
• Adds a test case to verify the onEnter callback functionality.

Simple usage example:

final GoRouter router = GoRouter(
  onEnter: (context, state) {
    // Save the referral code (if provided) and block navigation to /referral.
    if (state.uri.path == '/referral') {
      saveReferralCode(context, state.uri.queryParameters['code']);
      return false;
    }

    return true; // Allow navigation for all other routes.
  },
  routes: [ ... ],
);

Impact:
Enables developers to implement route-specific logic, such as support for handling action-based deep links without navigation (160602)

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., [go_router].
• [x] I [linked to at least one issue that this PR fixes] in the description above.
• [x] I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
• [x] I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes].
• [x] I updated/added relevant documentation (doc comments with ///).
• [x] I added new tests to check the change I am making, or this PR is [test-exempt].
• [x] All existing and new tests are passing.

[omar-hanafy]

Hi @chunhtai,

This PR introduces a top-level onEnter callback, addressing critical gap in go_router’s handling of deep links and redirect customizations. It allows developers to intercept routes, block navigation, or implement custom logic.

Given its potential impact, your feedback would be greatly appreciated.

Thank you for your time!

[omar-hanafy]

Hi @chunhtai, Following up on this PR. I've updated the implementation to:

1. Enhance OnEnter signature to include current and next state:

typedef OnEnter = bool Function(
  BuildContext context, 
  GoRouterState currentState,
  GoRouterState nextState
);

2. Deprecate top-level redirect in favor of OnEnter.

Let me know if you'd like any adjustments to this approach.

[omar-hanafy]

Hi @chunhtai,

I've expanded the OnEnter callback again to include a GoRouter parameter to handle early-stage navigation. This addition addresses an edge case where navigation is needed before the InheritedGoRouter is available in the widget tree (particularly during initial app launch and deep linking).

The implementation:

• Adds GoRouter as the fourth parameter to OnEnter
• Internally adds new _fallbackRouter to be passed to the OnEnter callback when goRouter instance is not yet available through context.

[cedvdb]

The callback signature makes sense to me.

The fallback router makes less sense to me. Since it's not the right instance, why would one use it ? Could you add its usage in the example ?

[omar-hanafy]

Hi @cedvdb, The callback signature is indeed straightforward. The fallback router isn’t ideal in that it isn’t coming directly from the widget tree (via GoRouter.of(context)), but it serves as a backup when the Inherited widget isn’t available yet—typically on app startup. In those cases, if you try to look up the router from context, you’d get null because the tree isn’t fully built. Instead, the fallback (usually provided as this when constructing the parser) lets your onEnter callback still have a router reference so you can, for example, call router.go(...) to redirect or modify navigation.

Here’s an example usage:

GoRouter(
  onEnter: (context, currentState, nextState, router) {
    // Even if GoRouter.of(context) is null (because the tree isn’t built yet),
    // the fallback router will be non-null.
    if (nextState.uri.path == '/referral') {
      // Use the router instance (either from context or fallback) to navigate
      router.go('/handle-referral');
      return false; // Prevent default navigation.
    }
    return true;
  },
  // other config...
)

So while it isn’t “the right instance” from context when the tree isn’t ready, it’s our best–available router reference. If someone’s using the router during startup (or in any context where the inherited widget isn’t available), the fallback router provides a safe way to still programmatically navigate.

The idea is that the fallback isn’t a “dummy” or incomplete instance—it’s the same router instance (typically provided as this when constructing the parser). Its methods (like go) work independently of whether the router is currently available in the widget tree. So you can safely call router.go('/handle-referral'); withuot causing errors, even if it wasn’t obtained from the widget context (correct me if I'm wrong).

If you think this fallback might lead to unexpected behavior, one option is to document it clearly and advise that it’s only intended as a temporary solution until the tree is built. Otherwise, it’s our practical solution for early navigation calls.

[cedvdb]

typically provided as this

My bad, I misread the code and thought the fallback router was a parameter of the Router constructor, that we had to define ourselves, but that was the RouterInformationParser constructor I was reading.

I will try this out on a decently sized project

[cedvdb]

Imo onEnter should be made asynchronous instead. One may query a local cache inside for example.

I sent a PR on your fork that doesn't break the synchronous future usage

[omar-hanafy]

@cedvdb thanks for the review, I've reviewed the changes in my last commit, and I believe I've resolved the issues effectively:

• Converting onEnter to be asynchronous to lets us handle operations like cache queries without blocking, also added test case for that.
• reverted the deprecation on redirectLimit and added support for redirect limit for onEnter, and added test case for the redirectionLimit with onEnter.
• created new _processOnEnter and re-organized the parseRouteInformationWithDependencies.
• changed fallbackRouter to router.

@chunhtai Regarding the discussion about using an enum (or even a class-based resolution) for a more expressive API, I would recommend we stick with the current asynchronous onEnter design. It is inherently stateless, works better with web navigation paradigms, and avoids the complications of stateful redirection. We can plan for future enhancements—if we decide that a more nuanced resolution (using an enum) is needed—but for now, this approach is a good improvement over the old redirect mechanism.

Look at the changes guys and let me know ur thoughts.

[cedvdb]

Both those tests should be added:

• a test where router.go is used inside onEnter (which returns false since it handled navigation)
• a test where router.push is used inside onEnter

The documentation should also be available on the public API (GoRouter)

[cedvdb]

@omar-hanafy The current implementation of on enter is breaking push

I changed the example found in example/stateful_shell_route.dart to add a floating action button that pushes a sign in page when pressed to produce a repro. The behaviour is not the same when onEnter is provided.

See this example

See the below example, this is the modified content of example/stateful_shell_route.dart. Try with onEnter commented and with onEnter uncommented, you'll see that the sign in page displays the back button only when onEnter is commented.

To display the sign-in page, press the floating action button.

// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

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

final GlobalKey<NavigatorState> _rootNavigatorKey =
    GlobalKey<NavigatorState>(debugLabel: 'root');
final GlobalKey<NavigatorState> _sectionANavigatorKey =
    GlobalKey<NavigatorState>(debugLabel: 'sectionANav');

// This example demonstrates how to setup nested navigation using a
// BottomNavigationBar, where each bar item uses its own persistent navigator,
// i.e. navigation state is maintained separately for each item. This setup also
// enables deep linking into nested pages.

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

/// An example demonstrating how to use nested navigators
class NestedTabNavigationExampleApp extends StatelessWidget {
  /// Creates a NestedTabNavigationExampleApp
  NestedTabNavigationExampleApp({super.key});

  late final GoRouter _router = GoRouter(
    navigatorKey: _rootNavigatorKey,
    initialLocation: '/a',
    // onEnter: (BuildContext context, GoRouterState currentState,
    //     GoRouterState nextState, GoRouter goRouter) {
    //   print('on enter ${nextState.uri.toString()}');
    //   if (nextState.uri.toString().contains('/b')) {
    //     _router.push('/sign-in');
    //     return false;
    //   }
    //   return true;
    // },
    routes: <RouteBase>[
      GoRoute(
        path: '/sign-in',
        name: 'sign-in',
        parentNavigatorKey: _rootNavigatorKey,
        builder: (BuildContext context, GoRouterState state) => Scaffold(
          appBar: AppBar(
            title: const Text('Sign in'),
          ),
          body: const Center(child: Text('Sign in')),
        ),
      ),
      // #docregion configuration-builder
      StatefulShellRoute.indexedStack(
        builder: (BuildContext context, GoRouterState state,
            StatefulNavigationShell navigationShell) {
          // Return the widget that implements the custom shell (in this case
          // using a BottomNavigationBar). The StatefulNavigationShell is passed
          // to be able access the state of the shell and to navigate to other
          // branches in a stateful way.
          return ScaffoldWithNavBar(
            navigationShell: navigationShell,
            floatingActionButtonLocation: FloatingActionButton(
              onPressed: () {
                _router.push('/sign-in');
              },
              child: const Icon(Icons.login),
            ),
          );
        },
        // #enddocregion configuration-builder
        // #docregion configuration-branches
        branches: <StatefulShellBranch>[
          // The route branch for the first tab of the bottom navigation bar.
          StatefulShellBranch(
            navigatorKey: _sectionANavigatorKey,
            routes: <RouteBase>[
              GoRoute(
                // The screen to display as the root in the first tab of the
                // bottom navigation bar.
                path: '/a',
                builder: (BuildContext context, GoRouterState state) =>
                    const RootScreen(label: 'A', detailsPath: '/a/details'),
                routes: <RouteBase>[
                  // The details screen to display stacked on navigator of the
                  // first tab. This will cover screen A but not the application
                  // shell (bottom navigation bar).
                  GoRoute(
                    path: 'details',
                    builder: (BuildContext context, GoRouterState state) =>
                        const DetailsScreen(label: 'A'),
                  ),
                ],
              ),
            ],
            // To enable preloading of the initial locations of branches, pass
            // 'true' for the parameter `preload` (false is default).
          ),
          // #enddocregion configuration-branches

          // The route branch for the second tab of the bottom navigation bar.
          StatefulShellBranch(
            // It's not necessary to provide a navigatorKey if it isn't also
            // needed elsewhere. If not provided, a default key will be used.
            routes: <RouteBase>[
              GoRoute(
                // The screen to display as the root in the second tab of the
                // bottom navigation bar.
                path: '/b',
                builder: (BuildContext context, GoRouterState state) =>
                    const RootScreen(
                  label: 'B',
                  detailsPath: '/b/details/1',
                  secondDetailsPath: '/b/details/2',
                ),
                routes: <RouteBase>[
                  GoRoute(
                    path: 'details/:param',
                    builder: (BuildContext context, GoRouterState state) =>
                        DetailsScreen(
                      label: 'B',
                      param: state.pathParameters['param'],
                    ),
                  ),
                ],
              ),
            ],
          ),

          // The route branch for the third tab of the bottom navigation bar.
          StatefulShellBranch(
            routes: <RouteBase>[
              GoRoute(
                // The screen to display as the root in the third tab of the
                // bottom navigation bar.
                path: '/c',
                builder: (BuildContext context, GoRouterState state) =>
                    const RootScreen(
                  label: 'C',
                  detailsPath: '/c/details',
                ),
                routes: <RouteBase>[
                  GoRoute(
                    path: 'details',
                    builder: (BuildContext context, GoRouterState state) =>
                        DetailsScreen(
                      label: 'C',
                      extra: state.extra,
                    ),
                  ),
                ],
              ),
            ],
          ),
        ],
      ),
    ],
  );

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

/// Builds the "shell" for the app by building a Scaffold with a
/// BottomNavigationBar, where [child] is placed in the body of the Scaffold.
class ScaffoldWithNavBar extends StatelessWidget {
  /// Constructs an [ScaffoldWithNavBar].
  const ScaffoldWithNavBar({
    required this.navigationShell,
    this.floatingActionButtonLocation,
    Key? key,
  }) : super(key: key ?? const ValueKey<String>('ScaffoldWithNavBar'));

  /// The navigation shell and container for the branch Navigators.
  final StatefulNavigationShell navigationShell;

  final Widget? floatingActionButtonLocation;

  // #docregion configuration-custom-shell
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      // The StatefulNavigationShell from the associated StatefulShellRoute is
      // directly passed as the body of the Scaffold.
      body: navigationShell,
      floatingActionButton: floatingActionButtonLocation,
      bottomNavigationBar: BottomNavigationBar(
        // Here, the items of BottomNavigationBar are hard coded. In a real
        // world scenario, the items would most likely be generated from the
        // branches of the shell route, which can be fetched using
        // `navigationShell.route.branches`.
        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: navigationShell.currentIndex,
        // Navigate to the current location of the branch at the provided index
        // when tapping an item in the BottomNavigationBar.
        onTap: (int index) => navigationShell.goBranch(index),
      ),
    );
  }
  // #enddocregion configuration-custom-shell

  /// NOTE: For a slightly more sophisticated branch switching, change the onTap
  /// handler on the BottomNavigationBar above to the following:
  /// `onTap: (int index) => _onTap(context, index),`
  // ignore: unused_element
  void _onTap(BuildContext context, int index) {
    // When navigating to a new branch, it's recommended to use the goBranch
    // method, as doing so makes sure the last navigation state of the
    // Navigator for the branch is restored.
    navigationShell.goBranch(
      index,
      // A common pattern when using bottom navigation bars is to support
      // navigating to the initial location when tapping the item that is
      // already active. This example demonstrates how to support this behavior,
      // using the initialLocation parameter of goBranch.
      initialLocation: index == navigationShell.currentIndex,
    );
  }
}

/// Widget for the root/initial pages in the bottom navigation bar.
class RootScreen extends StatelessWidget {
  /// Creates a RootScreen
  const RootScreen({
    required this.label,
    required this.detailsPath,
    this.secondDetailsPath,
    super.key,
  });

  /// The label
  final String label;

  /// The path to the detail page
  final String detailsPath;

  /// The path to another detail page
  final String? secondDetailsPath;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text('Root of section $label'),
      ),
      body: Center(
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            Text('Screen $label',
                style: Theme.of(context).textTheme.titleLarge),
            const Padding(padding: EdgeInsets.all(4)),
            TextButton(
              onPressed: () {
                GoRouter.of(context).go(detailsPath, extra: '$label-XYZ');
              },
              child: const Text('View details'),
            ),
            const Padding(padding: EdgeInsets.all(4)),
            if (secondDetailsPath != null)
              TextButton(
                onPressed: () {
                  GoRouter.of(context).go(secondDetailsPath!);
                },
                child: const Text('View more details'),
              ),
          ],
        ),
      ),
    );
  }
}

/// The details screen for either the A or B screen.
class DetailsScreen extends StatefulWidget {
  /// Constructs a [DetailsScreen].
  const DetailsScreen({
    required this.label,
    this.param,
    this.extra,
    this.withScaffold = true,
    super.key,
  });

  /// The label to display in the center of the screen.
  final String label;

  /// Optional param
  final String? param;

  /// Optional extra object
  final Object? extra;

  /// Wrap in scaffold
  final bool withScaffold;

  @override
  State<StatefulWidget> createState() => DetailsScreenState();
}

/// The state for DetailsScreen
class DetailsScreenState extends State<DetailsScreen> {
  int _counter = 0;

  @override
  Widget build(BuildContext context) {
    if (widget.withScaffold) {
      return Scaffold(
        appBar: AppBar(
          title: Text('Details Screen - ${widget.label}'),
        ),
        body: _build(context),
      );
    } else {
      return ColoredBox(
        color: Theme.of(context).scaffoldBackgroundColor,
        child: _build(context),
      );
    }
  }

  Widget _build(BuildContext context) {
    return Center(
      child: Column(
        mainAxisSize: MainAxisSize.min,
        children: <Widget>[
          Text('Details for ${widget.label} - Counter: $_counter',
              style: Theme.of(context).textTheme.titleLarge),
          const Padding(padding: EdgeInsets.all(4)),
          TextButton(
            onPressed: () {
              setState(() {
                _counter++;
              });
            },
            child: const Text('Increment counter'),
          ),
          const Padding(padding: EdgeInsets.all(8)),
          if (widget.param != null)
            Text('Parameter: ${widget.param!}',
                style: Theme.of(context).textTheme.titleMedium),
          const Padding(padding: EdgeInsets.all(8)),
          if (widget.extra != null)
            Text('Extra: ${widget.extra!}',
                style: Theme.of(context).textTheme.titleMedium),
          if (!widget.withScaffold) ...<Widget>[
            const Padding(padding: EdgeInsets.all(16)),
            TextButton(
              onPressed: () {
                GoRouter.of(context).pop();
              },
              child: const Text('< Back',
                  style: TextStyle(fontWeight: FontWeight.bold, fontSize: 18)),
            ),
          ]
        ],
      ),
    );
  }
}

I will investigate in the upcoming days.

Edit okay, I figured why (the type was hardcoded to "go"), will submit a PR on your fork with the fix

[omar-hanafy]

Hey @cedvdb,

Thanks for diving into this issue. I’ve been looking into it too, and I ended up exploring a couple of potential solutions.

in addition to what u found I tried to pass the type and the completer using the infoState by passing it to the _processOnEnter method, but I found that there is an interesting use case I did not think of while impl the logic.

because I was mixing an imperative push (which is asynchronous and adds a new route on top) with a return value of false (which signals that the original navigation should be canceled), the fallback ends up “resetting” things in a way that causes the pushed route to become part of the underlying stack unexpectedly.

My Investigations and Ideas:

Option 1: Tracking Imperative Navigations

One workaround I explored involves enhancing the existing logic by tracking when an imperative push occurs. The idea is to:

• Introduce a Counter and Store: We add a counter (e.g., _imperativePushCount) and store the latest route match list (_latestImperativeMatchList).
• Update on Push: Every time an imperative push (or pushReplacement) is called, we increment the counter and update the stored match list.
• Modify Fallback Logic: In the _processOnEnter method, if onEnter returns false and we detect that one or more imperative pushes have occurred (via the counter), we simply return the latest match list instead of triggering the fallback that’s hardcoded with NavigatingType.go.

This way, multiple imperative navigations can be properly accounted for without the fallback interfering with the intended push.

Option 2: A New NavigationDecision API

Alternatively, we can change the onEnter API itself to be more expressive. Rather than returning a bool, the onEnter callback could return a NavigationDecision.

For example:

enum NavigationDecisionType {
  allow, // Proceed with the original navigation.
  block, // Block the original navigation (use fallback).
  navigate, // Perform a specific navigation action.
}

class NavigationDecision {
  final NavigationDecisionType type;
  final String? location; // For go, push, replace
  final Object? extra;    
  final Map<String, String>? pathParameters;
  final Map<String, dynamic>? queryParameters;
  final String? name; // for Named routes

  // Constructors for various decisions:
  NavigationDecision.allow() 
      : type = NavigationDecisionType.allow, 
        location = null, extra = null, pathParameters = null, queryParameters = null, name = null;

  NavigationDecision.block() 
      : type = NavigationDecisionType.block, 
        location = null, extra = null, pathParameters = null, queryParameters = null, name = null;

  NavigationDecision.go(String this.location, {this.extra})
      : type = NavigationDecisionType.navigate, pathParameters = null, queryParameters = null, name = null;

  NavigationDecision.push(String this.location, {this.extra})
      : type = NavigationDecisionType.navigate, pathParameters = null, queryParameters = null, name = null;

  NavigationDecision.replace(String this.location, {this.extra})
      : type = NavigationDecisionType.navigate, pathParameters = null, queryParameters = null, name = null;

  NavigationDecision.goNamed(String this.name, {this.pathParameters = const {}, this.queryParameters = const {}, this.extra})
      : type = NavigationDecisionType.navigate, location = null;

  NavigationDecision.pushNamed(String this.name, {this.pathParameters = const {}, this.queryParameters = const {}, this.extra})
      : type = NavigationDecisionType.navigate, location = null;

  NavigationDecision.replaceNamed(String this.name, {this.pathParameters = const {}, this.queryParameters = const {}, this.extra})
      : type = NavigationDecisionType.navigate, location = null;
}

And then change the onEnter signature to:

typedef OnEnter = FutureOr<NavigationDecision> Function(
  BuildContext context,
  GoRouterState currentState,
  GoRouterState nextState,
);

With this approach, the developer can explicitly state:

• Allow: to proceed normally,
• Block: to cancel the navigation (triggering fallback), or
• Navigate: to perform an imperative action (go, push, or replace).

This makes the decision clear and avoids any ambiguity between triggering a navigation imperatively and the fallback behavior.

let me knwo your thoughts on these options.

Personally, I lean towards the NavigationDecision approach since it makes the intent explicit and sidesteps the complexity of tracking imperative pushes internally. But the counter approach could serve as a solid interim solution.

[cedvdb]

because I was mixing an imperative push (which is asynchronous and adds a new route on top) with a return value of false (which signals that the original navigation should be canceled), the fallback ends up “resetting” things in a way that causes the pushed route to become part of the underlying stack unexpectedly.

This is a bit hard to understand. Could you give more details / an example where you are experiencing this issue ?

In the first PR I sent on your repository for async, I did not have issues with push. (it seems like you answered on the wrong commit, all tests were passing).

NavigationDecision API

The point of onEnter is being able to hijack the navigation flow to run arbitrary navigation inside on enter, you may even navigate multiple times, it does not make assumptions. I'm against your solution 2 as it prevents that.

I need to understand the underlying issue to provide meaningful feedback / alternative approaches though

[feinstein]

Why is this not following the design doc for guards? I feel like we are deprecating redirect to get something similar.

We need independent guards that can deal with futures, and be able to deviate to a new path then come back (redirect to login, then continue the deeplink). See how auto router does this.

[cedvdb]

Why is this not following the design doc for guards? I feel like we are deprecating redirect to get something similar

We are definitely not getting something similar here as this allow hijacking the redirection process and do arbitrary navigation.

and be able to deviate to a new path then come back (redirect to login, then continue the deeplink)

You can do it in a way that wouldn't break on the web by using a query parameter (/signIn?onSuccess=redirectUrl). The example provided on auto route readme seems like it would break on the web if you refresh on the login page. The question is whether there is value in allowing designs that do not handle all cases by returning a class that people can extend or if it's better to force a stateless design. Imo it should be forced, since it's not immediately obvious.

@omar-hanafy please see if you are hitting your issue with this version, I worry it is due to how you dealt with asynchronousity to begin with and that it does not require any of the solutions you are thinking about.

https://github.com/cedvdb/flutter_packages/tree/on_enter

[feinstein]

In my apps when I open a deeplink I need to:

1. Check if the splash screen has being shown, as some long initialisations are made in the splash screen.
2. Check if the user is logged in, login in or register in case he's not, with forgot password working too.
3. Validate if the current user can access the deeplink.

All of those guards are async, and need to deviate the navigation to their own navigations (login/registration can have many screens), then they need to come back to the original navigation.

Using query strings will make this very complicated, cumbersome, hard to maintain, having redirects all over the place, which can be completely unmaintainable as the possibilities of alternative flows accumulate.

We need a mechanism to allow us to show something on screen during async gaps, while also enabling to continue, stop, deviate and resume the navigation. AutoRouter has something like this IIRC.

I understand the web might break if the page is refeshed, but extra isn't also compatible with the Web IIRC, so it's not a new thing to break compatibility when things can be better for mobile.

Mobile apps are the main platforms, and it should get the best experience there. Can we adapt to the Web in other ways? How the best Web navigation frameworks deal with this? Do we need to make 2 different flows so we get the best from both worlds?

[feinstein]

Also, I still don't get why you are not implementing guards, as it was proposed by @chunhtai.

With a list of guards we can separate each flow into its own unit, it's a lot better for reusabilily and maintainability.

Following my previous example, I would have a SplashScreenGuard, AuthGuard, PaymentGuard, etc.

Using only one callback will mix all of this into one place, which makes the code messier.

[cedvdb]

We need a mechanism to allow us to show something on screen during async gaps, while also enabling to continue, stop, deviate and resume the navigation.

There is nothing preventing you from doing that in the on enter (it is async, now). You could have:

onEnter: (_, _, _, router) {
   loadingScreen.show();
   // .. do some stuff
   loadingScreen.hide();
}

Using query strings will make this very complicated, cumbersome, hard to maintain, having redirects all over the place, which can be completely unmaintainable as the possibilities of alternative flows accumulate.

At the cost of breaking if an user refresh the page (when running on the web) ? Genuine question

but extra isn't also compatible with the Web IIRC

Extra is also a mistake in my opinion, even if it's convenient in some cases. The name is an hint.

That being said, as long as it won't prevent running arbitrary navigation code prior to entering a route, I'm personally not against returning a class, my navigation code will still be stateless. I believe it's the wrong direction, but if it's useful to some, I can't judge.

I still don't get why you are not implementing guards

That's one of the changes I requested above. I'm with you there

[omar-hanafy]

@cedvdb I think I was handling the method _processOnEnter wrong.

I have refactored things and took some insights from ur fork which was working, also enhanced the redirection handling, and added separate test file for onEnter with new test cases including recursive redirections.

[feinstein]

I don't think that returning classes is the way to go, this will limit the redirects to finish when we return. AutoRouter has a handler with methods, we call the methods to continue, reject, etc. This allows us to deviate and come back to where we stopped, like if we created a new temporary navigation stack that's going to be discarded once the guard is done with it. Stateless will increase the complexity of our code, can't we maintain stateful on the Web even with refreshes? I honestly don't know, there are so many new APIs in browsers these days.

I don't have web experience, but my gut feeling tells me we should do some research on how the Web routing frameworks deal with these scenarios, as lots of Web apps have deeplinks with guards. Maybe I will be surprised to know that they don't offer these mechanisms, or maybe we will discover a mechanism that can suit all our needs?

[feinstein]

@chunhtai this seems to be a big update, maybe you want to review what's being made?

[cedvdb]

I don't think that returning classes is the way to go, this will limit the redirects to finish when we return. AutoRouter has a handler with methods, we call the methods to continue, reject, etc. This allows us to deviate and come back to where we stopped, like if we created a new temporary navigation stack that's going to be discarded once the guard is done with it. Stateless will increase the complexity of our code, can't we maintain stateful on the Web even with refreshes? I honestly don't know, there are so many new APIs in browsers these days.

I don't have web experience, but my gut feeling tells me we should do some research on how the Web routing frameworks deal with these scenarios, as lots of Web apps have deeplinks with guards. Maybe I will be surprised to know that they don't offer these mechanisms, or maybe we will discover a mechanism that can suit all our needs?

When a refresh happen, the runtime is lost and restarts, so the navigation state (in this case what's the future destination) needs to be saved somewhere. It can be in the URL, in local storage, etc. For all intent and purpose the url is the practical place to place that state.

Having said that I assumed you were talking about this kind of guard from auto_route:

class AuthGuard extends AutoRouteGuard {

  @override
  void onNavigation(NavigationResolver resolver, StackRouter router) {
    // the navigation is paused until resolver.next() is called with either
    // true to resume/continue navigation or false to abort navigation
    if(authenticated) {
      // if user is authenticated we continue
      resolver.next(true);
    } else {
        // we redirect the user to our login page
        // tip: use resolver.redirect to have the redirected route
        // automatically removed from the stack when the resolver is completed
        resolver.redirect(
          LoginRoute(onResult: (success) {
            // if success == true the navigation will be resumed
            // else it will be aborted
            resolver.next(success);
          },
        );
      );
    }
  }
}

Which I guess something similar can be achieved with the current implementation like so:

onEnter: (context, current, next, router) async {
   if(await isAuthenticated()) {
      // if user is authenticated we continue
      return true;
   } else {
      router.push(LoginRoute.routePath).then((loggedIn) {
        // use the result from pop()
        if (loggedIn) router.go(next.uri.toString());
      });
      return false;
   }
}

This assumes you'd pop with a return value.

I'm currently using my fork with:

onEnter: (context, current, next, router) async {
   if(await isAuthenticated()) {
      // if user is authenticated we continue
      return true;
   } else {
      router.push(LoginRoute.routePath, queryParameters: { redirectToKey: next.uri.toString() });
      return false;
   }
}

Which does essentially the same thing, while being stateless. ( I have multiple levels of registration as well, with different types of users, using query string is the standard on the web and not as bad as you made it sound). This is miles ahead of what go_router currently provides with redirects, because it lets you do whatever you want. I could show a loader, navigate 3 times, etc, go router wouldn't get in the way.

[feinstein]

I agree it's better than what we have now, but once this lands, we won't be changing it any time soon, so I rather have it covering as many cases as possible.

The docs say that extra goes through serialisation on the Web, so can't we do something similar to maintain the state of the redirect?

I have a hard time accepting returns as the best solution bc once we return we are done, while calling handlers, we can have many options, all the time. It creates a much more powerful API.

[cedvdb]

it's better than what we have now, but once this lands, we won't be changing it any time soon, so I rather have it covering as many cases as possible..

agreed, this one shouldn't be rushed. Imo once we are happy with the state of this PR, we should ask for others in the related issues to try it out for a period of time before landing it

The docs say that extra goes through serialisation on the Web, so can't we do something similar to maintain the state of the redirect?

I did not know that, I edited that part on my comment above. With this:

onEnter: (context, current, next, router) async {
   if(await isAuthenticated()) {
      // if user is authenticated we continue
      return true;
   } else {
      router.push(LoginRoute.routePath).then((loggedIn) {
        if (loggedIn) router.go(next.uri.toString());
      });
      return false;
   }
}

Wouldn't that fit the bill ? You can pop with a result value. This assumes push is used though.

With guards instead it gives:

guards: [
  Guard(onEnter: context, current, next, router) async {
     if(await isAuthenticated()) {
        // if user is authenticated we continue
        return GuardResult.next;
     } else {
        router.push(LoginRoute.routePath).then((loggedIn) {
          if (loggedIn) router.go(next.uri.toString());
        });
        return GuardResult.stop;
     }
  })
]

As you said in the design doc, this would restart the guard chain, but how is that a problem ?

The auto_route API seems to lock you in whatever is implemented behind resolver.redirect, which is what guards are trying to solve in the first place (allow to choose how redirection is handled), we should definitely avoid opinionated solutions here. Do you have a concrete API in mind that would not be as opinionated ?

[cedvdb]

@omar-hanafy

I will have the time to try it in tomorrow, as a flyby review (I can help with those requests, sorry to be iterative but it would have been too many changes at once otherwise):

1. As also mentioned by @feinstein, the original design was a list of onEnter as a guard list. It would avoid friction to have a list. What I'd suggest to be closer to the design doc is:

• renaming on_enter.dart to guard.dart
• moving OnEnter type declaration wich is still in configuration.dart in guard.dart
• renaming OnEnterHandler to _GuardHandler (which implies the use of part to keep _GuardHandler private)
• adding the following Guard abstract to guard.dart

abstract class Guard {
   Future<bool> onEnter(
     BuildContext context,
     GoRouterState currentState,
     GoRouterState nextState,
     GoRouter goRouter,
   );
}

class CallbackGuard extends Guard {
  final OnEnter _onEnter;
  SimpleGuard({ required OnEnter onEnter}): _onEnter = onEnter;

   @override
   Future<bool> onEnter(
     BuildContext context,
     GoRouterState currentState,
     GoRouterState nextState,
     GoRouter goRouter,
   ) {
      _onEnter(context, currentState, nextState, goRouter);
   }
}

and change the signature of GoRouter to accept a List<Guard> instead of a single one.

GoRouter(
  guards: [ 
     CallbackGuard(onEnter: (ctx, current, next, router) {
        // on enter logic
     })
  ]
  // instead of onEnter:
)

2. After using it multiple times, I still have to refer to the doc every time to see if I should return false or true (wait, does true mean stop or is it false ?), I find it that annoying. An enum (.stop, .next), even if binary, would be a lot easier to use imo, as it more descriptive what the intent is.

3. Tests missing

• with go usage:

Future<bool>isAuthenticated() => Future.value(false);

onEnter: (context, current, next, router) async {
     if(await isAuthenticated()) {
        // if user is authenticated we continue
        return GuardResult.next;
     } else {
        router.go('/sign-in');
        return GuardResult.stop;
     }
  })

// expects that we are on /sign-in

• with push usage:

onEnter: (context, current, next, router) async {
     if(await isAuthenticated()) {
        // if user is authenticated we continue
        return GuardResult.next;
     } else {
        router.push('/sign-in');
        return GuardResult.stop;
     }
  })

// expects that if isAuthenticated is false it pushes the sign in route
// expects that if pop() is used on top of that, we are back on the initial route

• GoRouter should reexecute the guarding logic when router.refresh is used. I suspect this one will fail.

What do you think ? Would you like a PR on your fork for any of those ?

[feinstein]

I prefer enums too.

Just to add a bit more info, I usually use a user stream in my apps. Whenever there's a change in login state, the stream emits a new user or null. I connect this stream into the redirect, so the redirect is re-evaluated whenever the login state changes. So it's very important for the guards to run when the listener has a change.

I also have a whitelist of pages that can be accessed without login, otherwise I will redirect to login forever while on login pages (login, registration, forgot password, splash screen, etc). I don't think this changes much your example, but this adds complexity to the general guard logic, as we need to be always thinking how the guard will run for each particular page, especially when navigating during a guard, to satisfy the guard (e.g. Guard redirects to login, then user goes to the forgot password page, then gets a new code, then logs in, then the redirect needs to work and continue to the original target).

I wish the guard could simplify how we deal with redirects, so we don't have to manually manage the original deeplink destination as guards get triggered all over again while inside the login flow. This can get complex to deal with.

[omar-hanafy]

@cedvdb

and change the signature of GoRouter to accept a List<Guard> instead of a single one.

I actually prefer keeping it as a single guard (or onEnter) for now. I haven’t encountered a concrete case where multiple guards are essential. A simpler API reduces complexity, and as users start adopting it, we’ll likely receive feature requests that can guide any future expansion. I believe it’s better to start simple and then iterate.

An enum (.stop, .next)

I agree—using an enum would make the intent much clearer. It’s more self‑documenting than a bare boolean. also let me suggest some alternatives for naming:

• For continuation: .allow, .proceed, or .continue
• For stopping: .block, .reject, or .deny

onEnter: (context, current, next, router) async { if(await isAuthenticated()) { // if user is authenticated we continue return GuardResult.next; } else { router.push('/sign-in'); return GuardResult.stop; } })

I believe our current design already covers these scenarios. The redirection limit and asynchronous handling take care of the general behavior let me check again ... did u mean to have spicifc case for router.push('...') and router.go('...')?

• GoRouter should reexecute the guarding logic when router.refresh is used. I suspect this one will fail.

That's a great point—I hadn't fully considered the refresh scenario. We need to ensure that a router.refresh triggers the guard logic appropriately so that any changes in authentication state or similar conditions are rechecked.

Thanks for your suggestions if you’d like to open a PR, I’d be happy to review it.

[omar-hanafy]

@feinstein

so we don't have to manually manage the original deeplink destination as guards get triggered all over again while inside the login flow. This can get complex to deal with.

In many ways, this complexity already exists when using the top-level redirect callback—onEnter/Guard doesn’t remove that complexity so much as it gives you more flexibility. By letting you run asynchronous checks and call push or go from within the guard, you can handle deep links or multi-step flows in ways that the old redirect approach couldn’t easily support. But with that extra flexibility does come some added responsibility.

If you want a simpler guard pattern (e.g., just checking whether a route needs auth or not), you can still do it with a straightforward if (authenticated) return true; else return false;. More involved flows—like a deep link that leads to a login screen, then forgot password, etc.—require the app to “remember” the original target and handle re-navigation once the user logs in. Whether you’re using a redirect or onEnter, you’ll still need to track that state, decide when the guard has done its job, and pass control back to the router.

Essentially, onEnter is a superset of the old redirect, offering more options for complex scenarios while still allowing a simple approach when that’s all you need.

[feinstein]

I know, I don't disagree with this, my point is more about if we can make it better and remove this control from the devs. It's very hard to manage these "original destinations" when you are navigating between guards and the current destination changes all the time.

I understand your solution is better than what we have now, I am just pushing for us to have something that's not just better, but actually perfect for all use case with good dev experience, removing the burden from devs and allowing all the control we need.

[cedvdb]

Maybe It should trigger on pop as well ?

[omar-hanafy]

Do you mean onEnter should be fired when calling context.pop()? Why? It does not seem logical to me that exiting a route should trigger onEnter.