NavigationExtras when returning URLTree from guard

🚀 feature request

Relevant Package

This feature request is for @angular/router

Description

In #26521 (Angular 7.1.0), a feature was introduced that allows returning a URLTree from guards in order to initiate a redirect. However, this doesn't seem to allow using NavigationExtras such as skipLocationChange.

Since the change in #26521 wasn't just syntax sugar, but is relevant to have predictable behavior when dealing with multiple guards, it should be considered to somehow allow using these options when returning a URLTree as well.

Describe the solution you'd like

I don't have a really good proposal at this moment apart from next to returning URLTree also returning some kind of

// TODO: Find a better name?
interface NavigationOptions {
  urlTree: URLTree;
  navigationExtras: NavigationExtras;
}

such that I could use

class AwesomeGuard implements CanActivate {
  constructor(private router: Router) {}

  canActivate(): NavigationOptions | Observable<NavigationOptions> {
    return {
      urlTree: this.router.parseUrl("…"),
      navigationExtras: {
        skipLocationChange: true,
      },
    };
  }
}

I guess the exact typing of the guard would have to be

// Just a placeholder name
type GuardReturnType = boolean | URLTree | NavigationOptions;

interface CanActivate {
  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): 
    GuardReturnType 
    | Promise<GuardReturnType> 
    | Observable<GuardReturnType>
}

Admittedly, the return type of this function is growing increasingly complex with this.

Describe alternatives you've considered

Possible options that I don't think would make good solutions would be

Returning an array [URLTree, NavigationExtras] (this just feels unclean)
Extending URLTree with the navigationExtras and extend parseUrl to take them as well (this muddies the entire API for a change in a specific location)

Actually, if going down this route, we'd probably need to split NavigationExtras into multiple parts as it also contains things such as queryParams, which is already part of the URLTree. We'd need a base that only contains the extras like skipLocationChange. This should still be a compatible change, though, as it can be

// Again, names are placeholders

interface NavigationExtrasBase {
  skipLocationChange?: boolean
}

interface NavigationExtras extends NavigationExtrasBase {
  // … as today, just without skipLocationChange
}

interface NavigationOptions {
  urlTree: URLTree;
  // … and now this base type here:
  navigationExtras: NavigationExtrasBase;
}

@Airblader

@Airblader This is a good feature request and this was discussed when working on the API for this redirect feature. The question is, what is the use case? What if NavigationExtras were forwarded on from the current navigation that's being processed? Would that satisfy this use case? Or would that cause problems in the application?

I don't mind adding something like this, I just want to make sure the use cases are clearly defined. If it's something like skipLocationChange needs to be available, can you give a concrete example of when this would be needed?

Thanks for the issue!

@jasonaden

@jasonaden Thanks for the reply! Our concrete usecase is e.g. a 403 redirect where we want to show an error page but retain the URL.

So no, the forwarding of the navigation extras wouldn't help us here, unfortunately.

I had a whole comment ready to go to post on the primary issue (#24618) assuming this was a bug until I ran into this issue: in my opinion I would say the default use case should be for it to consider this a redirectTo and skipLocationChange - I'm not sure under what scenarios you'd want to keep the original route.

Here are the main scenarios I'm returning UrlTree currently:

The application is multi-tenant, where each user is a member of various tenants. If a user enters the site at the base / URL, my authorization guard will determine which tenants they are a member of and redirect the user to the last tenant they were viewing (e.g. tenantA).

Result: user has entries of / and /tenantA in their browser history and can navigate back to /
Shortened links: Let's say I have a guard that will transform the url /tenantA/subentities/456 to the url /tenantA/entities/123/subentities/456 (via an API call that determined the parent id). The reason being other systems may not know the parent entity id.

Result: user has entries of /tenantA/subentities/456 and /tenantA/entities/123/subentities/456, allowing them to navigate back to the invalid url
Link repair: using the same example from above, given the url /tenantA/entities/123/subentities/456, if the entities id was really 222, the guard can return a repaired url of /tenantA/entities/222/subentities/456 (in these two use cases the subentity is truly the primary entity we care about, and the entities level is more of a vanity hierarchical collection)

A similar example could be around formatting (i.e. if your route has a date like /1-1-2000/ and you redirect to standardize it to /01-01-2000/)

In all of these examples I wouldn't want the original url to exist as valid locations in the browser

@jayoungers I think in your second example replaceUrl would also be a reasonable choice, so I think just setting skipLocationChange doesn't cut it here. Also that would be pretty unexpected and confusing in my opinion.

@Airblader

@Airblader I think replaceUrl would probably work in all of the scenarios.

When the UrlTree result for canActivate was added as an option, in my mind the functionality was now:

Yes, you can go to this route (true)
No, you can't go to this route (false)
You're going to the wrong route - you should be requesting this route instead (UrlTree)

Regarding the scenario: is the route replacement the unexpected/confusing part? or just the fact the url is altered? My scenario probably isn't the best representation, but if the issue is around the route being altered/standardized in general, I think that's a pretty common practice (news sites usually rewrite the url with the article title, stackoverflow will do this as well if the url only contains the question id, etc)

is the route replacement the unexpected/confusing part?

The unexpected part is that the navigation works differently than any other navigation where I don't set any navigation extras. IMHO here Angular would suddenly dictate opinion over what navigation does or doesn't make sense, and I would like to avoid that. Everywhere else the user gets to choose the options, so it should be the same here. Failing that, it should at least behave the same way as everywhere else (which is the current behavior).

I see. I think the answer comes down to what the intention of returning UrlTree is:

I'm looking at is as though the requested route is not valid under any circumstance. If we're looking at it as a short hand for the navigate method, then what you're saying makes perfect sense.

In my mind I would have had the functionality setup so that if UrlTree is returned, then that is the route the user intended to go to (the guard result wasn't true, it wasn't false, it just lead to the correct path).

In scenarios such as authentication for example, where the user isn't logged in or the session has expired, I don't think UrlTree is the correct response: it should be false (this route is valid, but you don't have access to it), and the guard router.navigates to the login page.

But the benefit and entire point of allowing returning a tree is that it makes how multiple guards interact predictable and stable. This feature wasn't about specific usecases. I'd like to start returning URL tree in as many cases as I can (whenever I don't return true).

I agree: based on the original requirements I would say I'm definitely not looking at the feature as it was intended.

If you don't mind, just out of curiosity, would you happen to have an example scenario of multiple guards firing on a route that result in different UrlTrees?

+1 to this issue as having navigationExtras when creating a UrlTree allows canActivate to preserve extras like state if the url is changed.

In addition, it would be nice if there's a way to preserve the navigation extras when returning a url tree from CanActivate.

feat(router): Add ability to return `UrlTree` with `NavigationBehavio…

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

NavigationExtras when returning URLTree from guard #27148

🚀 feature request

Relevant Package

Description

Describe the solution you'd like

Describe alternatives you've considered

59 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

NavigationExtras when returning URLTree from guard #27148

Description

🚀 feature request

Relevant Package

Description

Describe the solution you'd like

Describe alternatives you've considered

Activity

Airblader commented on Nov 17, 2018

jasonaden commented on Nov 28, 2018

Airblader commented on Nov 28, 2018

JohnYoungers commented on Jan 7, 2019

Airblader commented on Jan 8, 2019

JohnYoungers commented on Jan 8, 2019

Airblader commented on Jan 9, 2019

JohnYoungers commented on Jan 9, 2019

Airblader commented on Jan 9, 2019

JohnYoungers commented on Jan 9, 2019

59 remaining items

chjernig-msft commented on Aug 16, 2022

angular-automatic-lock-bot commented on Apr 12, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions