Add a locator function to GoRouterState

[letsar]

This is the complete implementation of the solution suggested in #184. It would enable users to be able to get dependencies from any redirect callback through state.locator.

[csells]

This PR has no README update that describes the problem and how this is the solution.

[letsar]

Right, I've just add a section about that in the README.

[csells]

Sorry. I also realized that there's no example.

[letsar]

No problem, I've updated the PR to add an example under redirection_with_locator.dart. What do you think about this solution?

[csells]

I have to dig into it. you may well have fixed one of the problems w/ redirection.

[lulupointu]

If I may, I don't really like this.

As I explained in https://github.com/csells/go_router/issues/184 it is possible for the redirect to get a context, it just need some refactoring. I don't see how adding 1 attribute + 1 new concept is better.

What's more and most importantly, I don't even see when it would be useful to have the context (or locator) in the redirect callback since you already have a context in the State holding GoRouter.

Here is your example, is which I removed the `locator`

```dart import 'package:flutter/material.dart'; import 'package:go_router/go_router.dart'; import 'package:provider/provider.dart'; import 'shared/data.dart'; import 'shared/pages.dart'; final loginInfo = LoginInfo(); void main() => runApp( ChangeNotifierProvider.value( value: loginInfo, child: const App(), ), ); /// sample app using a context.read to redirect to another location class App extends StatefulWidget { const App({Key? key}) : super(key: key); @override State createState() => _AppState(); } class _AppState extends State { @override Widget build(BuildContext context) => MaterialApp.router( routeInformationParser: _router.routeInformationParser, routerDelegate: _router.routerDelegate, title: 'GoRouter Example: Redirection with Locator', debugShowCheckedModeBanner: false, ); late final _router = GoRouter( routes: [ GoRoute( path: '/', pageBuilder: (context, state) => MaterialPage( key: state.pageKey, child: HomePage(families: Families.data), ), routes: [ GoRoute( path: 'family/:fid', pageBuilder: (context, state) { final family = Families.family(state.params['fid']!); return MaterialPage( key: state.pageKey, child: FamilyPage(family: family), ); }, routes: [ GoRoute( path: 'person/:pid', pageBuilder: (context, state) { final family = Families.family(state.params['fid']!); final person = family.person(state.params['pid']!); return MaterialPage( key: state.pageKey, child: PersonPage(family: family, person: person), ); }, ), ], ), ], ), GoRoute( path: '/login', pageBuilder: (context, state) => MaterialPage( key: state.pageKey, child: const LoginPage(), ), ), ], errorPageBuilder: (context, state) => MaterialPage( key: state.pageKey, child: ErrorPage(state.error), ), // redirect to the login page if the user is not logged in redirect: (state) { final loginInfo = context.read(); final loggedIn = loginInfo.loggedIn; final goingToLogin = state.location == '/login'; // the user is not logged in and not headed to /login, they need to login if (!loggedIn && !goingToLogin) return '/login'; // the user is logged in and headed to /login, no need to login again if (loggedIn && goingToLogin) return '/'; // no need to redirect at all return null; }, // changes on the listenable will cause the router to refresh it's route refreshListenable: loginInfo, ); } ```

All I had to do is:

1. Remove the locator
2. Replace every locator by context.read

Am I missing something?

[letsar]

The real power of having the locator available though GoRouterState is to be able to have it in any GoRoute's redirect.

Let's say that you declare some route in a different file from where GoRouter is instantiated, how would you pass context.read in that case?

[letsar]

@lulupointu I've updated the example to demonstrate that this is solution is more flexible. I made the redirect function, a top-level function where we can't use context.

[nosmirck]

I like your solution, the only drawback is that you can access only stuff immediately provided the same level and above the MaterialApp (since that's where usually we declare the GoRouter).

This prevents us from using the context to get something that is injected deeper in the tree, however, it shouldn't be an issue since when on web you force a url it starts the app from scratch and whatever you had in your tree is reset.

I'll switch to this branch and test a few scenarios and see what happens.

My main concern is what happens in this case:

• we have a list of items
• we can open a details page for each item (which creates a new bloc for it)
• depending on some permission we can edit this item, the permission is in the item's model, accessible through the ItemDetailsBloc

If we think of the routes like this: /items <-- items page /items/details/42 <-- details page for item with id 42 /items/details/42/edit <-- edit page for item with id 42

For mobile I don't have an issue since I hide the edit button given the item's flag to be able to edit or not, however, on web I can force the url.

Unless I have to put checks everywhere, to prevent editing the item (more on front end and backend to return error) it would be great to handle a redirect to the edit route by checking the ItemDetailsBloc's state, see if the flag allows edit or not. If it's not allowed I can simply return the location without the /edit part which forces the user to stay in the details page. Even when they force the url.

[letsar]

I like your solution, the only drawback is that you can access only stuff immediately provided the same level and above the MaterialApp (since that's where usually we declare the GoRouter).

This prevents us from using the context to get something that is injected deeper in the tree

Yes that's true, but in the context of redirection I believe that such data should be shared among multiple pages and thus be higher in the tree.

For mobile I don't have an issue since I hide the edit button given the item's flag to be able to edit or not, however, on web I can force the url.

I'm not yet familiar to all the difference between flutter web and mobile, but do you mean that on web you can't rely on your block to enable/disable a button?

I'll switch to this branch and test a few scenarios and see what happens.

Great! Let us know if there are edge cases complicated to do with this solution.

[nosmirck]

Yes that's true, but in the context of redirection I believe that such data should be shared among multiple pages and thus be higher in the tree.

Not really, the scenario I proposed is very common (a bloc for a deep page that is instantiated and disposed only while using it, imagine a movie details page, an audio playback screen, a book reading page, etc. You create a new instance of a bloc per item when visiting that item's page.

I'm not yet familiar to all the difference between flutter web and mobile, but do you mean that on web you can't rely on your block to enable/disable a button?

Exactly, on mobile, there's no way to force a navigation other that triggering an event (like the press of a button) placed by the developer. On web, you can force your way in by typing stuff in the url, it's been like that since the internet exists.

Great! Let us know if there are edge cases complicated to do with this solution.

I just tested the scenario when I have something like this

/ /items /items/:id/details /items/:id/details/edit

In my details page builder I wrap my MaterialPage with a Provider (it's a simple string with the value of the id)

Then when I go to the edit page, the redirect should check for the above provided string it throws that it can't find it.

[letsar]

@csells can you describe why this PR has been closed? I though you believed it was a good solution.

[csells]

Sorry, @letsar . This PR was closed when I moved the default branch from master to main. However, as far as the PR itself, I'm with @lulupointu when he said: https://github.com/csells/go_router/pull/187#issuecomment-977259068

[letsar]

Ok, so you agree with him to refactor go router, so that we can have access to the BuildContext inside the redirect method? Or you need me to create a more complex example where it would not be easy to replace locator with context?

[csells]

I think that adding context to redirect would require the refactor he describes, yes. I'm not yet convinced that we need to add context to redirect however.

[letsar]

In our case we really need it. We create our routes outside of the widget tree and thus cannot have access to the context in the redirect. In the redirect we need to get access to an object available in the widget tree to act on the redirect logic. What do you suggest us to do to get our object in the redirect callback?

[Miiite]

For large applications I'm with @letsar on that aspect. If you have to manage/create dozens of routes for your application, you are going to figure out a way to split route creation into multiple files/classes.

When you do that, if you need to implement a "redirect" function inside one of these routes, I don't think passing around the BuildContext is a viable solution. Plus, when you use a Context-related solution for DI like Provider or RIverpod, you have to get a recent BuildContext object to be able to find a reference you'd want.

For example in your docs there is a great use case of the redirection logic:

redirect: (state) {
      final loggedIn = loginInfo.loggedIn;
      final goingToLogin = state.location == '/login';

      // the user is not logged in and not headed to /login, they need to login
      if (!loggedIn && !goingToLogin) return '/login';

      // the user is logged in and headed to /login, no need to login again
      if (loggedIn && goingToLogin) return '/';

      // no need to redirect at all
      return null;
    },

But to me that example is not a real-world example. In a project using Provider or Riverpod, what you are willing to write is more something like this:

redirect: (state) {
      final loggedIn = **context.read<LoggedInfo>().loggedIn**;
      final goingToLogin = state.location == '/login';

      // the user is not logged in and not headed to /login, they need to login
      if (!loggedIn && !goingToLogin) return '/login';

      // the user is logged in and headed to /login, no need to login again
      if (loggedIn && goingToLogin) return '/';

      // no need to redirect at all
      return null;
    },

And right now we can't do that "easily", unless I've missed something ?

[lulupointu]

@Miiite GoRouterState lives in a StatefulWidget. So if your provider is placed above the StatefulWidget containing GoRouterState, you can use its context to access context.read<LoggedInfo>().

Here is an example (click to show)

```dart import 'package:flutter/material.dart'; import 'package:go_router/go_router.dart'; import 'package:provider/provider.dart'; void main() { runApp( ChangeNotifierProvider( create: (_) => AuthState(auth: MockAuthentication()), child: BooksApp(), ), ); } class Credentials { final String username; final String password; Credentials(this.username, this.password); } abstract class Authentication { Future isSignedIn(); Future signOut(); Future signIn(String username, String password); } class MockAuthentication implements Authentication { bool _signedIn = false; @override Future isSignedIn() async { return _signedIn; } @override Future signOut() async { _signedIn = false; } @override Future signIn(String username, String password) async { return _signedIn = true; } } class AuthState extends ChangeNotifier { final Authentication _auth; bool? _isSignedIn = null; AuthState({required Authentication auth}) : _auth = auth; Future initialize() async { await Future.delayed(Duration(seconds: 2)); _isSignedIn = false; notifyListeners(); } Future signIn(Credentials credentials) async { var success = await _auth.signIn(credentials.username, credentials.password); _isSignedIn = success; notifyListeners(); return success; } Future signOut() async { await _auth.signOut(); _isSignedIn = false; notifyListeners(); } bool? get isSignedIn => _isSignedIn; } class BooksApp extends StatefulWidget { @override State createState() => _BooksAppState(); } class _BooksAppState extends State { @override void initState() { super.initState(); context.read().initialize(); } @override Widget build(BuildContext context) => MaterialApp.router( routeInformationParser: _router.routeInformationParser, routerDelegate: _router.routerDelegate, title: 'GoRouter Example: LogIn', ); late final _router = GoRouter( refreshListenable: context.watch(), redirect: (state) { final isSignedIn = context.read().isSignedIn; if (isSignedIn == null && state.location != '/loading') { return '/loading'; } if (isSignedIn == false && state.location != '/sign-in') { return '/sign-in'; } if (isSignedIn == true && (state.location == '/loading' || state.location == '/sign-in')) { return '/'; } }, routes: [ GoRoute( path: '/', pageBuilder: (_, state) => MaterialPage( key: state.pageKey, child: HomeScreen(onSignOut: context.watch().signOut), ), routes: [ GoRoute( path: 'books', pageBuilder: (_, state) => MaterialPage( key: state.pageKey, child: BooksListScreen(), ), ), ], ), GoRoute( path: '/sign-in', pageBuilder: (_, state) => MaterialPage( key: state.pageKey, child: SignInScreen(onSignedIn: context.watch().signIn), ), ), GoRoute( path: '/loading', pageBuilder: (_, state) => MaterialPage( key: state.pageKey, child: Center(child: Text('Loading signin state')), ), ), ], errorPageBuilder: (context, state) => MaterialPage( key: state.pageKey, child: Text('${state.error}'), ), ); } class AuthStateUpdateHandler extends StatefulWidget { final Widget child; final AuthState authState; AuthStateUpdateHandler({required this.child, required this.authState}); @override State createState() => _AuthStateUpdateHandlerState(); } class _AuthStateUpdateHandlerState extends State { void _onAuthStateChange() { final isSignedIn = widget.authState.isSignedIn; if (isSignedIn == null) return context.go('/loading'); context.go(isSignedIn ? '/' : '/log-in'); } @override void initState() { super.initState(); widget.authState.addListener(_onAuthStateChange); } @override void dispose() { widget.authState.removeListener(_onAuthStateChange); super.dispose(); } @override Widget build(BuildContext context) => widget.child; } class HomeScreen extends StatelessWidget { final VoidCallback onSignOut; HomeScreen({required this.onSignOut}); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(), body: Center( child: Column( children: [ ElevatedButton( onPressed: () => context.go('/books'), child: Text('View my bookshelf'), ), ElevatedButton( onPressed: onSignOut, child: Text('Sign out'), ), ], ), ), ); } } class SignInScreen extends StatefulWidget { final ValueChanged onSignedIn; SignInScreen({required this.onSignedIn}); @override _SignInScreenState createState() => _SignInScreenState(); } class _SignInScreenState extends State { String _username = ''; String _password = ''; @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(), body: Center( child: Column( children: [ TextField( decoration: InputDecoration(hintText: 'username (any)'), onChanged: (s) => _username = s, ), TextField( decoration: InputDecoration(hintText: 'password (any)'), obscureText: true, onChanged: (s) => _password = s, ), ElevatedButton( onPressed: () => widget.onSignedIn(Credentials(_username, _password)), child: Text('Sign in'), ), ], ), ), ); } } class BooksListScreen extends StatelessWidget { BooksListScreen(); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar(), body: ListView( children: [ ListTile( title: Text('Stranger in a Strange Land'), subtitle: Text('Robert A. Heinlein'), ), ListTile( title: Text('Foundation'), subtitle: Text('Isaac Asimov'), ), ListTile( title: Text('Fahrenheit 451'), subtitle: Text('Ray Bradbury'), ), ], ), ); } } ```

Honestly I still fail to see a use case where this feature would be useful. But do show me a concrete example to prove me wrong.

[letsar]

@lulupointu I think you missed the point where the routes are defined outside of the widget tree. I agree with you if the entire GoRouter is defined in a StatefulWidget. In that case there is no need to to be able to get the context through the redirect parameters. But if you deport some routes in another file and outside of the widget tree you can no longer use the context of the surrounding StatefulWidget to do that.

Let's take the example of a big app with different modules where each module has the responsibility to manage their own routes. Each module would inject them somehow in an object that would be used by the GoRouter to define all the routes. In the routes defined by each module, we don't have access to a BuildContext, and thus it would be very helpful to have a way to get it from the redirect parameters.

[lulupointu]

@letsar I have already given a solution to this here.

[letsar]

@lulupointu I already answered to that solution, which involves a lot of refactoring (in our case) and adds a lot of complexity. Passing the BuildContext to every intermediates has the same drawbacks as passing data through all widgets until the one that really needs it and I really want to avoid that.

[nosmirck]

I think that adding context to redirect would require the refactor he describes, yes. I'm not yet convinced that we need to add context to redirect however.

Then the redirect callback is almost useless. If we can't use it for more complex checks, what's the purpose of it then?

The app I'm developing uses permissions to allow/deny user's interaction with certain features (for instance, we have managers that can create accounts) I can add as many checks as I need to my UI to avoid the users getting into the users creation page, however, if a user taps in the url in the browser to go to that page, nothing can stop them.

Now, I can add checks on the page to see if the user has the required permissions and either redirect them to the home screen or show an error, but then, what's the purpose of the redirect? the basic example shows using a loginInfo notifier that simply holds the state of the user's login and is used to redirect the user to the login page if forcing the url to home, that's great, and I want to do the same for more complex stuff. Why should I change that?

I mean, why can I work with the redirect just for something simple as the login example for this package but it's not easy to do for more complex usecases.

I have to either have a singleton class holding the user's state (with all permissions, login info and all) and use that on the redirect methods I add on each page, and so far that's ok, but then, what happens with other more complex scenarios like the one I described in my previous comment? where the information that drives the condition for the redirect is in a widget deep down the tree? then the redirect becomes even more useless, because I can't have easy access to what I need. If I use a service locator like get_it, then I can use that to find the instance of the bloc I need, but I don't want to do that since I already have all my architecture defined using provider and bloc, or in the case I had to do that, I would just then go with getx and use the middlewares and the Get.find (locator) to simply use what I need from it, but there's a reason I'm not using getx because I don;t want to depend on a single package for everything, it's just a time bomb.

I understand that for go_router there's no easy way to do this, it might not even be possible at all, but at least having access to the context right above the declaration of the material app should be possible and easy to add.