angular / router

The Angular 1 Component Router
MIT License
665 stars 135 forks source link

canDeactivate passes wrong component as first argument #447

Open mike-co opened 4 years ago

mike-co commented 4 years ago

Environment:

 "dependencies": {
    "@angular/animations": "9.1.2",
    "@angular/common": "^9.1.2",
    "@angular/compiler": "^9.1.2",
    "@angular/core": "^9.1.2",
    "@angular/forms": "^9.1.2",
    "@angular/platform-browser": "^9.1.2",
    "@angular/platform-browser-dynamic": "^9.1.2",
    "@angular/router": "^9.1.2",
    "core-js": "^3.6.4",
    "rxjs": "^6.5.4",
    "tslib": "^1.11.1",
    "zone.js": "^0.10.3"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "~0.901.1",
    "@angular/cli": "^9.1.1",
    "@angular/compiler-cli": "9.1.2",
    "@angular/language-service": "9.1.2",

Overview of the Issue

Consider the following routing tree: image which has canDeactivate guard

import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, CanDeactivate, RouterStateSnapshot } from '@angular/router';
import { SubAComponent } from './sub-a.component';

@Injectable()
export class SubACanDeactivateGuard implements CanDeactivate<SubAComponent> {
    public canDeactivate(
        component: SubAComponent,
        _route: ActivatedRouteSnapshot,
        _state: RouterStateSnapshot
    ): boolean {
        console.log('SubACanDeactivateGuard: ', component);
        return true;
    }
}

The guard is attached to SubAComponent

import { NgModule } from '@angular/core';
import { Routes, RouterModule } from '@angular/router';
import { SubAComponent } from './sub-a.component';
import { SubACanDeactivateGuard } from './sub-a-can-deactivate.guard';

const routes: Routes = [
    { path: '', component: SubAComponent, canDeactivate: [SubACanDeactivateGuard]  }
];

@NgModule({
    imports: [RouterModule.forChild(routes)],
    declarations: [SubAComponent],
    providers: [SubACanDeactivateGuard]
})
export class SubAModule {}

Then if I navigate from sub-a route to b route then the canDeactivate guard will be called with the instance of the AComponent, while it used to be called with the instance of SubAComponent. If I navigate from sub-a to a, then the guard is called with the proper instance of SubAComponent.

Expected behavior

From my understanding, the guard should only be called with SubAComponent.

is it a regression?

Possibly

Reproduce the Error

The following is router v9.1.2 example which shows broken behavior: https://stackblitz.com/edit/angular-candeactivate-issue-ng9

And the example with the router v9.1.0 which shows I would assume proper behavior https://stackblitz.com/edit/angular-candeactivate-issue-ng9-router910

In both cases:

Possibly related to

https://github.com/angular/angular/commit/8e7f903 https://github.com/angular/angular/issues/34614 https://github.com/angular/angular/pull/36302

8thnote commented 4 years ago

We just discovered this bug in our application. Seems like it was working properly in v9.1.1 and not in v9.1.2. We're reverting to v9.1.1 for now and will keep an eye on this.

atscott commented 4 years ago

Thanks for the report. This was caused by merging https://github.com/angular/angular/pull/36302. The failure was detected and the commit was reverted for the 9.1.3 release, which will be cut today.

FYI - please direct your future bug reports to the main angular/angular repo.

dcatoday commented 4 years ago

I'm still seeing this issue after upgrading to @angular/router v9.1.3. Was this included in the release?

atscott commented 4 years ago

@dcatoday - the commit that was reverted was attempting to fix a bug with the incorrect component being passed to canDeactivate but caused a regression in other configs. https://github.com/angular/angular/pull/36302 We reverted the commit to return behavior to what it was before, but that means canDeactivate will still pass the wrong component in the situation that is described in that PR. Without seeing your config, it’s hard to tell what the cause is. Please file an issue with a minimal reproduction on the main Angular repo if you believe there is a bug that isn’t addressed in the existing issue report.