From 8e5b582b61faf87972e73f8a1dc52e822471cf8f Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Fri, 6 Oct 2017 18:15:19 -0700 Subject: [PATCH] Revert "fix(router): navigating to the current location works (#19463)" This reverts commit b67d574a9562d76ab0a838f4d1b42501e5c5f09b. --- packages/common/testing/src/location_mock.ts | 4 +- packages/router/src/router.ts | 147 +++++++++---------- packages/router/test/integration.spec.ts | 64 +++----- 3 files changed, 95 insertions(+), 120 deletions(-) diff --git a/packages/common/testing/src/location_mock.ts b/packages/common/testing/src/location_mock.ts index b2acc613ca..f52fcc4ecb 100644 --- a/packages/common/testing/src/location_mock.ts +++ b/packages/common/testing/src/location_mock.ts @@ -41,9 +41,7 @@ export class SpyLocation implements Location { return currPath == givenPath + (query.length > 0 ? ('?' + query) : ''); } - simulateUrlPop(pathname: string) { - this._subject.emit({'url': pathname, 'pop': true, 'type': 'popstate'}); - } + simulateUrlPop(pathname: string) { this._subject.emit({'url': pathname, 'pop': true}); } simulateHashChange(pathname: string) { // Because we don't prevent the native event, the browser will independently update the path diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 920715c1c4..cd2d68c35d 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -43,12 +43,12 @@ declare let Zone: any; */ export interface NavigationExtras { /** - * Enables relative navigation from the current ActivatedRoute. - * - * Configuration: - * - * ``` - * [{ + * Enables relative navigation from the current ActivatedRoute. + * + * Configuration: + * + * ``` + * [{ * path: 'parent', * component: ParentComponent, * children: [{ @@ -59,92 +59,92 @@ export interface NavigationExtras { * component: ChildComponent * }] * }] - * ``` - * - * Navigate to list route from child route: - * - * ``` - * @Component({...}) - * class ChildComponent { + * ``` + * + * Navigate to list route from child route: + * + * ``` + * @Component({...}) + * class ChildComponent { * constructor(private router: Router, private route: ActivatedRoute) {} * * go() { * this.router.navigate(['../list'], { relativeTo: this.route }); * } * } - * ``` - */ + * ``` + */ relativeTo?: ActivatedRoute|null; /** - * Sets query parameters to the URL. - * - * ``` - * // Navigate to /results?page=1 - * this.router.navigate(['/results'], { queryParams: { page: 1 } }); - * ``` - */ + * Sets query parameters to the URL. + * + * ``` + * // Navigate to /results?page=1 + * this.router.navigate(['/results'], { queryParams: { page: 1 } }); + * ``` + */ queryParams?: Params|null; /** - * Sets the hash fragment for the URL. - * - * ``` - * // Navigate to /results#top - * this.router.navigate(['/results'], { fragment: 'top' }); - * ``` - */ + * Sets the hash fragment for the URL. + * + * ``` + * // Navigate to /results#top + * this.router.navigate(['/results'], { fragment: 'top' }); + * ``` + */ fragment?: string; /** - * Preserves the query parameters for the next navigation. - * - * deprecated, use `queryParamsHandling` instead - * - * ``` - * // Preserve query params from /results?page=1 to /view?page=1 - * this.router.navigate(['/view'], { preserveQueryParams: true }); - * ``` - * - * @deprecated since v4 - */ + * Preserves the query parameters for the next navigation. + * + * deprecated, use `queryParamsHandling` instead + * + * ``` + * // Preserve query params from /results?page=1 to /view?page=1 + * this.router.navigate(['/view'], { preserveQueryParams: true }); + * ``` + * + * @deprecated since v4 + */ preserveQueryParams?: boolean; /** - * config strategy to handle the query parameters for the next navigation. - * - * ``` - * // from /results?page=1 to /view?page=1&page=2 - * this.router.navigate(['/view'], { queryParams: { page: 2 }, queryParamsHandling: "merge" }); - * ``` - */ + * config strategy to handle the query parameters for the next navigation. + * + * ``` + * // from /results?page=1 to /view?page=1&page=2 + * this.router.navigate(['/view'], { queryParams: { page: 2 }, queryParamsHandling: "merge" }); + * ``` + */ queryParamsHandling?: QueryParamsHandling|null; /** - * Preserves the fragment for the next navigation - * - * ``` - * // Preserve fragment from /results#top to /view#top - * this.router.navigate(['/view'], { preserveFragment: true }); - * ``` - */ + * Preserves the fragment for the next navigation + * + * ``` + * // Preserve fragment from /results#top to /view#top + * this.router.navigate(['/view'], { preserveFragment: true }); + * ``` + */ preserveFragment?: boolean; /** - * Navigates without pushing a new state into history. - * - * ``` - * // Navigate silently to /view - * this.router.navigate(['/view'], { skipLocationChange: true }); - * ``` - */ + * Navigates without pushing a new state into history. + * + * ``` + * // Navigate silently to /view + * this.router.navigate(['/view'], { skipLocationChange: true }); + * ``` + */ skipLocationChange?: boolean; /** - * Navigates while replacing the current state in history. - * - * ``` - * // Navigate to /view - * this.router.navigate(['/view'], { replaceUrl: true }); - * ``` - */ + * Navigates while replacing the current state in history. + * + * ``` + * // Navigate to /view + * this.router.navigate(['/view'], { replaceUrl: true }); + * ``` + */ replaceUrl?: boolean; } @@ -518,18 +518,11 @@ export class Router { // Because of a bug in IE and Edge, the location class fires two events (popstate and // hashchange) every single time. The second one should be ignored. Otherwise, the URL will - // flicker. Handles the case when a popstate was emitted first. + // flicker. if (lastNavigation && source == 'hashchange' && lastNavigation.source === 'popstate' && lastNavigation.rawUrl.toString() === rawUrl.toString()) { return Promise.resolve(true); // return value is not used } - // Because of a bug in IE and Edge, the location class fires two events (popstate and - // hashchange) every single time. The second one should be ignored. Otherwise, the URL will - // flicker. Handles the case when a hashchange was emitted first. - if (lastNavigation && source == 'popstate' && lastNavigation.source === 'hashchange' && - lastNavigation.rawUrl.toString() === rawUrl.toString()) { - return Promise.resolve(true); // return value is not used - } let resolve: any = null; let reject: any = null; @@ -552,7 +545,7 @@ export class Router { const url = this.urlHandlingStrategy.extract(rawUrl); const urlTransition = !this.navigated || url.toString() !== this.currentUrlTree.toString(); - if (this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) { + if (urlTransition && this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) { (this.events as Subject).next(new NavigationStart(id, this.serializeUrl(url))); Promise.resolve() .then( diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 4682d8b93e..5c78c3e083 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -50,29 +50,6 @@ describe('Integration', () => { expect(fixture.nativeElement).toHaveText('route'); }))); - it('should navigate to the current URL', - fakeAsync(inject([Router, Location], (router: Router) => { - router.resetConfig([ - {path: '', component: SimpleCmp}, - {path: 'simple', component: SimpleCmp}, - ]); - - const fixture = createRoot(router, RootCmp); - const events: Event[] = []; - router.events.subscribe(e => onlyNavigationStartAndEnd(e) && events.push(e)); - - router.navigateByUrl('/simple'); - tick(); - - router.navigateByUrl('/simple'); - tick(); - - expectEvents(events, [ - [NavigationStart, '/simple'], [NavigationEnd, '/simple'], [NavigationStart, '/simple'], - [NavigationEnd, '/simple'] - ]); - }))); - describe('should execute navigations serially', () => { let log: any[] = []; @@ -451,7 +428,7 @@ describe('Integration', () => { }]); const recordedEvents: any[] = []; - router.events.forEach(e => onlyNavigationStartAndEnd(e) && recordedEvents.push(e)); + router.events.forEach(e => e instanceof RouterEvent && recordedEvents.push(e)); router.navigateByUrl('/team/22/user/victor'); advance(fixture); @@ -465,8 +442,15 @@ describe('Integration', () => { expect(fixture.nativeElement).toHaveText('team 22 [ user fedor, right: ]'); expectEvents(recordedEvents, [ - [NavigationStart, '/team/22/user/victor'], [NavigationEnd, '/team/22/user/victor'], - [NavigationStart, '/team/22/user/fedor'], [NavigationEnd, '/team/22/user/fedor'] + [NavigationStart, '/team/22/user/victor'], [RoutesRecognized, '/team/22/user/victor'], + [GuardsCheckStart, '/team/22/user/victor'], [GuardsCheckEnd, '/team/22/user/victor'], + [ResolveStart, '/team/22/user/victor'], [ResolveEnd, '/team/22/user/victor'], + [NavigationEnd, '/team/22/user/victor'], + + [NavigationStart, '/team/22/user/fedor'], [RoutesRecognized, '/team/22/user/fedor'], + [GuardsCheckStart, '/team/22/user/fedor'], [GuardsCheckEnd, '/team/22/user/fedor'], + [ResolveStart, '/team/22/user/fedor'], [ResolveEnd, '/team/22/user/fedor'], + [NavigationEnd, '/team/22/user/fedor'] ]); }))); @@ -3505,30 +3489,34 @@ describe('Integration', () => { }]); const events: any[] = []; - router.events.subscribe(e => onlyNavigationStartAndEnd(e) && events.push(e)); + router.events.subscribe(e => e instanceof RouterEvent && events.push(e)); location.go('/include/user/kate(aux:excluded)'); advance(fixture); expect(location.path()).toEqual('/include/user/kate(aux:excluded)'); - expectEvents( - events, - [[NavigationStart, '/include/user/kate'], [NavigationEnd, '/include/user/kate']]); + expectEvents(events, [ + [NavigationStart, '/include/user/kate'], [RoutesRecognized, '/include/user/kate'], + [GuardsCheckStart, '/include/user/kate'], [GuardsCheckEnd, '/include/user/kate'], + [ResolveStart, '/include/user/kate'], [ResolveEnd, '/include/user/kate'], + [NavigationEnd, '/include/user/kate'] + ]); events.splice(0); location.go('/include/user/kate(aux:excluded2)'); advance(fixture); - expectEvents( - events, - [[NavigationStart, '/include/user/kate'], [NavigationEnd, '/include/user/kate']]); - events.splice(0); + expectEvents(events, []); router.navigateByUrl('/include/simple'); advance(fixture); expect(location.path()).toEqual('/include/simple(aux:excluded2)'); - expectEvents( - events, [[NavigationStart, '/include/simple'], [NavigationEnd, '/include/simple']]); + expectEvents(events, [ + [NavigationStart, '/include/simple'], [RoutesRecognized, '/include/simple'], + [GuardsCheckStart, '/include/simple'], [GuardsCheckEnd, '/include/simple'], + [ResolveStart, '/include/simple'], [ResolveEnd, '/include/simple'], + [NavigationEnd, '/include/simple'] + ]); }))); }); }); @@ -3647,10 +3635,6 @@ function expectEvents(events: Event[], pairs: any[]) { } } -function onlyNavigationStartAndEnd(e: Event): boolean { - return e instanceof NavigationStart || e instanceof NavigationEnd; -} - @Component( {selector: 'link-cmp', template: `link`}) class StringLinkCmp {