From af2c0b34a251a597e9142bb87e325b0c0ba7d407 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 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). --- 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 d3997e4f90484..c09bce6258093 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 0f5cb5f32ec96..177b6a474a538 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 55b71a7e08453..802d9978f1378 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 b4983799cbde7..34d1d71425578 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 b6a5d46fca465..67e1508499226 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 4c1f278c3c4e0..fcbdeb76277a5 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 8df1c43ef88a0..9a09b4ba26acb 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 40d879e2eade9..7edceead7b398 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 edac756fc1fb2..e428ab66923b9 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 f9f05242266cf..7fa17a655eb1d 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; }