Repository navigation
Add option to not reset Url and Router state to Angular Router on Navigation Errors #42915
Description
Which @angular/* package(s) are relevant/releated to the feature request?
router
Description
Currently the Angular router resets the state and the url on navigation errors [1] and on Navigation cancels that do not redirect [2]. Due to this, if someone is navigating from “a” -> “b” and hits a navigation error or a failed canActivate guard, the url stays on “a”. Furthermore, if the url update strategy is ‘eager’, the url actually gets changed from ‘a’ to ‘b’ prior to the error and is then changed back to ‘a’, resulting in the url flickering for the user from the destination url back to their origin url.
This makes it difficult to show an error page while maintaining the context of the destination url “b”, and is somewhat counterintuitive when using the url update strategy of ‘eager’ as the url changes back and forth. See #17004 for a similar feature request that focuses on showing an error page on pages that have failed canActivate guards.
[1]
angular/packages/router/src/router.ts
Line 955 in e2aa765
[2]
angular/packages/router/src/router.ts
Line 923 in e2aa765
Proposed solution
Add a new setting in ExtraOptions[1] named resetUrlOnError that is by default true.
export interface ExtraOptions{
...
resetUrlOnError?: boolean;
}
And then where resetStateAndUrl is called, we reference this option to conditionally skip calling it if that setting is disabled.
if(this.resetUrlOnError){
this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl);
}
However, this only results in the behaviour we want when the url update strategy is set to eager, as the url is updated prior to when the navigation error is encountered. Due to this, we will also only skip this call if the url update strategy is set to eager. We would also make sure to call out in a comment for resetUrlOnError that it only works in eager mode.
if(this.resetUrlOnError || this.urlUpdateStrategy !== ‘eager’){
this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl);
}
[1]
angular/packages/router/src/router_module.ts
Line 256 in 1d9d026
Alternatives considered
Along with the above change, we could modify the state and url for navigation errors that are encountered when the url update strategy is set to deferred. However as we do not know where in the navigation the error occurred, and therefore we do not know what state the url and router state will be in, it seems safer to only support this setting with the url update strategy set to eager. This also makes more sense with the eager url update strategy as we are specifically choosing to not wait until after the navigation ends to change the url.
Another alternative would be to move the resetStateAndUrl call into the default ErrorHandler [1], which would allow users to overwrite this and define their own error handling in that handler. However this function is currently empty and is often overwritten, so moving this call into this function would require a careful migration process to ensure no angular users are broken by the change.
#27148 would also enable us to achieve this by giving us more options to set during failed guards. However, those settings would need to be set for every guard and as it is focused on guards, it would not solve the more general case of errors encountered during a navigation.
On top of all this, this proposed change is fairly simple and straightforward, which results in no change in behavior by default. Each of the above alternatives add complexity which would require more investigation to ensure that no existing users of Angular are broken by a change to the Router code.
[1]
angular/packages/router/src/router.ts
Line 223 in e2aa765
Activity
Airblader commentedon Jul 22, 2021
I don't understand how this change would solve #27148. Can you elaborate on that?
It seems to me that #27148 is about the guard defining this behavior while the proposal here aims to define the behavior on the navigation attempt itself. Wouldn't that mean that you need to set this flag potentially for every navigation in the entire app (including routerLink)?
angular-robot commentedon Jul 22, 2021
This feature request is now candidate for our backlog! In the next phase, the community has 60 days to upvote. If the request receives more than 20 upvotes, we'll move it to our consideration list.
You can find more details about the feature request process in our documentation.
DiegoRGonzalez commentedon Jul 22, 2021
Good catch, thanks! I actually meant to add it to the RouterModule's ExtraOptions, not the NavigationBehaviourOptions, which would define this setting for all navigations, the same as the UrlHandlingStrategy. I've edited my feature request to reflect that.
Airblader commentedon Jul 22, 2021
Thanks for the clarification! I'd still like to add that this wouldn't solve the use case presented in #27148 because we wouldn't want to have this globally, but only for the redirects done from (a certain) guard.
DiegoRGonzalez commentedon Jul 22, 2021
Ah I see, that does seem distinct, thanks for clearing that up! I've modified my comment to specify that #27148 would allow us to achieve our desired behavior for guards instead of saying that both of these fix the same issue.
angular-robot commentedon Aug 30, 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 commentedon Sep 19, 2021
Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.
We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.
You can find more details about the feature request process in our documentation.
atscott commentedon Dec 16, 2021
Closing. This feature was requested as a potential fix for an internal application's needs. After further investigation, I don't believe this is the direction we would want to address the issues there. Some notes:
resetStateAndUrlis only called when the navigation fails due to an error. However, it really should always be called when a navigation fails. This change to make the reset consistent would affect the desired outcome for this request.angular-automatic-lock-bot commentedon Jan 16, 2022
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.
feat(router): `withNavigationErrorHandler` can convert errors to redi…