Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router Resolve<T> should have official support for UrlTree based redirection #29089

Closed
simeyla opened this issue Mar 4, 2019 · 7 comments
Closed
Labels
area: router feature: under consideration Feature request for which voting has completed and the request is now under consideration feature: votes required Feature request which is currently still in the voting phase feature Issue that requests a new feature freq3: high
Milestone

Comments

@simeyla
Copy link

simeyla commented Mar 4, 2019

馃殌 feature request

Relevant Package

@angular router

Description

I see that in the docs it is recommended to do the following to cancel a navigation event in a Resolve<T> route data resolver.

resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<Crisis> | Observable<never> {
    let id = route.paramMap.get('id');
 
    return this.cs.getCrisis(id).pipe(
      take(1),
      mergeMap(crisis => {
        if (crisis) {
          return of(crisis);
        } else { // id not found
          this.router.navigate(['/crisis-center']);
          return EMPTY;
        }
      })
    );
  }

While this does work it generates a pretty gnarly NavigationCancel event that basically says 'You broke my internal state':

NavigationCancel {id: 5, url: "/crisis-center/4444", reason: "Navigation ID 5 is not equal to the current navigation id 6"}
id: 5
reason: "Navigation ID 5 is not equal to the current navigation id 6"
url: "/crisis-center/4444"

This is because we have overlapping navigation events, and the original navigation is cancelled because we essentially triggered the router's panic mode.

Describe the solution you'd like

Angular 7.1 introduced the option of returning a UrlTree from a canActivate, and I think something similar should be added to Resolve<T> for feature parity.

A Resolve may fail for many reasons, but we should have a clean option to redirect within the resolver - especially in cases where it is impossible to know the final destination until you've attempted to get the data.

I think perhaps the signature of resolve needs to become something like :

 resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<T | UrlTree> | Promise<T | UrlTree> | T | UrlTree;

Describe alternatives you've considered

Yes, delving into the code showed me that I could throw an error that satisfies the isNavigationCancelingError() type-guard and not only would it give me a nicer NavigationCancel event it would also allow me to pass a url: UrlTree parameter which will then be navigated to on my behalf.

      throw { ngNavigationCancelingError: true, url: this.router.createUrlTree(newUrl) };

While this actually works quite well - and gives the following 'nice' event it isn't really supported since I have to use the magic internal property name - and also not every reason for aborting a Resolve is an error condition.

NavigationCancel {id: 11, url: "/account/orders/latest", reason: undefined}
id: 11
reason: undefined
url: "/account/orders/latest"

The other alternative is what is suggested in the docs, but the error (Navigation ID 5 is not equal to the current navigation id 6) from doing that just makes it look like something is broken.

I think the mechanics of this are basically already there, expecially with the new functionality in 7.1 to handle a UrlTree. There is also plenty of need for such a feature on Stackoverflow..

@ngbot ngbot bot added this to the needsTriage milestone Mar 4, 2019
@jasonaden jasonaden added freq3: high feature Issue that requests a new feature labels Apr 4, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Apr 4, 2019
@jasonaden
Copy link
Contributor

Agreed, this should be added as a feature. I'll assign to myself, but I know I won't be able to get to this until some time after version 8 is finalized (probably next month). If anyone in the community would like to pick this up, that would be great. It could be modeled on the changes we did to implement this feature in Guards (#26521 and #26478).

@euskadi31
Copy link

Any news ?

@SchnWalter
Copy link
Contributor

SchnWalter commented May 29, 2020

Angular 7.1 introduced the option of returning a UrlTree from a canActivate, and I think something similar should be added to Resolve for feature parity.

Returning an UrlTree instead of the actual data will only cause confusion. I think that the best approach would be to improve the router specific error handler so that it can do more things.


TL;DR: You can use ExtraOptions.errorHandler to perform these redirects.


We can either replace it with a proper ErrorHandler class provided via dependency injection, just like for the global ErrorHandler or, at least, we could provide the Router service instance to the router error handler function, not just the error, and it will allow you to do something like this:

const ROUTER_CONFIG: ExtraOptions = {
  errorHandler: (err: unknown, router: Router): any => {
    if (err instanceof AccessDeniedError) {
      router.navigateByUrl(PAGE_URL.ACCESS_DENIED);
    } else if (err instanceof PageNotFoundError) {
      router.navigateByUrl(PAGE_URL.PAGE_NOT_FOUND);
    } else if (err instanceof HttpErrorResponse) {
      // verify the response code and do something..
    } else {
      throw err;
    }
  }
}

@NgModule({
  exports: [RouterModule],
  imports: [RouterModule.forRoot(ROUTES, ROUTER_CONFIG)],
})
export class AppRoutingModule { }

This last option would be very easy to implement, you just add this (an instance of Router) to t.resolve(this.errorHandler(e)); in:

https://github.com/angular/angular/blob/10.0.0-rc.0/packages/router/src/router.ts#L803-L811

If we get a 馃憤 from the Angular Core maintainers, I'll update the code + write the tests.

L.E.

It looks like there is just one test for custom error handlers, which doesn't cover much

https://github.com/angular/angular/blob/10.0.0-rc.0/packages/router/test/integration.spec.ts#L1346-L1361

And looking through the history (#25740) I found that the current errorHandler code has been inherited from the old Promise-based router, and you also couldn't to much with it in the previous version. Maybe I'm missing something, but in the current state, the errorHandler seems to be very limited. Any ideas what you could do with it? If a redirect could be performed?

L.E.2

It turns out that you can actually use this, if you pass the error handler as a normal function, not an arrow function, so the above code works if you just change the definition to something like this:

export function appRoutingErrorHandler(this: Router, err: unknown): any {
  // ...
}
const ROUTER_CONFIG: ExtraOptions = {
  errorHandler: appRoutingErrorHandler,
}

I think that this part should be documented, and maybe an example could be given. I'll look into this in the next week or so.

@angular-robot angular-robot bot added the feature: votes required Feature request which is currently still in the voting phase label Jun 4, 2021
@angular-robot
Copy link
Contributor

angular-robot bot commented Jun 4, 2021

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@angular-robot angular-robot bot added the feature: under consideration Feature request for which voting has completed and the request is now under consideration label Jun 5, 2021
@atscott
Copy link
Contributor

atscott commented Jun 22, 2022

This would be possible after introducing a RedirectCommand (#45023). Resolvers that return the Router's RedirectCommand would be assumed to be distinctly indicating a redirect rather than resolved data.

atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Jul 7, 2022
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
@Adamicov
Copy link

Any update on that?

atscott added a commit to atscott/angular that referenced this issue Nov 16, 2023
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 20, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 20, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 20, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 20, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 20, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 20, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Feb 22, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
atscott added a commit to atscott/angular that referenced this issue Mar 21, 2024
Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089
ilirbeqirii pushed a commit to ilirbeqirii/angular that referenced this issue Apr 6, 2024
)

Returning a `RedirectCommand` from a resolver can be interpreted as
distinctly different from regular resolved data. When a resolver returns
`RedirectCommand` we can interperet this as an intention to redirect in
the same way as other guards.

resolves angular#29089

PR Close angular#54556
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: router feature: under consideration Feature request for which voting has completed and the request is now under consideration feature: votes required Feature request which is currently still in the voting phase feature Issue that requests a new feature freq3: high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants