csells / go_router

The purpose of the go_router for Flutter is to use declarative routes to reduce complexity, regardless of the platform you're targeting (mobile, web, desktop), handling deep linking from Android, iOS and the web while still allowing an easy-to-use developer experience.
https://gorouter.dev
441 stars 97 forks source link

App flashes the initial location prior to actually deep linking to correct route. #139

Closed mlars84 closed 2 years ago

mlars84 commented 3 years ago

I just started re-writing my app's routing to go_router and so far everything is great, docs are great, etc. The only issue I am having is that when attempting to deep link (or even just link to another route at all) the initial location always flashes first when opening a new tab. E.g., initial route is '/login'. Open a new tab and paste in http://localhost:5000/#/inbox for example and login screen will flash before re-directing to '/inbox'. I've re-created this with the example as well.

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

import 'shared/pages.dart';

void main() => runApp(App());

/// sample class using simple declarative routes
class App extends StatelessWidget {
  App({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) => MaterialApp.router(
        routeInformationParser: _router.routeInformationParser,
        routerDelegate: _router.routerDelegate,
        title: 'GoRouter Example: Declarative Routes',
      );

  final _router = GoRouter(
    initialLocation: '/',
    routes: [
      GoRoute(
        path: '/',
        pageBuilder: (context, state) => MaterialPage<void>(
          key: state.pageKey,
          child: const Page1Page(),
        ),
      ),
      GoRoute(
        path: '/page2',
        pageBuilder: (context, state) => MaterialPage<void>(
          key: state.pageKey,
          child: const Page2Page(),
        ),
      ),
    ],
    errorPageBuilder: (context, state) => MaterialPage<void>(
      key: state.pageKey,
      child: ErrorPage(state.error),
    ),
  );
}

Take the above code, open a new tab and paste in http://localhost:5000/#/page2 or http://localhost:5000/#/asdf and you will see a flash of the home screen prior to being properly re-directed.

Browser is Chrome 95.0.4638.69 on a Mac OS running 11.6.

csells commented 3 years ago

The intialRoute isn't overridden with the deep link? Let me take a look.

leoshusar commented 3 years ago

I believe I had similar issue, but my problem was with redirection to /init with ?from=/some-deep-link for after init redirection. It behaved like this:

Log looks like this (I added DateTime.now() at the top of the redirect so you can see what happens when):

GoRouter: setting initial location /
[00:41:56.324] state.location: /
GoRouter: redirecting to /init?redirect=/
[00:41:56.326] state.location: /init?redirect=/
GoRouter: deep linking to /users
[00:41:56.599] state.location: /users
GoRouter: redirecting to /init?redirect=/users
[00:41:56.599] state.location: /init?redirect=/users
GoRouter: going to /users
[00:41:56.613] state.location: /users

I also found this. There's described exactly this behavior but on iOS platform, and on iOS this behavior is correct. My guess is it works the same on web?

csells commented 3 years ago

I'm unable to duplicate the described behavior. deep linking to /page2 works as expected -- it goes right to that page w/o flashing any other pages.

https://user-images.githubusercontent.com/2568253/141213794-3a523c4f-e3af-456d-8389-cc6ad833b5de.mov

leoshusar commented 3 years ago

Interesting, I can see it in your example.

https://user-images.githubusercontent.com/13891066/141215526-603a6d0f-3127-4df9-ad28-b2d2f8391a99.mp4

https://user-images.githubusercontent.com/13891066/141215532-2650bc3d-946d-4b87-8c53-690cff774679.mp4

This is on latest commit in master branch. I'm on Flutter 2.7.0-3.1.pre, if that's anyhow useful.

csells commented 3 years ago

to which example are you referring? it doesn't happen in the code you posted above that I can see.

leoshusar commented 3 years ago

I'm not OP, I just came to provide more input :)

OP used example from here and that's also what I ran and recorded.

csells commented 3 years ago

Neither of these videos opens up a new tab and goes to the deep link as described by OP.

leoshusar commented 3 years ago

The point of opening a new tab is because this is happening only on "cold start". It's the same as just refreshing.

https://user-images.githubusercontent.com/13891066/141220446-79f2c603-6a01-476c-8882-a9cb247a32a1.mp4

Here also slowed down so you can see how it exactly matches log messages

https://user-images.githubusercontent.com/13891066/141221121-097f9069-c5a0-4384-bc65-466d0ca5d086.mp4

jhlabs commented 3 years ago

I believe I have the same issue. I will describe my use case here. I have an app where the user logs in with Google:

  1. The user starts on the login page
  2. The user gets redirected to Google Auth page, service, exchange tokens, etc.
  3. The user gets redirected to http://localhost:5000/#/web-login-callback?access_token=XXX in the flutter app

But when I log out the router state in the global redirect function, the first location that is called is /, the initial route. Only after that is the user redirected to the original path without any query params.

@csells maybe you could confirm the issue by printing out the routes that are being handled by the redirect function? You might also see the initial route as the first one, even though it should deep link directly into the app.

lulupointu commented 2 years ago

What is the issue

This issue is due to the fact that RouterDelegate.setInitialRoutePath in asynchronous and is therefore not called before the first build. This means that during the first build, GoRouterDelegate always uses GoRouter.initialLocation. This can be viewed by adding print(matches) in GoRouterDelegate._builder.

So this is indeed linked only to "cold-start" which can be triggered either by navigating in a new tab or hitting the browser reload button.

Why this issue does not always appear

This issue is tricky because it only appears when the number of pages shown by GoRouter.initialLocation defers from the one that should be shown when deep-linking. This is what happens in @leoshusar example:

What is the solution

Fortunatly, there is an easy solution: Make GoRouterDelegate.setInitialRoutePath synchronous ! To achieve this we need to:

  1. Return a SynchronousFuture from GoRouterDelegate.setInitialRoutePath
  2. Return a SynchronousFuture from GoRouteInformationParser Point 1 is easy to deduce from Flutter RouterDelegate.setInitialRoutePath however I had to dive in the Flutter implementation to see that using RouteInformationParser.SynchronousFuture was also needed :wink:
csells commented 2 years ago

should be fixed in v2.2.7. @mlars84 and @lulupointu can you verify?

lulupointu commented 2 years ago

It's working as expected in v2.2.7 on my side.