Skip to content

Deactivation Guard breaks the routing history #13586

Closed

Description

@patrickracicot

I'm submitting a ... (check one with "x")

[X ] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior
Currently, I have a deactivation guard on a route which will return false or true depending on a condition. To get to this guarded route, the user must pass though 3 navigation step. Now, once on the guarded route, when using location.back(), the guard is called. If it returns true, the previous route is loaded. If the guard returns false, the navigation is cancelled. But if we redo a Location.back() after a failed navigation, the previous route that will be loaded will be 2 steps in the history instead of 1 (user perception).

Workflow

Main    -- navigate to     --> Route 1
Route 1 -- navigate to     --> Route 2
Route 2 -- navigate to     --> Route 3
Route 3 -- location.back() --> guard returns true  --> Route 2
Route 2 -- navigate to     --> Route 3
Route 3 -- location.back() --> guard returns false --> Route 3
Route 3 -- location.back() --> guard returns true  --> Route 1  (should be Route 2)

Expected behavior
An expected behavior for a user would be that navigating back brings back to the previous routed page.
Workflow

Main    -- navigate to     --> Route 1
Route 1 -- navigate to     --> Route 2
Route 2 -- navigate to     --> Route 3
Route 3 -- location.back() --> guard returns true  --> Route 2
Route 2 -- navigate to     --> Route 3
Route 3 -- location.back() --> guard returns false --> Route 3
Route 3 -- location.back() --> guard returns true  --> Route 2  (expected)

Minimal reproduction of the problem with instructions
Plnkr

  1. Click button Nav to route1
  2. Click button Nav to route2
  3. Click button Nav to route3
  4. Click button Block Nav Back
  5. Click button Nav back
    • BOGUE: The location.back() routed on Route1 instead of Route2

Personnal investigation
After some investigation, I saw that in routerState$.then (router.ts line 752) this logic used when navigationIsSuccessful == false is pretty simply but it is the actual cause of this bug. Basically, when a deactivation guard is hit, the location of the browser is already changed to the previous route. Which means that when the guard returns false, the routerState$ runs his logic and calls resetUrlToCurrentUrlTree(). At this point we can see that we replace the state of the current location. But by doing this, we loose that route in the history which means that in my plunker, if we click the block nav back 3 times and then click the nav back we will actually kill the application.

What is the motivation / use case for changing the behavior?
This is for me a pretty big bug since a guard that returns false breaks alters the current routing history. In the case of our application this breaks the workflow and brings wrong business scopes to a user.

Please tell us about your environment:

Windows 10, NPM, Nodejs, Visual Studio 2015 (using nodejs for typescript compilation)

  • Angular version: 2.3.3

  • Browser: [ all ]

  • Language: [TypeScript 2.0.10 | ES5]

Activity

  1. saverett commented on Jan 13, 2017

    @saverett

    A Router.back() method wouldn't cover using the browser back and forward buttons though, correct? That seems to be the predominant use case.

  2. DzmitryShylovich commented on Jan 13, 2017

    @DzmitryShylovich
    Contributor

    A Router.back() method wouldn't cover using the browser back and forward buttons though, correct?

    yeah. So just returning history to the previous state is the only way I can think of here.

  3. michelherv commented on Feb 22, 2017

    @michelherv

    Hi,
    I have the same problem in case of Activation Guard. Have I to create a new issue ?

    • Angular version: 2.4.6
    • Browser: all
    • Language: TypeScript 2.1.5
  4. DzmitryShylovich commented on Feb 22, 2017

    @DzmitryShylovich
    Contributor

    @michelherv yeah, reproduce on plunkr and file a new issue pls

  5. michelherv commented on Feb 22, 2017

    @michelherv

    @DzmitryShylovich done! You can found the description here #14645

  6. szh commented on Mar 30, 2017

    @szh

    This is still broken in Angular 4.0. Any plans to fix it?

  7. DzmitryShylovich commented on Mar 30, 2017

    @DzmitryShylovich
    Contributor

    @szh please don't spam everywhere. Pr is here #13922

  8. CRodriguez25 commented on May 16, 2017

    @CRodriguez25

    Is there a work around for this by any chance?

  9. arutnik commented on May 24, 2017

    @arutnik

    I couldn't find any workaround inside my candeactivate class. Still waiting for the PR

  10. skaiser commented on May 26, 2017

    @skaiser

    It seems it should work like resolve does, no? That is, it doesn't navigate until canDeactivate cancels or confirms whether the component can be deactivated (or routed away from). Otherwise, I don't really understand what it's purpose is.

  11. CRodriguez25 commented on May 26, 2017

    @CRodriguez25

    Skaiser, its purpose is what you described, but there is a bug currently. A PR has been made to solve the issue, but until then the CanDeactivate guard breaks your history.

  12. 159 remaining items

  13. atscott commented on Jul 1, 2021

    @atscott
    Contributor

    Hi, quick update here:

    // @ts-ignore: private option not yet exposed for public use
    router.canceledNavigationResolution = 'computed';
    
  14. atscott commented on Jul 16, 2021

    @atscott
    Contributor

    The known issue mentioned above was fixed in the 12.1.2 release this week. There are currently no known issues and it would be great if those interested in the fix could give it a quick test in their applications using the instructions above.

  15. mduft commented on Jul 19, 2021

    @mduft

    At first sight the fix seems to work perfectly, I couldn't reproduce a problematic case in our scenario anymore.

  16. andrew-landsverk-win commented on Aug 13, 2021

    @andrew-landsverk-win

    I am on 12.2.1 and I have added

    // @ts-ignore: private option not yet exposed for public use
    router.canceledNavigationResolution = 'computed';
    

    to the constructor in my app.component.ts.

    Is there anything further I need to look at or do at this time?

  17. liamdebeasi commented on Aug 17, 2021

    @liamdebeasi

    I can confirm that adding the code in #13586 (comment) resolves the issue for my use case (see: #43181)

  18. jcn-ithaca commented on Aug 18, 2021

    @jcn-ithaca

    The 'canceledNavigationResolution' fix worked in our application. (Note for thoroughness: I believe you have to also inject the router in the constructor);

  19. angular-automatic-lock-bot commented on Oct 3, 2021

    @angular-automatic-lock-bot

    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.

  20. locked and limited conversation to collaborators on Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions