Change Route guard and resolver ordering to follow route parent/child hierarchy

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

Resolve blocks are processed only distinctly after all guards are finished, meaning guards cannot protect routes that rely on asynchronous data (i.e. fetching the user) without breaking the guard/resolver paradigm.

Expected behavior

Process guards before resolvers, but follow the route parent -> child hierarchy; allow parent data to be loaded before processing authorization for child routes.

Minimal reproduction of the problem with instructions

StackBlitz: https://angular-gitter-cui2tq.stackblitz.io

Click the inline link at the top, which will attempt to load the "/somewhere" route; however, since the parent's resolver does not fire before the child's guard, the user will be undefined and you'll be redirected to the "/elsewhere" route.

What is the motivation / use case for changing the behavior?

The guard/resolver dichotomy crumbles with asynchronous data and does not allow for proper segregation of duties as described in the documenation; either guards must load data AND handle authorization, or resolvers must load data AND handle authorization. As it stands, you cannot use both as all guards are always processed first.

I actually wrote a complete breakdown in #15026, comment here -- this is probably the best explanation of why this is an issue that I can provide.

Environment


Angular version: 6.0.3


Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: 8.11.2  
- Platform:  MacOS

Others:

Related to #18247

@SamuelMS

To keep the info inside this ticket and easy to read, below is a copy of #15026 (comment) by @SamuelMS

This generally defeats the purpose of route guards for async dependencies.

Imagine this use-case:

// loaded in the root app router
const appRoutes: Routes = [
    {
        path: '',
        resolve: {
            config: ConfigResolver,
            user: UserResolver,
        },
        children: [
            { path: 'error', component: ErrorComponent },
            {
                path: 'somewhere',
                loadChildren: 'app/somewhere/somewhere.module#SomewhereModule'
            },
            {
                path: 'elsewhere',
                loadChildren: 'app/elsewhere/elsewhere.module#ElsewhereModule'
            },
            { path: '**', redirectTo: 'somewhere' }
        ]
    },
    { path: '**', redirectTo: '' }
];

// loaded in a fictional "somewhere" router
const somewhereRoutes: Routes = [
    {
        path: '',
        resolve: {
            userWorkspace: UserWorkspaceResolver,
        },
        children: [
            {
                path: '',
                component: WorkspaceDetailComponent,
                canActivateChild: [ WorkspaceDetailGuard ],
                children: [
                    // ...
                ],
            },
            {
                path: 'onboarding',
                component: WorkspaceOnboardingComponent,
                canActivateChild: [ WorkspaceOnboardingGuard ],
                children: [
                    // ...
                ],
            },
            { path: '**', redirectTo: '' }
        ]
    },
    { path: '**', redirectTo: '' }
];

Pretty simple. Two relevant modules: the root "app" module and a lazy-loaded "somewhere" module. Pardon the naming.

A bunch of top-level information is resolved at the highest-order routes, and then more information about some fictional "workspace" is resolved within the somewhere module.

Here's the catch: the "WorkspaceDetailGuard" and "WorkspaceOnboardingGuard" guards are dependent on the workspace resolved at a parent route (via UserWorkspaceResolver), which in turn is dependent on the user data loaded even before then (via UserResolver).

Per the explanations in this thread, this will never work as-is, since guards (CanActivate, CanActivateChild, etc) are always processed before resolvers can even begin their work. This makes sense if you ask "why start loading data before we know the user can even activate XYZ route" -- but only from a strictly synchronous standpoint; there's no reason to assume users will have all the information they need to properly "guard" routes before they make any asynchronous calls.

So, the apparent solution from this issue is to convert all async guards to resolvers, and do work in them -- or convert all resolvers to guards -- so that they fire in hierarchical order and can properly depend on each other. But this is not intuitive at all from a segregation of duties standpoint: guards are meant to handle authorization, and resolvers load data. In either of these scenarios, you're essentially using one or the other, but not both; you'll have guards that load data, or you'll have resolvers that handle authorization. This is backwards and is confusing.

Let me know if I'm misunderstanding something here; otherwise, I'd like to re-open this ticket and suggest that Angular handle guards first, then resolvers, but follow the parent -> child route hierarchy; in other words, guarantee that parent route data is resolved before child route logic fires.

Any updates?

I really would appreciate this change. The parent stuff should be handled first before ever going to the child parts.

I'm going crazy trying to find a scalable solution. The current evaluation order cannot work decently in enterprise size applications with dozens (or hundreds) of routes.

The best solution I could find for this without actually breaking Guard or Resolver logic is to create a dummy wrapper Component. I use this component for the original route path and give the actual component a child route with an empty path. This component checks the condition and displays a router outlet if it is satisfied or reroutes if not. Not the best solution but it works both as a client side and server side (SSR) solution. This wrapper component can be generic/resuable or it can be specific for each component.

For anyone interested, my generic component is like this:

export class ConditionalRerouteComponent implements OnInit, OnDestroy {
  public displayChild$ = new BehaviorSubject<boolean>(false);
  private readonly onDestroy$ = new Subject<void>();

  constructor(
    private readonly route: ActivatedRoute,
    private readonly router: Router
  ) {
  }

  public ngOnInit(): void {
    this.route.data.pipe(
      takeUntil(this.onDestroy$),
    ).subscribe(
      (data: Data) => this.checkCondition(data),
    );
  }

  public ngOnDestroy(): void {
    this.onDestroy$.next();
  }

  private checkCondition(data: Data): void {
    const result: any = data.checkCondition.call(this, this.route, this.router, data);
    if (result === true) {
      this.displayChild$.next(true);
    }
    else {
      this.router.navigateByUrl(result);
    }
  }
}

It's template is like this:

<ng-template #routerOutlet>
  <router-outlet></router-outlet>
</ng-template>

<ng-container *ngIf="displayChild$ | async; then routerOutlet"></ng-container>

And a route definition is like this:

export function myCondition(route: ActivatedRoute, router: Router, data: Data) {
  const someData = data.someData;
  const needsRedirect = someCalculation(someData);

  if (needsRedirect) {
    return someRedirectUrlCalculated(someData);
  }
  else {
    return true;
  }
}

const routes: Routes = [
  {
    path: 'some_path',
    component: ConditionalRerouteComponent,
    resolve: {
      someData: SomeDataResolverService,
    },
    runGuardsAndResolvers: 'always',
    data: {
      checkCondition: myCondition,
    },
    children: [
      {
        path: '',
        component: ActualComponent,
      },
    ],
  },
//
];

If a custom service is needed when going with the generic component approach, Injector can be injected to the generic component and can be passed to the checkCondition function.

Any update on this?

Also interested in a solution...

Experiencing this as well. My fix will be to run canActivate in place of resolvers higher up in the route hierarchy and just treat it as a resolver.

@jasonaden

@jasonaden Hey Jason, pinging you directly here because you added a couple tags a few years ago. There seems to be a lot of interest / support for this issue from the greater Angular community – do you have thoughts on who we should involve to get contributor eyes on this? 2.5 years for response time (and 4 major releases) seems a little excessive to me.

@jasonaden

@jasonaden is no longer working on the Angular project. You probably want to ping @atscott now :-)

@jasonaden

@jasonaden is no longer working on the Angular project. You probably want to ping @atscott now :-)

Thanks for the quick reply. Pinging @atscott! 🙌

I’ll start by saying that I understand the reasoning behind this request. It can make sense to conceptualize guards vs routers in this way and the current behavior simply does not allow this.

That said, you can also reason about it in a different way. You could say that the result of all guards can be determined at the beginning of routing because, after all, they’re guarding the navigation. Any guard that depends on a state that is modified during routing is invalid. A resolver could conceptually be a “guard”: “can I resolve a result for this query? If not block routing by returning EMPTY”. Maybe you can name these differently in your head to defend this conceptualization a bit more. I’m not saying this is right or how it should be, it’s just how it is now and unfortunately cannot be configured differently.

Both of these arguments could be seen as valid - they’re just conceptual ideas of what guards and resolvers should do. But the fact remains that we do not have a way to allow developers to determine which definition they want. In my opinion, one of the core issues with the router design is the inflexibility of it.

There are numerous requests to change certain behavior, like loading components at a level immediately rather than waiting until the end, this request to block at each level, another request to allow fallback routes if the first match fails.

A request like this one cannot be made as a unilateral change to the behavior. Instead, it would currently require additional code inside the router black box with a configurable option exposed at the highest level to switch the behavior. Over time, this would make the router package enormous in size and in API surface.

These requests really point at core problem with the router’s lack of configurability. Long term, the solution to these would be for the router to provide the building blocks, maybe in addition to the current behavior as an example “full system”. A router overhaul like this just simply (and disappointingly for everyone waiting for it) isn’t a priority right now.

The only solution I can recommend in any reasonable timeframe is moving state-changing logic to resolvers and keeping state-change-dependent code inside resolvers as well.

But isn't using using resolvers as guards is wrong by design?

Why not add pre guard resolvers and keep current ones behaviour as is. So we would make everyone happy without any changes to current projects. W dniu śr., 11.11.2020 o 18:30 Alexander Dmitrievich < [email protected]> napisał(a):

…

But isn't using using resolvers as guards is wrong by design? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#24187 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFKBQQPTTD2K27PQMFMR6LDSPLC37ANCNFSM4FCJDIVQ> .

@criskrzysiu, I not insist on changes. I am asking the question as is.
I need to be sure i am using features how they are intended to be used. Even if some features is buggy or behavior is not clear at this moment.
If i will need to use another mechanism of resolving guards/resolvers in my project - i will find another solution.

I meant that Angular should have that mechanizm and resolvers arę clearly shown in docs as a płace to resolve async dependencies and guards to only check if You have access to part of app. W dniu czw., 12.11.2020 o 04:07 Alexander Dmitrievich < [email protected]> napisał(a):

…

@criskrzysiu <https://github.com/criskrzysiu>, I not insist on changes. I am asking the question as is. I need to be sure i am using features how they are intended to be used. If i will need to use another mechanism of resolving guards/resolvers in my project - i will find another solution. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24187 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFKBQQN5JRODWJOKUC4FTDDSPNGP3ANCNFSM4FCJDIVQ> .

Closing in favor of #42953 (which links to this for tracking and discoverability). The unfortunate truth at the moment is that we cannot feasibly make this change with the current state of the router code. This is a good feature request and the ability to configure this type of thing without a bundle size cost to every Router user even if they don't use it would be one of the main goals there.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

Navigation Menu

Repository navigation

Change Route guard and resolver ordering to follow route parent/child hierarchy #24187

I'm submitting a...

Current behavior

Expected behavior

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

Environment

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Change Route guard and resolver ordering to follow route parent/child hierarchy #24187

Description

I'm submitting a...

Current behavior

Expected behavior

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

Environment

Activity

Toub commented on May 30, 2018

jibbers42 commented on Oct 17, 2018

Yura13 commented on Apr 19, 2019

ipzKellyR commented on Jun 19, 2019

lppedd commented on Jun 21, 2019

berserk81 commented on Feb 3, 2020

pantonis commented on Jun 15, 2020

digaus commented on Sep 11, 2020

1 remaining item

skwny commented on Oct 17, 2020

SamuelMS commented on Nov 11, 2020

petebacondarwin commented on Nov 11, 2020

SamuelMS commented on Nov 11, 2020

atscott commented on Nov 11, 2020

nullifiedtsk commented on Nov 11, 2020

kbrilla commented on Nov 11, 2020

nullifiedtsk commented on Nov 12, 2020

kbrilla commented on Nov 12, 2020

atscott commented on Jul 23, 2021

angular-automatic-lock-bot commented on Aug 23, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions