lulupointu / vrouter

MIT License
202 stars 38 forks source link

VGuard with multiple unauth & auth redirect conditions (how to? infinite loop?) #74

Closed Frank-D closed 3 years ago

Frank-D commented 3 years ago

Hi @lulupointu, like many others, amazing project, thank you.

First of all, sorry for the super long description, I wanted to make sure you have all the info to understand my issue, but as soon as you get my point, just skip all my explanation and go below to the simplified code snippet I've provided, thanks!:)

So I'm trying to use VGuard in my flutter mobile app to protect all my authenticated screens, by trying to conduct a few different checks, and wondering if I'm going in the wrong direction and missing the point entirely..

Basically, I'm trying to achieve 3 specific things with the VGuard: 1- Check if user is authenticated. If not, redirect back to [unauthenticated] login screen (classic, it's in all your examples, and working very well). 2- If user authenticated (previous check 1-), then check if user has verified email. If not, redirect to [authenticated] verify email screen. 3- If user is authenticated (1-) and has verified email (2-), then check if user profile is complete. If not, redirect to [authenticated] user setup screen. 4- ELSE, when 1/2/3 checks are all OK (basically, all false/skipped), then any requested authenticated page like /home should be served to the user.

The problem that I'm having with this approach, is that for checks 2- and 3-, when any of those checks fails, it will redirect to an authenticated page as highlighted above (..because those 2 pages should only be accessed if user is authenticated..). When this redirect is happening, it will trigger the same VGuard checks again, causing by default an infinite loop..oh-oh.

(ex.: User is coming from unauthenticated /login screen, and logs in successfully. However, user email is not yet verified, and [authenticated] /home screen is requested. VGuard will fail check 2-, so a redirect to [authenticated] /user_verify_email screen will be triggered at the end of this first VGuard execution. This will then trigger the same VGuard to execute again, a 2nd time (..this is just the beginning of the infinite loop, since by default, my same Vguard check that failed in previous vguard execution will fail again on 2nd vguard execution, and again and again on 3rd, 4th...infinite loop!).

Of course, if I move those 2 screens (UserVerifyEmailPage & UserSetupPage) from the protected vguard section to the "unauthenticated / non-protected" vrouter section, it will work np. However, I would like to keep those screens protected for a few reasons..

One way I found to solve this situation, was to find a way that the 2nd time the VGuard is executed (after its first execution ended up redirecting to another [auth] screen after check 2- or 3- failed..), the check that failed during first execution should now ideally be successful to avoid the infinite loop, so I managed to solve this by checking if this 2nd time around, the "vRedirector.to" equals that requested redirection screen route set during VGuard first execution, and if so, I make the entire check to fail and not execute again, which prevents the infinite loop..

But I feel like this is not the right way to solve this, this seems a bit odd to me to build such "exit" logic based on the "vRedirector.to" value (..and I feel like this could easily be hacked, especially on the web if I end up deploying the same app online as well..). Ideally, when any of my VGuard check fails, I would have preferred to have a way to redirect to an authenticated page "directly" (~ vRedirector.push('/some_authenticated_page', bypassAllVGuards: true)), without going all over again on that VGuard logic, which could easily cause such infinite loop.. (I'm sure there are better solutions to solve this, this is just to help understand the concern; 'bypassAllVGuards' is probably a very bad idea!..).

Here's a simplified version of my existing code: (fyi, I'm using an older version of VRouter at the time of this writing, e.g. ^1.1.0+22. I'm planning to update to latest soon, but I wouldn't think it impacts my situation described above...)

...
...
Widget createMaterialApp(BuildContext context) {
  return VRouter( // ~ MaterialApp
    routes: [
      VNester(
        path: null,
        widgetBuilder: (child) => createCommonRootTreeWidgets(child), // Initializes a few things for the app...
        nestedRoutes: [
          // -----------------------------
          // NON-Protected routes/screens:
          // -----------------------------
          // *ROOT* ('/')
          VRouteRedirector(
              path: Routes.root,
              redirectTo: Routes.login,
          ),
          // *Login Screen*
          VWidget(path: Routes.login, widget: LoginPage()),
          VGuard(
            // Protects all pages that require user to be authenticated + email verified + profile complete.
            beforeEnter: (vRedirector) async {
              if (!isUserAuthenticated(context)) {
                vRedirector.push(Routes.login); // non-protected root
              } else if (!isUserEmailVerified(context, vRedirector)) {
                vRedirector.push(Routes.user_verify_email); // protected route...oh-oh...could cause infinite loop!
              } else if (!isUserProfileComplete(context, vRedirector)) {
                vRedirector.push(Routes.user_setup); // protected route...oh-oh...could cause infinite loop!
              }
            },
            stackedRoutes: [
              // -------------------------
              // Protected routes/screens:
              // -------------------------
              VWidget(path: Routes.user_verify_email, widget: UserVerifyEmailPage()),
              VWidget(path: Routes.user_setup, widget: UserSetupPage()),
              VWidget(path: Routes.home, widget: HomePage()),
              // ...
              // ...
            ],
          ),
        ],
      )
    ],
  );
}

bool isUserAuthenticated(BuildContext context) {
  return context.read<AppAuthBloc>().state.status == AuthStatus.authenticated;
}

bool isUserEmailVerified(BuildContext context, VRedirector vRedirector) {
  return (vRedirector.to == Routes.user_verify_email) || // *** THIS LINE IS THE HACKY SOLUTION TO MAKE 2ND VGUARD EXECUTION TO WORK AND AVOID THE INFINITE WHILE LOOPS OF REDIRECTION...
      context.read<AppAuthBloc>().state.user.emailVerified;
}

bool isUserProfileComplete(BuildContext context, VRedirector vRedirector) {
  // TODO: replace "false" with real ~isUserProfileComplted logic...
  return (vRedirector.to == Routes.user_setup || vRedirector.to == Routes.user_verify_email) // *** THIS LINE IS THE HACKY SOLUTION TO MAKE 2ND VGUARD EXECUTION TO WORK AND AVOID THE INFINITE WHILE LOOPS OF REDIRECTION...
      || false;
}
...
...

Do you have any suggestion on how I could handle/design such VGuard logic better? Thanks

Frank-D commented 3 years ago

Question #2 (it's somewhat related to previous question IMO..):

Once I am "inside" a vguard stacked route/page/vwidget, interestingly enough, navigating from one authenticated/vguard page to another does no longer trigger my VGuard checks. Is this expected behaviour? Would probably be nice to add this little but important detail in the doc, since I was expecting that ootb my vguard checks would "always" execute as soon as I request a route from the vguard stacked routes, even if I'm already inside one of its route and asking another internal vguard route. This way, if something happens for whatever reasons, and user profile state change from completed back to incomplete (my app should not let this happen, but still, I was relying on the VGuard to always ensure that the user profile is complete, if not, then redirect to the user setup page to let the user complete his/her profile...).

ex.: From UserVerifyEmailPage, clicking on the below button:

ElevatedButton(
    onPressed: () => context.vRouter.push(Routes.home), // why does this vrouter-push bypasses the Vguard logic entirely?
    child: Text("Home"),
)

...will redirect the user to the Home() (/home) screen directly, without executing any Vguard logic, so even if user email is still 'not' verified, and even if user profile is still 'not' complete.

But that was the goal of my vguard checks, to ensure the user can not access Home (/home) screen and all the screens under, until user is [ authenticated + email verified + profile complete ]. So in this above example, since user is on the UserVerifyEmailPage because his/her email still not verified, then requesting /home (through that fake/tmp button just to test things) should have made Vrouter, imo, to execute the Vguard logic again since /home is under such Vguard's responsibility, and the vguard logic check 2- should have failed again, and user should have been redirected back onto the exact same page in the end, so from UserVerifyEmailPage to UserVerifyEmailPage (even though user requested, from UserVerifyEmailPage, to go to Home)...but Home was served directly, unfortunately. And I don't want to have to go add such redirecting logic/checks on ALL my app pages, this wouldn't scale.

lulupointu commented 3 years ago

First, thanks for the very detailed explanations! If there is something that makes me want to help someone it's this :wink:.

I'll answer your 2nd question (https://github.com/lulupointu/vrouter/issues/74#issuecomment-850769347) first, since I don't really need to write code: VGuard.beforeEnter is triggered only before the VGuard is entered. When the VGuard is already is the routes stack and is pushed trough another time, VGuard.beforeUpdate is called. You can read it in the description of the function (if you go inside VGuard), here on vrouter.dev or here on pub.dev/vrouter. This is useful because some actions needs only to be performed once, such as data fetching for example. You are not the first not noticing VGuard.beforeUpdate so if you have an idea of how to improve this I'll be glad to hear it (you can come to the discord server for more casual discussions like this)

For your first question, let me write a bit of code and come back to you, but you are on the right track :wink:

lulupointu commented 3 years ago

Here is response to question 1 (https://github.com/lulupointu/vrouter/issues/74#issue-906347218).

For your particular setup you have 2 options:

  1. Use VRedirector.to
  2. Use VRouteElementBuilder

1. Use VRedirector.to

As I see it, you are already on the right track. Using VRedirector.to is what I had in mind and what I use most of the time. THe only thing I would change is your check logic.

Here is VGuard:

VGuard(
  // Protects all pages that require user to be authenticated + email verified + profile complete.
  beforeEnter: userCheck,
  beforeUpdate: userCheck, // You NEED this as explained in https://github.com/lulupointu/vrouter/issues/74#issuecomment-850783473
  stackedRoutes: [
    // -------------------------
    // Protected routes/screens:
    // -------------------------
    VWidget(path: Routes.user_verify_email, widget: UserVerifyEmailPage()),
    VWidget(path: Routes.user_setup, widget: UserSetupPage()),
    VWidget(path: Routes.home, widget: HomePage()),
    // ...
    // ...
  ],
)

Here are your is... functions:

Future<void> userCheck(VRedirector vRedirector) async {
  if (!isUserAuthenticated(context)) {
    vRedirector.push(Routes.login); // non-protected root
  } else if (!isUserEmailVerified(context, vRedirector)) {
    if (vRedirector.to == Routes.user_verify_email)
      return; // Prevent looping, we are already in the right place
    vRedirector.push(Routes.user_verify_email);
  } else if (!isUserProfileComplete(context, vRedirector)) {
    if (vRedirector.to == Routes.user_setup)
      return; // Prevent looping, we are already in the right place
    vRedirector.push(Routes.user_setup);
  }
}

bool isUserAuthenticated(BuildContext context) {
  return context.read<AppAuthBloc>().state.status == AuthStatus.authenticated;
}

bool isUserEmailVerified(BuildContext context) {
  return context.read<AppAuthBloc>().state.user.emailVerified;
}

bool isUserProfileComplete(BuildContext context) {
  // TODO: replace "false" with real ~isUserProfileComplted logic...
  return false;
}

as you saw, I did not really change anything but the logic of your checks. However I did you VGuard.beforeUpdate as per what I explained above

2. Use VRouteElementBuilder

If you have never heard of VRouteElementBuilder check this from pub.dev/vrouter or that from vrouter.dev.

Imo, for your specific case, this is what I would go for because it might be easier to read. But since you like using functions rather than classes I'm not sure this will be appealing to you. Here it is anyway.

The solution here is to have 3 VGuard: one for Routes.user_verify_email, one for Routes.user_setup and one for the others. The three of them will basically have the same checks but the first one will not implement isUserAuthenticated and the second one will not implement isUserEmailVerified.

Since this is fastidious and hard to maintain, a good solution is to use a reusable VRouteElement. To do so, you must create your own VRouteElement using VRouteElementBuilder.

Here is your custom VRouteElement:

class VUserCompleteGuard extends VRouteElementBuilder {
  final BuildContext context;
  final bool shouldCheckAuthenticated;
  final bool shouldCheckEmailVerified;
  final bool shouldCheckProfileComplete;
  final VRouteElement stackedRoute;

  VUserCompleteGuard(
    this.context, {
    required this.stackedRoute,
    this.shouldCheckAuthenticated = true,
    this.shouldCheckEmailVerified = true,
    this.shouldCheckProfileComplete = true,
  });

  @override
  List<VRouteElement> buildRoutes() => [
        VGuard(
          stackedRoutes: [stackedRoute],
          beforeEnter: (vRedirector) async {
            if (shouldCheckAuthenticated && !isUserAuthenticated) {
              return vRedirector.push(Routes.login);
            }
            if (shouldCheckEmailVerified && !isUserEmailVerified) {
              return vRedirector.push(Routes.user_verify_email);
            }
            if (shouldCheckProfileComplete && !isUserProfileComplete) {
              return vRedirector.push(Routes.user_setup);
            }
          },
        )
      ];

  bool get isUserAuthenticated =>
      context.read<AppAuthBloc>().state.status == AuthStatus.authenticated;

  bool get isUserEmailVerified => context.read<AppAuthBloc>().state.user.emailVerified;

  bool get isUserProfileComplete => false;
}

Here is how to use it in your routes:

VRouter(
  // ~ MaterialApp
  routes: [
    VNester(
      path: null,
      widgetBuilder: (child) => createCommonRootTreeWidgets(child),
      // Initializes a few things for the app...
      nestedRoutes: [
        // -----------------------------
        // NON-Protected routes/screens:
        // -----------------------------
        // *ROOT* ('/')
        VRouteRedirector(
          path: Routes.root,
          redirectTo: Routes.login,
        ),
        // *Login Screen*
        VWidget(path: Routes.login, widget: LoginPage()),

        // -------------------------
        // Protected routes/screens:
        // -------------------------
        VUserCompleteGuard(
          context,
          shouldCheckEmailVerified: false,
          stackedRoute:
              VWidget(path: Routes.user_verify_email, widget: UserVerifyEmailPage()),
        ),
        VUserCompleteGuard(
          context,
          shouldCheckProfileComplete: false,
          stackedRoute: VWidget(path: Routes.user_setup, widget: UserSetupPage()),
        ),
        VUserCompleteGuard(
          context,
          stackedRoute: VWidget(path: Routes.home, widget: HomePage()),
        ),
        // ...
        // ...
      ],
    )
  ],
);

This definitely reads better according to me but this is personal taste really.

Also note that you don't need VGuard.beforeUpdate here because since the 3 routes have 3 different instances of you custom VRouteElement, VGuard.beforeEnter will be called when navigating between each other. However note that if you want your checks to be executed even when navigating inside your custom VGuard (as I expect this might be the case if you have several routes and not just Home and want to check when navigating between them).

Why VRedirector.to is a secure approach

Both approaches are as safe as the other. But you had you doubt about VRedirector.to so let me address this in particular. Using a confition such as if (vRedirector.to == Routes.user_verify_email) return; is very secure because in the worst case scenario, this won't be true while it should and the user will loop. What I mean is that this is very restrictive since only the url Routes.user_verify_email will match and if it does not the user will be redirected.

Conclusion

Thanks again for the well written issue, this was a pleasure. Hope you have your answers and if any more question don't hesitate to ask šŸ˜Š

Frank-D commented 3 years ago

Oh wow, what a fast reply with so much details, much appreciated! (I've seen that same amazing level of support in all the other issues as well, that definitely helps keeping this project super-damn-healthy!)

And if others can benefit from this one more advanced VGuard use case, then I'm glad, because the other issues from others also really helped me figuring out some other things, especially when I started using this lib, so it was super helpful, and those well-described issues / Q&As definitely act as extra/bonus documentation of real-life scenarios, imo, so this is great.


Alright, let's go back to the details now:) Let me start with easiest point 1 first in this comment. Will post another comment for point 2.

  1. VGuard.beforeEnter vs VGuard.beforeUpdate Amazing thanks for your answer on this, I knew there was probably something I could do by leveraging the proper step of the navigation lifecycle, but I had read both docs you provided (either here on vrouter.dev or here on pub.dev/vrouter ) and it still wasn't crystal clear for me, from the terms used to describe those cycles. To be honest, I find your response here in this thread on that topic to be much better;) e.g.:
    • beforeEnter is called only the very first time user access any of the guarded routes contained within a given vguard, but then after, if user navigates from one guarded route to another, the vguard beforeEnter condition(s) will NO longer be executed.
    • (..vs "4. Call beforeEnter in the [VRouter]" or "// Triggered when a route in stackedRoutes is first displayed")
    • --> I find point 4. to not really help me, not sure what the "in the [VRouter]" really means, or how this is suppose to help me figuring out that it will be called first time entering the vguard, but not the other times from one vguard page to another..:)
    • --> I think for the other vguard-related doc/explanation, it's better, but the "...is first displayed" is what I found confusing, and I see multiple ways to interpret it, e.g., when moving from one guarded stackedRoute to another, say from page 1 to page 2, then page 2 is also the "first time it gets displayed" technically, so that is why I found that explanation a bit confusing..anyways, just sharing how my brain read your doc, if that helps..;)
    • beforeUpdate, as opposed to beforeEnter, is called whenever a user navigates from one stacked guarded route to another. Simple!
    • (...vs "6. Call beforeUpdate in all reused [VRouteElement]" or "// Triggered when a route in stackedRoutes is displayed but changes")
    • --> I also find here point 6. to not really help me, not sure what "in all reused [VRouteElement]" really means, there might be a different way to explain this perhaps so that newcomers to VRouter will understand this concept faster:), and of course if sometimes you can't find the right words to describe it, sometimes an real simple example in the doc under the same section and right after the initial explanation will provide everything people need to really understand the explanation/concept.. (ex.: step 1- user goes on un-protected login page, this is what gets call based on our above vrouter config..., step 2- user logs in successfully and navigates to guarded /home screen, this will trigger 'beforeEnter' and this that...etc, step 3- user then navigates to another guarded route /settings, this time only 'beforeUpdate' will execute.......etc etc, an example like this in the doc could be helpful perhaps...especially around understanding better the different lifecycles.
    • --> For the other vguard explanation, I find hard to interpret "..is displayed but changes". Now that I understand it form your previous comment, I read again this explanation and I sort of understand it, but without your explanation in this issue I would not understand it and that's actually what happened to me:). I think the focus should be more about the routes "transition" then talking about what is currently 'displayed' no? For me, this is as simple as saying that this 'beforeUpdate' logic will get executed whenever user transition/navigates FROM one guarded route to another guarded route of the same Vguard stacked routes.
    • however here, I would probably need to do some more quick testing, but when user 'first' navigates to a guarded route and 'beforeEnter' executes, will beforeUpdate "ALSO" execute, or not?

Thanks (and thanks for inviting me to that vrouter discord.com channel btw, I'll keep that in mind for next time..;))

Frank-D commented 3 years ago

Ok, and now, regarding point 2! Again, thank you so much for ALL the details that you replied back to me for that point, included code examples etc, I could not have asked for more! (and was expecting much less, and it would have been fine too!;))

Alright, so thanks for sharing the 2 options/solutions.

I like them both actually, I fond pros and cons for both.

Option 1: Keep all the details inside the Vrouter config/routes, 'inline' if I can say.

Option 2: Move/abstract routing details by leveraging VRouteElementBuilder

Having written down now those pros and cons for both options, I'm still considering both at this time, should make my choice shortly though, since I want to progress on all this and move on now..;)

If I look at your option 2 code snippet (much much appreciated), I feel like it could possibly be even simplified and not have to expose to the ~outside world those shouldCheck... booleans. Here's what it could be maybe:

class VUserCompleteGuard extends VRouteElementBuilder {
  // *** update: removed all "shouldCheck..." bool vars definition...
  final BuildContext context;
  final VRouteElement stackedRoute;

  VUserCompleteGuard(
    this.context, {
    required this.stackedRoute,
    // *** update: removed all "shouldCheck" vars initialization...
  });

  @override
  List<VRouteElement> buildRoutes() => [
        VGuard(
          stackedRoutes: [stackedRoute],
          beforeEnter: (vRedirector) async {
            if (!isUserAuthenticated) {   // ***update: removed the unnecessary 'shouldCheck..' here, since we're navigating user outside the vguard anyways, hence no infinite-loop possible..
              return vRedirector.push(Routes.login);
            }
            if (stackedRoute !=  Routes.user_verify_email && !isUserEmailVerified) {  // ***update: replaced 'shouldCheckEmailVerified' with that first if-check..
              return vRedirector.push(Routes.user_verify_email);
            }
            if (stackedRoute !=  Routes.user_setup && !isUserProfileComplete) { // ***update: replaced 'shouldCheckEmailVerified' with that first if-check..
              return vRedirector.push(Routes.user_setup);
            }
          },
        )
      ];

  bool get isUserAuthenticated =>
      context.read<AppAuthBloc>().state.status == AuthStatus.authenticated;

  bool get isUserEmailVerified => context.read<AppAuthBloc>().state.user.emailVerified;

  bool get isUserProfileComplete => false;
}

And here is the updated VRouter config:

VRouter(
  // ~ MaterialApp
  routes: [
    VNester(
      path: null,
      widgetBuilder: (child) => createCommonRootTreeWidgets(child),
      // Initializes a few things for the app...
      nestedRoutes: [
        // -----------------------------
        // NON-Protected routes/screens:
        // -----------------------------
        // *ROOT* ('/')
        VRouteRedirector(
          path: Routes.root,
          redirectTo: Routes.login,
        ),
        // *Login Screen*
        VWidget(path: Routes.login, widget: LoginPage()),

        // -------------------------
        // Protected routes/screens:
        // -------------------------
        VUserCompleteGuard(
          context, // ***update: removed the "shouldCheckEmailVerified: false" param right below..
          stackedRoute:
              VWidget(path: Routes.user_verify_email, widget: UserVerifyEmailPage()),
        ),
        VUserCompleteGuard(
          context, // ***update: removed the "shouldCheckProfileComplete: false" param right below..
          stackedRoute: VWidget(path: Routes.user_setup, widget: UserSetupPage()),
        ),
        VUserCompleteGuard(
          context,
          stackedRoute: VWidget(path: Routes.home, widget: HomePage()),
        ),
        // ...
        // ...
      ],
    )
  ],
);

Q1. I'm curious, what do you think? I think it simplifies the use of such 'VUserCompleteGuard' when looking at the VRouter routes where it's being used/referenced, so a tad cleaner/simpler.. Might just comes down to personal preference at this point...

Q2. That is probably my last question on this whole issue because you've pretty much nailed down everything super fast already(!): Let's say IF I end up going with option 2 described here right above, can I still chain all the basic VRouter components/VWidget/etc "around" those custom VRouteElementBuilder component (in this example above, precisely VUserCompleteGuard)? As you understood correctly, I will have multiple stacked routes 'under' my Home screen (/home), so how would that work? From the previous example, we're only passing a single route to the VUserCompleteGuard:

VUserCompleteGuard(
    context,
    stackedRoute: VWidget(path: Routes.home, widget: HomePage()),
),

So I guess the strategy remains the same, I simply provide to my home page its stacked routes like that:

VUserCompleteGuard(
    context,
    stackedRoute: 
        VWidget(path: Routes.home, widget: HomePage(), stackedRoutes: [
                VWidget(path: Routes.settings, widget: SettingsAccountPage()),
                ...
                ...
        ]),
),

I'm just wondering at this point how those "child" stacked routes will behave, in terms of navigation lifecycle in regards to this custom VRouteElementBuilder "VUserCompleteGuard" component.. For instance, regarding all those VWidget child screens under HomePage, once accessed by the user, will the parent "VUserCompleteGuard.beforeEnter" logic that we've defined execute, or perhaps I should add another "VUserCompleteGuard.beforeUpdate" with the same checks, similar to your previous explanation regarding that topic?:) Of course, I could probably just do some more testing and answer all those questions myself, but at the same time, if this helps others in this github channel, then everybody wins!:)

thanks again

lulupointu commented 3 years ago

Regarding point 1:

When a user navigates from one guarded route to another guarded route of the SAME VGuard stacked routes

I like this, I think I'll change to that.

I also agree that an example explaining this would be a good idea.

Reused [VRouteElement]

This sound so clear to me, I must have spent too much time in my own system šŸ˜„

Also to answer your question: for the first navigation, beforeUpdate is NOT called.

I will answer point 2 later

lulupointu commented 3 years ago

Here is the answer for the second point!

I understand you point about abstraction vs inline reading. However I still like my second option better because it's in-between: its abstract because it guard a route (you don't exactly now how by reading the class name), but it's also really explicit from the default behavior thanks to the attribute. For example when you see shouldCheckEmailVerified: false, there is no magic: it guard the route as default, but here we don't check "email verified". I don't know if I am clear but I hope so :smile: Also as you said, it's a matter of taste really.

About your question on VRouteElementBuilder, it is literally a VRouteElement and so behaves as any that you are used to. It's just that rather than using stackedRoutes you define the buildRoutes function. So if you need to have multiple routes in VUserCompleteGuard you just have to change its definition to the following:

class VUserCompleteGuard extends VRouteElementBuilder {
  final BuildContext context;
  final bool shouldCheckAuthenticated;
  final bool shouldCheckEmailVerified;
  final bool shouldCheckProfileComplete;
  final List<VRouteElement> stackedRoutes; // This is changed to a List

  VUserCompleteGuard(
      this.context, {
        required this.stackedRoutes,
        this.shouldCheckAuthenticated = true,
        this.shouldCheckEmailVerified = true,
        this.shouldCheckProfileComplete = true,
      });

  @override
  List<VRouteElement> buildRoutes() => [
    VGuard(
      stackedRoutes: stackedRoutes, // Give the list directly
      beforeEnter: (vRedirector) async {
        if (shouldCheckAuthenticated && !isUserAuthenticated) {
          return vRedirector.push(Routes.login);
        }
        if (shouldCheckEmailVerified && !isUserEmailVerified) {
          return vRedirector.push(Routes.user_verify_email);
        }
        if (shouldCheckProfileComplete && !isUserProfileComplete) {
          return vRedirector.push(Routes.user_setup);
        }
      },
    )
  ];

  bool get isUserAuthenticated =>
      context.read<AppAuthBloc>().state.status == AuthStatus.authenticated;

  bool get isUserEmailVerified => context.read<AppAuthBloc>().state.user.emailVerified;

  bool get isUserProfileComplete => false;
}

Really you can think of it like this: Widget -> StatelessWidget with Widget build(context) VRouteElement -> VRouteElementBuilder with List<VRouteElement> buildRoutes()

Therefore custom VRouteElement can be used as you use custom Widgets: Either as reusable component (as VUserCompleteGuard is, or as a way to separate you code (you could create VInsideApp which contains you routes for HomeScreen and SettingsScreen for example)

Regarding all those VWidget child screens under HomePage, once accessed by the user, will the parent "VUserCompleteGuard.beforeEnter" logic that we've defined execute, or perhaps I should add another "VUserCompleteGuard.beforeUpdate" with the same checks, similar to your previous explanation regarding that topic

So if you need a specific explanation for this: Indeed VUserCompleteGuard.beforeEnter will be called the first time a route is entered (whichever it is) and them, if you navigate between routes inside the same VUserCompleteGuard THEN VUserCompleteGuard.beforeUpdate will be called.

I hope I'm clear, please tell me if something is not !

Frank-D commented 3 years ago

Regarding exposing or not those "shouldCheck....." vars to outside the VRouteElementBuilder, I guess it indeed really comes down to personal preference. Imo, the fact that we are exposing them only to "disable" checks (as opposed to "enable/include" them), I don't find this super intuitive, if let's say another coder would be joining my project. He/she would still need to go inside that custom VRouteElementBuilder to see what checks are being performed, so in other words, setting one by one those checks to 'false' in the VRouter config, imo, does not help documenting the behaviour of my custom VRouteElementBuilder. I would usually prefer a more 'declarative' way to do it, more like option 1, where we are declaring what are 'all' the checks that will be performed, when accessing any route guarded by such vguard. Else, if we go with option 2 and that VRouteElementBuilder, since the concept is different, I have a preference for just hiding it all to VRouter and keep those checks to the internal mechanics of such VRouteElementBuilder (..because again, either I declare all checks and it becomes clear to the code reader what are all the checks being done, or else, nothing and keep this impl details inside the builder, if the only thing I'm able to display to the code reader in VRouter are what checks are disabled and when, but still does not tell what are the other checks still active/being done, so...again....just my personal pref., and it's ok to think differently right? ..;) )


Regarding the remaining of your answer, thanks a lot again, everything is super clear.....except only one little thing: What's the difference between the previous builder supporting only one route at its root, and the newer version from your last post where it supports multiple routes at its root, for the case where I would let's say need to support only one route at the root (let's say again, /home), and then all other routes will be child of that /home routes...? And no I don't have this use case in my app, I will have multiple routes side-by-side at the root of the builder, so if I end up picking option 2 for my app, I'll definitely follow your last example so thanks for that, but just for my own knowledge (and to others), any difference in overall processing/navigation between those 2 below examples (our previous version, and your latest):

  1. Previous version (builder supports only one route at its root, but I feel like that route can still have multiple child stacked routes, since it's a VWidget which supports stacked routes..):

    VUserCompleteV1Guard(
    context,
    stackedRoute: 
        VWidget(path: Routes.home, widget: HomePage(), stackedRoutes: [
                VWidget(path: Routes.settings, widget: ChildPageOne()),
                VWidget(path: Routes.settings, widget: ChildPageTwo()),
                VWidget(path: Routes.settings, widget: ChildPageThree()),
                ...
        ]),
    ),
  2. Newest version (builder supports multiple routes at its root, but I guess that If I only need one route at the root, followed by child stacked routes under that main/root route, both version 1 and 2 would work the same, is that fair to assume?):

    VUserCompleteV2Guard(
    context,
    stackedRoutes:  // *** Here, notice the 's' at the end, followed by the array brackets below, the only diffs..
    [
        VWidget(path: Routes.home, widget: HomePage(), stackedRoutes: [
                VWidget(path: Routes.settings, widget: ChildPageOne()),
                VWidget(path: Routes.settings, widget: ChildPageTwo()),
                VWidget(path: Routes.settings, widget: ChildPageThree()),
                ...
        ]),
    ],
    ),

    Obviously, version 2 allows to define multiple routes at its root all at the same level, and each can have their own child routes / stackedRoutes, right? But otherwise, from the 2 examples/versions above, the navigation processing would be exactly the same in the end, right?

lulupointu commented 3 years ago

Agree to disagree :wink:


You are completely right, absolutely 0 difference for VRouter. The only thing is that stacking routes is not the same as putting them side by side. If you stack them you can pop from the top one to the one below. So this is only dependent of your UI/UX.

Frank-D commented 3 years ago

Wonderful, thanks for everything. All clear. šŸ¤™