honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

This commit is contained in:
Victor Savkin 2016-12-02 15:19:00 -08:00 committed by Alex Rickabaugh
parent bbb7a39414
commit d46b8deeea
2 changed files with 100 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
modules/@angular/router/src/router.ts
View File

@ -281,11 +281,11 @@ function defaultErrorHandler(error: any): any {
type NavigationParams = {
id: number,
rawUrl: UrlTree,
prevRawUrl: UrlTree,
extras: NavigationExtras,
resolve: any,
reject: any,
promise: Promise<boolean>
promise: Promise<boolean>,
imperative: boolean
};
@ -387,8 +387,19 @@ export class Router {
// which does not work properly with zone.js in IE and Safari
this.locationSubscription = <any>this.location.subscribe(Zone.current.wrap((change: any) => {
const rawUrlTree = this.urlSerializer.parse(change['url']);
const lastNavigation = this.navigations.value;
// If the user triggers a navigation imperatively (e.g., by using navigateByUrl),
// and that navigation results in 'replaceState' that leads to the same URL,
// we should skip those.
if (lastNavigation && lastNavigation.imperative &&
lastNavigation.rawUrl.toString() === rawUrlTree.toString()) {
return;
}
setTimeout(() => {
this.scheduleNavigation(rawUrlTree, {skipLocationChange: change['pop'], replaceUrl: true});
this.scheduleNavigation(
rawUrlTree, false, {skipLocationChange: change['pop'], replaceUrl: true});
}, 0);
}));
}
@ -510,11 +521,12 @@ export class Router {
navigateByUrl(url: string|UrlTree, extras: NavigationExtras = {skipLocationChange: false}):
Promise<boolean> {
if (url instanceof UrlTree) {
return this.scheduleNavigation(this.urlHandlingStrategy.merge(url, this.rawUrlTree), extras);
return this.scheduleNavigation(
this.urlHandlingStrategy.merge(url, this.rawUrlTree), true, extras);
} else {
const urlTree = this.urlSerializer.parse(url);
return this.scheduleNavigation(
this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree), extras);
this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree), true, extras);
}
}
@ -596,12 +608,8 @@ export class Router {
.subscribe(() => {});
}
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;
}
private scheduleNavigation(rawUrl: UrlTree, imperative: boolean, extras: NavigationExtras):
Promise<boolean> {
let resolve: any = null;
let reject: any = null;
@ -611,18 +619,17 @@ export class Router {
});
const id = ++this.navigationId;
this.navigations.next({id, rawUrl, prevRawUrl, extras, resolve, reject, promise});
this.navigations.next({id, imperative, rawUrl, extras, resolve, reject, promise});
// Make sure that the error is propagated even though `processNavigations` catch
// handler does not rethrow
return promise.catch((e: any) => Promise.reject(e));
}
private executeScheduledNavigation({id, rawUrl, prevRawUrl, extras, resolve,
reject}: NavigationParams): void {
private executeScheduledNavigation({id, rawUrl, 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();
const urlTransition = !this.navigated || url.toString() !== this.currentUrlTree.toString();
if (urlTransition && this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) {
this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url)));
@ -635,7 +642,8 @@ export class Router {
// we cannot process the current URL, but we could process the previous one =>
// we need to do some cleanup
} else if (
urlTransition && prevRawUrl && this.urlHandlingStrategy.shouldProcessUrl(prevRawUrl)) {
urlTransition && this.rawUrlTree &&
this.urlHandlingStrategy.shouldProcessUrl(this.rawUrlTree)) {
this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url)));
Promise.resolve()
.then(

118
modules/@angular/router/test/integration.spec.ts
View File

@ -1383,6 +1383,13 @@ describe('Integration', () => {
useValue:
(c: any, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => { return false; }
},
{
provide: 'alwaysFalseAndLogging',
useValue: (c: any, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
log.push('called');
return false;
}
},
]
});
});
@ -1532,62 +1539,86 @@ describe('Integration', () => {
expect(location.path()).toEqual('/main/component1');
})));
});
describe('should work when given a class', () => {
class AlwaysTrue implements CanDeactivate<TeamCmp> {
canDeactivate(
component: TeamCmp, route: ActivatedRouteSnapshot,
state: RouterStateSnapshot): boolean {
return true;
}
}
beforeEach(() => { TestBed.configureTestingModule({providers: [AlwaysTrue]}); });
it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
it('should call guards every time when navigating to the same url over and over again',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig(
[{path: 'team/:id', component: TeamCmp, canDeactivate: [AlwaysTrue]}]);
router.resetConfig([
{path: 'simple', component: SimpleCmp, canDeactivate: ['alwaysFalseAndLogging']},
{path: 'blank', component: BlankCmp}
router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');
]);
router.navigateByUrl('/team/33');
router.navigateByUrl('/simple');
advance(fixture);
expect(location.path()).toEqual('/team/33');
router.navigateByUrl('/blank');
advance(fixture);
expect(log).toEqual(['called']);
expect(location.path()).toEqual('/simple');
router.navigateByUrl('/blank');
advance(fixture);
expect(log).toEqual(['called', 'called']);
expect(location.path()).toEqual('/simple');
})));
});
describe('should work when returns an observable', () => {
beforeEach(() => {
TestBed.configureTestingModule({
providers: [{
provide: 'CanDeactivate',
useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
return Observable.create((observer: any) => { observer.next(false); });
}
}]
});
describe('should work when given a class', () => {
class AlwaysTrue implements CanDeactivate<TeamCmp> {
canDeactivate(
component: TeamCmp, route: ActivatedRouteSnapshot,
state: RouterStateSnapshot): boolean {
return true;
}
}
beforeEach(() => { TestBed.configureTestingModule({providers: [AlwaysTrue]}); });
it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig(
[{path: 'team/:id', component: TeamCmp, canDeactivate: [AlwaysTrue]}]);
router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');
router.navigateByUrl('/team/33');
advance(fixture);
expect(location.path()).toEqual('/team/33');
})));
});
it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig(
[{path: 'team/:id', component: TeamCmp, canDeactivate: ['CanDeactivate']}]);
describe('should work when returns an observable', () => {
beforeEach(() => {
TestBed.configureTestingModule({
providers: [{
provide: 'CanDeactivate',
useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
return Observable.create((observer: any) => { observer.next(false); });
}
}]
});
});
router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');
it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);
router.navigateByUrl('/team/33');
advance(fixture);
expect(location.path()).toEqual('/team/22');
})));
router.resetConfig(
[{path: 'team/:id', component: TeamCmp, canDeactivate: ['CanDeactivate']}]);
router.navigateByUrl('/team/22');
advance(fixture);
expect(location.path()).toEqual('/team/22');
router.navigateByUrl('/team/33');
advance(fixture);
expect(location.path()).toEqual('/team/22');
})));
});
});
});
@ -1826,6 +1857,7 @@ describe('Integration', () => {
TestBed.configureTestingModule({declarations: [RootCmpWithLink]});
const router: Router = TestBed.get(Router);
const loc: any = TestBed.get(Location);
const f = TestBed.createComponent(RootCmpWithLink);
advance(f);