Revert "fix(router): fix navigation ignoring logic to compare to the browser url (#37408)" (#37650)

This reverts commit d3a817549b.

The reason for the revert is the problem reported in g3 which requires additional investigation.

PR Close #37650
This commit is contained in:
Andrew Kushnir 2020-06-19 12:34:12 -07:00 committed by Misko Hevery
parent bd7f440357
commit 2a970a0af8
2 changed files with 5 additions and 169 deletions

View File

@ -1127,20 +1127,11 @@ export class Router {
extras: NavigationExtras, extras: NavigationExtras,
priorPromise?: {resolve: any, reject: any, promise: Promise<boolean>}): Promise<boolean> { priorPromise?: {resolve: any, reject: any, promise: Promise<boolean>}): Promise<boolean> {
const lastNavigation = this.getTransition(); const lastNavigation = this.getTransition();
// * Imperative navigations (router.navigate) might trigger additional navigations to the same // If the user triggers a navigation imperatively (e.g., by using navigateByUrl),
// URL via a popstate event and the locationChangeListener. We should skip these duplicate // and that navigation results in 'replaceState' that leads to the same URL,
// navs. // we should skip those.
// * Imperative navigations can be cancelled by router guards, meaning the URL won't change. If if (lastNavigation && source !== 'imperative' && lastNavigation.source === 'imperative' &&
// the user follows that with a navigation using the back/forward button or manual URL change, lastNavigation.rawUrl.toString() === rawUrl.toString()) {
// the destination may be the same as the previous imperative attempt. We should not skip
// these navigations because it's a separate case from the one above -- it's not a duplicate
// navigation.
// Note that imperative navs might only trigger a popstate in tests because the
// SpyLocation triggers it on replaceState. Real browsers don't; see #27059.
const navigationToSameUrl = lastNavigation.urlAfterRedirects.toString() === rawUrl.toString();
const browserNavPrecededByRouterNav =
lastNavigation && source !== 'imperative' && lastNavigation.source === 'imperative';
if (browserNavPrecededByRouterNav && navigationToSameUrl) {
return Promise.resolve(true); // return value is not used return Promise.resolve(true); // return value is not used
} }

View File

@ -2539,161 +2539,6 @@ describe('Integration', () => {
}))); })));
}); });
describe('should not break the history', () => {
@Injectable({providedIn: 'root'})
class MyGuard implements CanDeactivate<any> {
allow: boolean = true;
canDeactivate(): boolean {
return this.allow;
}
}
@Component({selector: 'parent', template: '<router-outlet></router-outlet>'})
class Parent {
}
@Component({selector: 'home', template: 'home'})
class Home {
}
@Component({selector: 'child1', template: 'child1'})
class Child1 {
}
@Component({selector: 'child2', template: 'child2'})
class Child2 {
}
@Component({selector: 'child3', template: 'child3'})
class Child3 {
}
@Component({selector: 'child4', template: 'child4'})
class Child4 {
}
@Component({selector: 'child5', template: 'child5'})
class Child5 {
}
@NgModule({
declarations: [Parent, Home, Child1, Child2, Child3, Child4, Child5],
entryComponents: [Child1, Child2, Child3, Child4, Child5],
imports: [RouterModule]
})
class TestModule {
}
let fixture: ComponentFixture<unknown>;
beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({imports: [TestModule]});
const router = TestBed.get(Router);
const location = TestBed.get(Location);
fixture = createRoot(router, Parent);
router.resetConfig([
{path: '', component: Home},
{path: 'first', component: Child1},
{path: 'second', component: Child2},
{path: 'third', component: Child3, canDeactivate: [MyGuard]},
{path: 'fourth', component: Child4},
{path: 'fifth', component: Child5},
]);
// Create a navigation history of pages 1-5, and go back to 3 so that there is both
// back and forward history.
router.navigateByUrl('/first');
advance(fixture);
router.navigateByUrl('/second');
advance(fixture);
router.navigateByUrl('/third');
advance(fixture);
router.navigateByUrl('/fourth');
advance(fixture);
router.navigateByUrl('/fifth');
advance(fixture);
location.back();
advance(fixture);
location.back();
advance(fixture);
}));
// TODO(https://github.com/angular/angular/issues/13586)
// A fix to this requires much more design
xit('when navigate back using Back button', fakeAsync(() => {
const location = TestBed.get(Location);
expect(location.path()).toEqual('/third');
TestBed.get(MyGuard).allow = false;
location.back();
advance(fixture);
expect(location.path()).toEqual('/third');
expect(fixture.nativeElement).toHaveText('child3');
TestBed.get(MyGuard).allow = true;
location.back();
advance(fixture);
expect(location.path()).toEqual('/second');
expect(fixture.nativeElement).toHaveText('child2');
}));
it('when navigate back imperatively', fakeAsync(() => {
const router = TestBed.get(Router);
const location = TestBed.get(Location);
expect(location.path()).toEqual('/third');
TestBed.get(MyGuard).allow = false;
router.navigateByUrl('/second');
advance(fixture);
expect(location.path()).toEqual('/third');
expect(fixture.nativeElement).toHaveText('child3');
TestBed.get(MyGuard).allow = true;
location.back();
advance(fixture);
expect(location.path()).toEqual('/second');
expect(fixture.nativeElement).toHaveText('child2');
}));
// TODO(https://github.com/angular/angular/issues/13586)
// A fix to this requires much more design
xit('when navigate back using Foward button', fakeAsync(() => {
const location = TestBed.get(Location);
expect(location.path()).toEqual('/third');
TestBed.get(MyGuard).allow = false;
location.forward();
advance(fixture);
expect(location.path()).toEqual('/third');
expect(fixture.nativeElement).toHaveText('child3');
TestBed.get(MyGuard).allow = true;
location.forward();
advance(fixture);
expect(location.path()).toEqual('/fourth');
expect(fixture.nativeElement).toHaveText('child4');
}));
it('when navigate forward imperatively', fakeAsync(() => {
const router = TestBed.get(Router);
const location = TestBed.get(Location);
expect(location.path()).toEqual('/third');
TestBed.get(MyGuard).allow = false;
router.navigateByUrl('/fourth');
advance(fixture);
expect(location.path()).toEqual('/third');
expect(fixture.nativeElement).toHaveText('child3');
TestBed.get(MyGuard).allow = true;
location.forward();
advance(fixture);
expect(location.path()).toEqual('/fourth');
expect(fixture.nativeElement).toHaveText('child4');
}));
});
describe('should redirect when guard returns UrlTree', () => { describe('should redirect when guard returns UrlTree', () => {
beforeEach(() => TestBed.configureTestingModule({ beforeEach(() => TestBed.configureTestingModule({
providers: [ providers: [