Router 3.2.0 - canDeactivate runs only once

[alvipeo]

let's say you have a confirmation dialog on canDeactivate. if user refuses to leave a form (clicks No), the dialog closes (promise is resolved to false). now, user does the same thing again but canDeactivate doesn't fire anymore.

This doesn't happen on 3.1.0.

[robwormald]

closing as submitter deleted/ignored the provided issue template and did not provide a reproduction. i'll re-open if this changes.

[alvipeo]

Here's my code.

Guard:

export class SaveChangesGuard implements CanDeactivate<SubscriberEditComponent> {
    canDeactivate(component: SubscriberEditComponent, route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Promise<boolean> {
        console.log("inside canDeactivate"); // THIS fires ONLY ONCE
        return component.confirmLeaveDlg.requestConfirmation("Are you sure you want to leave without saving changes?");
    }
}

Dialog component (the one used above):

@Component({
    selector: "ilg-dialog",
    templateUrl: "./dialog.html"
})
export class IlgDialog implements OnInit, AfterContentInit, OnChanges {
    @Output() confirmResult = new EventEmitter<IlgConfirmResult>();
...
    requestConfirmation(confirmQtn: string): Promise<boolean> {  // tried Observable first, before finding the bug
        if (!this.isConfirmation) return Promise.resolve(true);

        this.show();
        return new Promise<boolean>(
            (resolve, reject) => {    // yes, ugly, but Observable didn't work, so I though I'd go with a Promise (but the problem with the router)
                this.confirmResult.map(res => res.result === IlgResultValue.Yes).do(val => console.log(val)).subscribe(res => {
                    resolve(res);
                });
            });
    }
}

Routes:

export const routes: Routes = [
  {
    path: '', component: SubscribersListComponent
  },
  {
    path: "new",
    component: SubscriberEditComponent,
    canDeactivate: [SaveChangesGuard]
  },
  {
    path: "edit/:email",
    component: SubscriberEditComponent,
    canDeactivate: [SaveChangesGuard]
  }
];

and the module:

@NgModule({
    imports: [
        CommonModule,
        ReactiveFormsModule,
        SharedModule,
        RouterModule.forChild(routes)
    ],
    declarations: [SubscribersListComponent, SubscriberEditComponent, SubscriberFormComponent],
    providers: [SaveChangesGuard]
})
export class SubscribersModule { }

So, again, the standard scenario - user edits a form, clicks on Cancel, canDeactivate works the first time (suppose user wishes to stay on the form). He edits the form and tries to leave it again => this time canDeactivate doesn't work in 3.2.0-rc.0. But works on 3.1.0.

[zoechi]

Please provide a Plunker.

[alvipeo]

Guys, I'm not really sure I'm that familiar with plunkr. Can I publish a repo on GitHub? Is it ok?

BTW, just checked with router 3.2.0 - problem is still there.

[alvipeo]

Edit: since guys know better than me how to provide a plunker, my repo and app are deleted.

ok, here's my repo - https://github.com/alvipeo/ng2router-test .

and the app running here - http://ng-2.azurewebsites.net/. You'll find step on how to repro on the main page.

The guard is implemented here - https://github.com/alvipeo/ng2router-test/blob/master/src/app/features/subscribers/saveChangesGuard.ts. It has console.log(component) inside canDeactivate. So if it really fires, you should see an output in the console.

The routing - https://github.com/alvipeo/ng2router-test/blob/master/src/app/features/subscribers/subscribers.routing.ts.

Hopefully, this helps.

[MartinNuc]

I actually encounted similar bug with CanActivate. I posted SO question because I thought there is some guard result caching going on: http://stackoverflow.com/questions/40692497/calling-angular2-route-guard-multiple-times/40693108#40693108

Issue is that when guard resolves to false value it is not being called anymore.

Here is plunker with 3.1.2 where guard is evaluated every time:
https://plnkr.co/edit/jUhVJY?p=preview

Here is plunker with 3.2.0 where guard is evaluated only once:
https://plnkr.co/edit/2A0Wfu?p=preview

[ericmartinezr]

Isn't this because of fixing these issues #11754, #9530, #11520 ? The fix : 091c390

[Krustie101]

In my application, I notice that the guard is not called a second time if you try to follow the same route as before. It does not even try to to execute the same route a second time. If you try to follow another route, the guard is called.

[DzmitryShylovich]

https://angular.io/docs/ts/latest/guide/router.html#!#reuse

[Krustie101]

Let me explain the scenario more precisely.

(1) first I route to A
(2) Then I try to route to B, which causes a CanDeActivate check on A which returns false. So no routing happens.
(3) Then I try to route to B again. No action is performed at all: no CanDeActivate call on A or a routing tot B.

But if try to route to C instead of B in step (3) or after step (3), the CanDeActivate check on A is performed.

[alvipeo]

@DzmitryShylovich so what? how is this related to this bug? just didn't get it

[Krustie101]

@ericmartinezr

You are right, the following piece of code causes these kind of issues

private scheduleNavigation(rawUrl: UrlTree, extras: NavigationExtras): Promise<boolean> {

  const prevRawUrl = this.navigations.value ? this.navigations.value.rawUrl : null;  

  if (prevRawUrl && prevRawUrl.toString() === rawUrl.toString()) {  

    return this.navigations.value.promise;  

  }

navigations is a BehaviorSubject with all scheduled navigations, navigations.value is the most recent scheduled navigation. The check does not take into account whether the promise was resolved to false.

After this piece of code, the new navigation is added to the scheduled navigations "queue".

[justinrassier]

I am encountering this exact scenario. I haven't broken it down into a plunker yet, but basically it looks like a promise that resolves to false inside a canDeactivate guard only runs once. It seems that if the guard just returns a boolean, all is fine.

[marcinkaszuba]

I have the same problem, canDeactivate guard runs only once. However the issue also exists in the Tour of Heros live example so I think any additional plunkr is not really necessary.

[adrienboulle]

I have the same issu in my code but in my understanding it is not specifically related to the gards but to the navigation scheduling.

in addition to @Krustie101 comment, this part of the code is also related:

private executeScheduledNavigation({id, rawUrl, prevRawUrl, extras, resolve,
                                      reject}: NavigationParams): void {
    const url = this.urlHandlingStrategy.extract(rawUrl);
    const prevUrl = prevRawUrl ? this.urlHandlingStrategy.extract(prevRawUrl) : null;
    const urlTransition = !prevUrl || url.toString() !== prevUrl.toString();

    if (urlTransition && this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) {
      //
    } else if (
        urlTransition && prevRawUrl && this.urlHandlingStrategy.shouldProcessUrl(prevRawUrl)) {
      this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url)));
      //
    } else {
      //
    }
}

Should this behaviour only occure when navigation is triggered from browser back button ?

[mwawrusch]

I ended up here because of this Stack Overflow article: http://stackoverflow.com/questions/40692497/calling-angular2-route-guard-multiple-times/40693108

This is a serious issue as it brakes people's authentication systems.

[adrienboulle]

There is one walkaround you can use.

Just before you return false on your CanActivate, force to navigate where you came from :

this._router.navigate([this._router.url]);
return false;

This will force the last navigation in the router history not to be the one that you just canceled and therefore the router will actually try again to navigate as expected.

[Falx]

No workaround for CanDeactivate though..

[ericeubank-powerschool]

This plunkr demonstrates the issue:
http://plnkr.co/edit/ZfPl1T0PJxBd6JDgYE25?p=preview

Click 'Guarded Component Two', dismiss the alert, and then click it again. Notice no alert is shown.

This plunkr is the same except the router is reverted to version 3.1.2:
http://plnkr.co/edit/lUbal7NcYYOagjj9vaYK?p=preview

Follow the same steps and notice you get an alert every time you click the link.

[adrienboulle]

@eeubank perfect demonstration

here is the workaround for CanActivate: http://plnkr.co/edit/fxpouRAdOyUOp4CpjW1u?p=preview

edit: it also works for CanDeactivate: http://plnkr.co/edit/olw4z85aBXenJhFawi5G?p=preview

[ericeubank-powerschool]

Similar to @Adboul's workaround for CanActivate, CanDeactivate can be worked around also by using RouteStateSnapshot:

http://plnkr.co/edit/6BC3IE5t9CELDtYLbaTt?p=preview

canDeactivate(component: ComponentTwo, route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
    let can = component.canDeactivate();
    if (!can) {
      alert('Deactivation blocked');
      
      this.router.navigate([state.url]);
      return false;
    }

    return true;
  }

[brendanbates89]

Glad I searched the issue tracker - this has been driving me nuts for hours. Having the same problem, and also traced it via breakpoints to the scheduleNavigation function as referenced above. I hope this gets fixed soon - there seems to be absolutely no simple way to retrieve the previous route from the route guard.

Just saw previous comment - looks like there is an easy way to get the previous/next route info. Good to know!

[Falx]

The Deactivate workaround doesn't seem to work in my case, I return the response of a prompt dialog via a Observable instead of just boolean.

[adrienboulle]

@Falx
It seems to work with promises:
http://plnkr.co/edit/n8bgrL2fFl1Ei4NjkKWn?p=preview
as well as with Observales:
http://plnkr.co/edit/ajsv5DHPm28tLymq6WMB?p=preview

[Falx]

In your plunkr, my Observable would be returned by the component.canDeactivate(); function. (since that will throw up a Dialog for the user to click if he has unsaved changes)

Or.. Am I doing it wrong?

edit: Seems I am getting somewhere now, only my prompt dialog is getting fired twice, but that is probably a mistake in my own code

[adrienboulle]

@Falx seems to work that way:
http://plnkr.co/edit/v4VkTMQfMR19rEJxsXqG?p=preview

[Falx]

@Adboul Thanks a lot man, I got it working thanks to you!

[brendanbates89]

Unfortunately after some messing around this morning, I haven't found a way to fix this without breaking a bunch of unit tests. My initial attempt, which does fix this problem, is a patch to the scheduleNavigation function:

private scheduleNavigation(rawUrl: UrlTree, extras: NavigationExtras): Promise<boolean> {
    const prevRawUrl = this.navigations.value ? this.navigations.value.rawUrl : null;
    if (prevRawUrl && prevRawUrl.toString() === rawUrl.toString()) {
      return this.navigations.value.promise;
    }

    let resolve: any = null;
    let reject: any = null;

    const promise = new Promise((res, rej) => {
      resolve = res;
      reject = rej;
    });

    const id = ++this.navigationId;
    const previousNavigation = this.navigations.value;
    this.navigations.next({id, rawUrl, prevRawUrl, extras, resolve, reject, promise});

    // Make sure that the error is propagated even though `processNavigations` catch
    // handler does not rethrow
    return promise
      .then((result: boolean) => {
          if (!result) {
          this.navigations.next(previousNavigation);
          }

          return new Promise(resolve => resolve(result));
      })
      .catch((e: any) => Promise.reject(e));
}

Basically, if navigation fails, then restore the previous navigation. However, as I said, this breaks several other unit tests, so I'm sure this is not the right solution. It causes more events to be triggered than expected, which is because I'm basically creating a new navigation event. It may need to be more complicated than this. I also tried keeping a string member variable of the previous URL and using that for the comparison, but that seems to also break a bunch of tests.

[adrienboulle]

@brendanbates89 I think that your attempt kind of break the logic of the router. A rejected navigation attempt is still a navigation attempt and should be kept as the last navigation.

Also, anybody who subscribe to navigations would think that a new navigation was fired which is not the case.

I think that a solution could be to keep track of a resolved/reject navigation and return the current navigation promise if the it's not pending. Something like that:

if (prevRawUrl && prevRawUrl.toString() === rawUrl.toString()
                           && this.navigations.value.promise.isPending) {
    return this.navigations.value.promise;
}

(I know isPending is not a promise property, I wrote that for the logic)

[brendanbates89]

@Adboul Yeah, that's what I figured, I had a feeling that really wasn't the proper solution. The real issue here is that the current value in this.navigations is not really where the user is currently, since the guard failed. Looking through the code, I'm not sure I see an easy way to work around that without causing other problems or side effects. I am, however, not experienced with the Angular2 codebase at all, just thought I'd give it a shot as a learning experience.

[adrienboulle]

@brendanbates89 no worries I am in the exact same situation as you are :)

[brendanbates89]

Just to note, there is another side effect to this as well. If you are on Page A navigating to Page B, and the guard returns false, the router thinks you are now on Page B, however the user is still on Page A. If there is no guard on Page B, then the user can go to any other page since the current guard will not be called again.

It definitely has to do with the fact that this.navigations is not an accurate representation of where the user actually is at the current point in time.

[mlakmal]

i have the same issue....

[alvipeo]

Yes! Thank you guys!

[luisabreu]

Hello guys.

Is this fix available on the npm repository? Ie, if i do a npm install for the router package, does it include the fix?

[ericmartinezr]

@luisabreu yes, if you check the changelog you'll see that the fix for this issue landed with 2.3.0

router: runs guards every time when unsuccessfully navigating to the same url over and over again (#13209) (d46b8de)

[phacic]

Had the same problem and realized it was because i was return a promise. I changed it to an observable and it was fixed...

[usama-jarral]

Somehow I've managed this to working with

this.location.pushState({}, "", this.router.url);

right before you return false in CanDeactivate it'll restore the state again so back button will be detected.
Here we have another issue when we land on first url there is no state present in history so our CanDeactivate wont executed. I'm using a hack to solve my issue

this.location.pushState({}, "", `/fake-url`);
this.location.pushState({}, "", this.router.url);

just call this when landing on a page in constructor of any service and it will add a state before your current state. let me know if there is any decent solution present :(

[gilsdav]

My Workaround to keep exact same state than before back click using canDeactivate:

const destinationLink = window.location.href;
setTimeout(() => {
    window.history.replaceState({}, '', destinationLink);
    window.history.pushState({}, '', this.router.url);
});
return false;

[Deepak993]

I am using angular router version 4.4.4 and canDeactivate function is not working for me.
I tried solution given by @adrienboulle, but no luck. When I choose cancel option from confirmation window, it appears again.

[drweb86-work]

sorry, that was mine issue

[WL-HY]

I am using angular router version 5.2.2 and canDeactivate will change history when I use window.history.go(-1) to navigate back.
I return a subject and see that nextState changed up once again.
And it will not run if the next history is out-site url
Anyone knows how to solve it?
After I use this before return false to canDeactivate to solve it,thanks to @babarxm
window.history.pushState({}, "", this.router.url)

[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.}