From 3b7bab7d22064d545f1745dc3e062fcb1d91073b Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 24 Jan 2018 12:19:59 -0500 Subject: [PATCH] feat(router): add navigationSource and restoredState to NavigationStart event (#21728) Currently, NavigationStart there is no way to know if an navigation was triggered imperatively or via the location change. These two use cases should be handled differently for a variety of use cases (e.g., scroll position restoration). This PR adds a navigation source field and restored navigation id (passed to navigations triggered by a URL change). PR Close #21728 --- packages/common/src/location/location.ts | 10 +-- .../common/src/location/platform_location.ts | 5 +- packages/common/testing/src/location_mock.ts | 22 +++--- packages/platform-server/src/location.ts | 5 +- packages/router/src/events.ts | 47 +++++++++++++ packages/router/src/router.ts | 44 +++++++----- packages/router/test/integration.spec.ts | 69 +++++++++++++++++++ tools/public_api_guard/common/common.d.ts | 6 +- tools/public_api_guard/common/testing.d.ts | 4 +- tools/public_api_guard/router/router.d.ts | 11 +++ 10 files changed, 183 insertions(+), 40 deletions(-) diff --git a/packages/common/src/location/location.ts b/packages/common/src/location/location.ts index d3997e4f90..c09bce6258 100644 --- a/packages/common/src/location/location.ts +++ b/packages/common/src/location/location.ts @@ -14,6 +14,7 @@ import {LocationStrategy} from './location_strategy'; /** @experimental */ export interface PopStateEvent { pop?: boolean; + state?: any; type?: string; url?: string; } @@ -56,6 +57,7 @@ export class Location { this._subject.emit({ 'url': this.path(true), 'pop': true, + 'state': ev.state, 'type': ev.type, }); }); @@ -103,16 +105,16 @@ export class Location { * Changes the browsers URL to the normalized version of the given URL, and pushes a * new item onto the platform's history. */ - go(path: string, query: string = ''): void { - this._platformStrategy.pushState(null, '', path, query); + go(path: string, query: string = '', state: any = null): void { + this._platformStrategy.pushState(state, '', path, query); } /** * Changes the browsers URL to the normalized version of the given URL, and replaces * the top item on the platform's history stack. */ - replaceState(path: string, query: string = ''): void { - this._platformStrategy.replaceState(null, '', path, query); + replaceState(path: string, query: string = '', state: any = null): void { + this._platformStrategy.replaceState(state, '', path, query); } /** diff --git a/packages/common/src/location/platform_location.ts b/packages/common/src/location/platform_location.ts index 0f5cb5f32e..177b6a474a 100644 --- a/packages/common/src/location/platform_location.ts +++ b/packages/common/src/location/platform_location.ts @@ -58,7 +58,10 @@ export const LOCATION_INITIALIZED = new InjectionToken>('Location I * * @experimental */ -export interface LocationChangeEvent { type: string; } +export interface LocationChangeEvent { + type: string; + state: any; +} /** * @experimental diff --git a/packages/common/testing/src/location_mock.ts b/packages/common/testing/src/location_mock.ts index 55b71a7e08..802d9978f1 100644 --- a/packages/common/testing/src/location_mock.ts +++ b/packages/common/testing/src/location_mock.ts @@ -19,7 +19,7 @@ import {ISubscription} from 'rxjs/Subscription'; @Injectable() export class SpyLocation implements Location { urlChanges: string[] = []; - private _history: LocationState[] = [new LocationState('', '')]; + private _history: LocationState[] = [new LocationState('', '', null)]; private _historyIndex: number = 0; /** @internal */ _subject: EventEmitter = new EventEmitter(); @@ -34,6 +34,8 @@ export class SpyLocation implements Location { path(): string { return this._history[this._historyIndex].path; } + private state(): string { return this._history[this._historyIndex].state; } + isCurrentPathEqualTo(path: string, query: string = ''): boolean { const givenPath = path.endsWith('/') ? path.substring(0, path.length - 1) : path; const currPath = @@ -60,13 +62,13 @@ export class SpyLocation implements Location { return this._baseHref + url; } - go(path: string, query: string = '') { + go(path: string, query: string = '', state: any = null) { path = this.prepareExternalUrl(path); if (this._historyIndex > 0) { this._history.splice(this._historyIndex + 1); } - this._history.push(new LocationState(path, query)); + this._history.push(new LocationState(path, query, state)); this._historyIndex = this._history.length - 1; const locationState = this._history[this._historyIndex - 1]; @@ -79,7 +81,7 @@ export class SpyLocation implements Location { this._subject.emit({'url': url, 'pop': false}); } - replaceState(path: string, query: string = '') { + replaceState(path: string, query: string = '', state: any = null) { path = this.prepareExternalUrl(path); const history = this._history[this._historyIndex]; @@ -89,6 +91,7 @@ export class SpyLocation implements Location { history.path = path; history.query = query; + history.state = state; const url = path + (query.length > 0 ? ('?' + query) : ''); this.urlChanges.push('replace: ' + url); @@ -97,14 +100,14 @@ export class SpyLocation implements Location { forward() { if (this._historyIndex < (this._history.length - 1)) { this._historyIndex++; - this._subject.emit({'url': this.path(), 'pop': true}); + this._subject.emit({'url': this.path(), 'state': this.state(), 'pop': true}); } } back() { if (this._historyIndex > 0) { this._historyIndex--; - this._subject.emit({'url': this.path(), 'pop': true}); + this._subject.emit({'url': this.path(), 'state': this.state(), 'pop': true}); } } @@ -118,10 +121,5 @@ export class SpyLocation implements Location { } class LocationState { - path: string; - query: string; - constructor(path: string, query: string) { - this.path = path; - this.query = query; - } + constructor(public path: string, public query: string, public state: any) {} } diff --git a/packages/platform-server/src/location.ts b/packages/platform-server/src/location.ts index b4983799cb..34d1d71425 100644 --- a/packages/platform-server/src/location.ts +++ b/packages/platform-server/src/location.ts @@ -63,8 +63,9 @@ export class ServerPlatformLocation implements PlatformLocation { } (this as{hash: string}).hash = value; const newUrl = this.url; - scheduleMicroTask( - () => this._hashUpdate.next({ type: 'hashchange', oldUrl, newUrl } as LocationChangeEvent)); + scheduleMicroTask(() => this._hashUpdate.next({ + type: 'hashchange', state: null, oldUrl, newUrl + } as LocationChangeEvent)); } replaceState(state: any, title: string, newUrl: string): void { diff --git a/packages/router/src/events.ts b/packages/router/src/events.ts index b6a5d46fca..67e1508499 100644 --- a/packages/router/src/events.ts +++ b/packages/router/src/events.ts @@ -9,6 +9,16 @@ import {Route} from './config'; import {ActivatedRouteSnapshot, RouterStateSnapshot} from './router_state'; +/** + * @whatItDoes Identifies the trigger of the navigation. + * + * * 'imperative'--triggered by `router.navigateByUrl` or `router.navigate`. + * * 'popstate'--triggered by a popstate event + * * 'hashchange'--triggered by a hashchange event + * + * @experimental + */ +export type NavigationTrigger = 'imperative' | 'popstate' | 'hashchange'; /** * @whatItDoes Base for events the Router goes through, as opposed to events tied to a specific @@ -42,6 +52,43 @@ export class RouterEvent { * @stable */ export class NavigationStart extends RouterEvent { + /** + * Identifies the trigger of the navigation. + * + * * 'imperative'--triggered by `router.navigateByUrl` or `router.navigate`. + * * 'popstate'--triggered by a popstate event + * * 'hashchange'--triggered by a hashchange event + */ + navigationTrigger?: 'imperative'|'popstate'|'hashchange'; + + /** + * This contains the navigation id that pushed the history record that the router navigates + * back to. This is not null only when the navigation is triggered by a popstate event. + * + * The router assigns a navigationId to every router transition/navigation. Even when the user + * clicks on the back button in the browser, a new navigation id will be created. So from + * the perspective of the router, the router never "goes back". By using the `restoredState` + * and its navigationId, you can implement behavior that differentiates between creating new + * states + * and popstate events. In the latter case you can restore some remembered state (e.g., scroll + * position). + */ + restoredState?: {navigationId: number}|null; + + constructor( + /** @docsNotRequired */ + id: number, + /** @docsNotRequired */ + url: string, + /** @docsNotRequired */ + navigationTrigger: 'imperative'|'popstate'|'hashchange' = 'imperative', + /** @docsNotRequired */ + restoredState: {navigationId: number}|null = null) { + super(id, url); + this.navigationTrigger = navigationTrigger; + this.restoredState = restoredState; + } + /** @docsNotRequired */ toString(): string { return `NavigationStart(id: ${this.id}, url: '${this.url}')`; } } diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 4c1f278c3c..fcbdeb7627 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -21,7 +21,7 @@ import {applyRedirects} from './apply_redirects'; import {LoadedRouterConfig, QueryParamsHandling, Route, Routes, validateConfig} from './config'; import {createRouterState} from './create_router_state'; import {createUrlTree} from './create_url_tree'; -import {ActivationEnd, ChildActivationEnd, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RoutesRecognized} from './events'; +import {ActivationEnd, ChildActivationEnd, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, NavigationTrigger, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RoutesRecognized} from './events'; import {PreActivation} from './pre_activation'; import {recognize} from './recognize'; import {DefaultRouteReuseStrategy, DetachedRouteHandleInternal, RouteReuseStrategy} from './route_reuse_strategy'; @@ -164,8 +164,6 @@ function defaultErrorHandler(error: any): any { throw error; } -type NavigationSource = 'imperative' | 'popstate' | 'hashchange'; - type NavigationParams = { id: number, rawUrl: UrlTree, @@ -173,7 +171,8 @@ type NavigationParams = { resolve: any, reject: any, promise: Promise, - source: NavigationSource, + source: NavigationTrigger, + state: {navigationId: number} | null }; /** @@ -223,6 +222,7 @@ export class Router { * Indicates if at least one navigation happened. */ navigated: boolean = false; + private lastSuccessfulId: number = -1; /** * Used by RouterModule. This allows us to @@ -311,8 +311,12 @@ export class Router { if (!this.locationSubscription) { this.locationSubscription = this.location.subscribe(Zone.current.wrap((change: any) => { const rawUrlTree = this.urlSerializer.parse(change['url']); - const source: NavigationSource = change['type'] === 'popstate' ? 'popstate' : 'hashchange'; - setTimeout(() => { this.scheduleNavigation(rawUrlTree, source, {replaceUrl: true}); }, 0); + const source: NavigationTrigger = change['type'] === 'popstate' ? 'popstate' : 'hashchange'; + const state = change.state && change.state.navigationId ? + {navigationId: change.state.navigationId} : + null; + setTimeout( + () => { this.scheduleNavigation(rawUrlTree, source, state, {replaceUrl: true}); }, 0); })); } } @@ -341,6 +345,7 @@ export class Router { validateConfig(config); this.config = config; this.navigated = false; + this.lastSuccessfulId = -1; } /** @docsNotRequired */ @@ -449,7 +454,7 @@ export class Router { const urlTree = url instanceof UrlTree ? url : this.parseUrl(url); const mergedTree = this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree); - return this.scheduleNavigation(mergedTree, 'imperative', extras); + return this.scheduleNavigation(mergedTree, 'imperative', null, extras); } /** @@ -522,8 +527,9 @@ export class Router { .subscribe(() => {}); } - private scheduleNavigation(rawUrl: UrlTree, source: NavigationSource, extras: NavigationExtras): - Promise { + private scheduleNavigation( + rawUrl: UrlTree, source: NavigationTrigger, state: {navigationId: number}|null, + extras: NavigationExtras): Promise { const lastNavigation = this.navigations.value; // If the user triggers a navigation imperatively (e.g., by using navigateByUrl), @@ -558,21 +564,22 @@ export class Router { }); const id = ++this.navigationId; - this.navigations.next({id, source, rawUrl, extras, resolve, reject, promise}); + this.navigations.next({id, source, state, 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, extras, resolve, reject}: NavigationParams): - void { + private executeScheduledNavigation({id, rawUrl, extras, resolve, reject, source, + state}: NavigationParams): void { const url = this.urlHandlingStrategy.extract(rawUrl); const urlTransition = !this.navigated || url.toString() !== this.currentUrlTree.toString(); if ((this.onSameUrlNavigation === 'reload' ? true : urlTransition) && this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) { - (this.events as Subject).next(new NavigationStart(id, this.serializeUrl(url))); + (this.events as Subject) + .next(new NavigationStart(id, this.serializeUrl(url), source, state)); Promise.resolve() .then( (_) => this.runNavigate( @@ -584,7 +591,8 @@ export class Router { } else if ( urlTransition && this.rawUrlTree && this.urlHandlingStrategy.shouldProcessUrl(this.rawUrlTree)) { - (this.events as Subject).next(new NavigationStart(id, this.serializeUrl(url))); + (this.events as Subject) + .next(new NavigationStart(id, this.serializeUrl(url), source, state)); Promise.resolve() .then( (_) => this.runNavigate( @@ -727,9 +735,9 @@ export class Router { if (!skipLocationChange) { const path = this.urlSerializer.serialize(this.rawUrlTree); if (this.location.isCurrentPathEqualTo(path) || replaceUrl) { - this.location.replaceState(path); + this.location.replaceState(path, '', {navigationId: id}); } else { - this.location.go(path); + this.location.go(path, '', {navigationId: id}); } } @@ -743,6 +751,7 @@ export class Router { () => { if (navigationIsSuccessful) { this.navigated = true; + this.lastSuccessfulId = id; (this.events as Subject) .next(new NavigationEnd( id, this.serializeUrl(url), this.serializeUrl(this.currentUrlTree))); @@ -784,7 +793,8 @@ export class Router { } private resetUrlToCurrentUrlTree(): void { - this.location.replaceState(this.urlSerializer.serialize(this.rawUrlTree)); + this.location.replaceState( + this.urlSerializer.serialize(this.rawUrlTree), '', {navigationId: this.lastSuccessfulId}); } } diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 8df1c43ef8..9a09b4ba26 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -76,6 +76,28 @@ describe('Integration', () => { ]); }))); + it('should set the restoredState to null when executing imperative navigations', + fakeAsync(inject([Router], (router: Router) => { + router.resetConfig([ + {path: '', component: SimpleCmp}, + {path: 'simple', component: SimpleCmp}, + ]); + + const fixture = createRoot(router, RootCmp); + let event: NavigationStart; + router.events.subscribe(e => { + if (e instanceof NavigationStart) { + event = e; + } + }); + + router.navigateByUrl('/simple'); + tick(); + + expect(event !.navigationTrigger).toEqual('imperative'); + expect(event !.restoredState).toEqual(null); + }))); + it('should not pollute browser history when replaceUrl is set to true', fakeAsync(inject([Router, Location], (router: Router, location: SpyLocation) => { router.resetConfig([ @@ -466,21 +488,34 @@ describe('Integration', () => { [{path: 'simple', component: SimpleCmp}, {path: 'user/:name', component: UserCmp}] }]); + let event: NavigationStart; + router.events.subscribe(e => { + if (e instanceof NavigationStart) { + event = e; + } + }); router.navigateByUrl('/team/33/simple'); advance(fixture); expect(location.path()).toEqual('/team/33/simple'); + const simpleNavStart = event !; router.navigateByUrl('/team/22/user/victor'); advance(fixture); + const userVictorNavStart = event !; + location.back(); advance(fixture); expect(location.path()).toEqual('/team/33/simple'); + expect(event !.navigationTrigger).toEqual('hashchange'); + expect(event !.restoredState !.navigationId).toEqual(simpleNavStart.id); location.forward(); advance(fixture); expect(location.path()).toEqual('/team/22/user/victor'); + expect(event !.navigationTrigger).toEqual('hashchange'); + expect(event !.restoredState !.navigationId).toEqual(userVictorNavStart.id); }))); it('should navigate to the same url when config changes', @@ -868,6 +903,40 @@ describe('Integration', () => { expect(locationUrlBeforeEmittingError).toEqual('/simple'); })); + it('should reset the url with the right state when navigation errors', fakeAsync(() => { + const router: Router = TestBed.get(Router); + const location: SpyLocation = TestBed.get(Location); + const fixture = createRoot(router, RootCmp); + + router.resetConfig([ + {path: 'simple1', component: SimpleCmp}, {path: 'simple2', component: SimpleCmp}, + {path: 'throwing', component: ThrowingCmp} + ]); + + + let event: NavigationStart; + router.events.subscribe(e => { + if (e instanceof NavigationStart) { + event = e; + } + }); + + router.navigateByUrl('/simple1'); + advance(fixture); + const simple1NavStart = event !; + + router.navigateByUrl('/throwing').catch(() => null); + advance(fixture); + + router.navigateByUrl('/simple2'); + advance(fixture); + + location.back(); + tick(); + + expect(event !.restoredState !.navigationId).toEqual(simple1NavStart.id); + })); + it('should not trigger another navigation when resetting the url back due to a NavigationError', fakeAsync(() => { const router = TestBed.get(Router); diff --git a/tools/public_api_guard/common/common.d.ts b/tools/public_api_guard/common/common.d.ts index 40d879e2ea..7edceead7b 100644 --- a/tools/public_api_guard/common/common.d.ts +++ b/tools/public_api_guard/common/common.d.ts @@ -182,12 +182,12 @@ export declare class Location { constructor(platformStrategy: LocationStrategy); back(): void; forward(): void; - go(path: string, query?: string): void; + go(path: string, query?: string, state?: any): void; isCurrentPathEqualTo(path: string, query?: string): boolean; normalize(url: string): string; path(includeHash?: boolean): string; prepareExternalUrl(url: string): string; - replaceState(path: string, query?: string): void; + replaceState(path: string, query?: string, state?: any): void; subscribe(onNext: (value: PopStateEvent) => void, onThrow?: ((exception: any) => void) | null, onReturn?: (() => void) | null): ISubscription; static joinWithSlash(start: string, end: string): string; static normalizeQueryParams(params: string): string; @@ -199,6 +199,7 @@ export declare const LOCATION_INITIALIZED: InjectionToken>; /** @experimental */ export interface LocationChangeEvent { + state: any; type: string; } @@ -415,6 +416,7 @@ export declare enum Plural { /** @experimental */ export interface PopStateEvent { pop?: boolean; + state?: any; type?: string; url?: string; } diff --git a/tools/public_api_guard/common/testing.d.ts b/tools/public_api_guard/common/testing.d.ts index edac756fc1..e428ab6692 100644 --- a/tools/public_api_guard/common/testing.d.ts +++ b/tools/public_api_guard/common/testing.d.ts @@ -21,12 +21,12 @@ export declare class SpyLocation implements Location { urlChanges: string[]; back(): void; forward(): void; - go(path: string, query?: string): void; + go(path: string, query?: string, state?: any): void; isCurrentPathEqualTo(path: string, query?: string): boolean; normalize(url: string): string; path(): string; prepareExternalUrl(url: string): string; - replaceState(path: string, query?: string): void; + replaceState(path: string, query?: string, state?: any): void; setBaseHref(url: string): void; setInitialPath(url: string): void; simulateHashChange(pathname: string): void; diff --git a/tools/public_api_guard/router/router.d.ts b/tools/public_api_guard/router/router.d.ts index f9f0524226..7fa17a655e 100644 --- a/tools/public_api_guard/router/router.d.ts +++ b/tools/public_api_guard/router/router.d.ts @@ -208,6 +208,17 @@ export interface NavigationExtras { /** @stable */ export declare class NavigationStart extends RouterEvent { + navigationTrigger?: 'imperative' | 'popstate' | 'hashchange'; + restoredState?: { + navigationId: number; + } | null; + constructor( + id: number, + url: string, + navigationTrigger?: 'imperative' | 'popstate' | 'hashchange', + restoredState?: { + navigationId: number; + } | null); toString(): string; }