refactor(router): update test based on router quick-cancelling ongoing navigations (#25740)

PR Close #25740
This commit is contained in:
Jason Aden 2018-09-11 22:18:50 -07:00 committed by Kara Erickson
parent c634176035
commit 12ccf57340
6 changed files with 205 additions and 284 deletions

View File

@ -1,23 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {EMPTY, MonoTypeOperatorFunction, Observable, of } from 'rxjs';
import {mergeMap} from 'rxjs/operators';
export function mergeMapIf<T>(
predicate: (value: T) => boolean, tap: (value: T) => any): MonoTypeOperatorFunction<T> {
return (source: Observable<T>) => {
return source.pipe(mergeMap(s => {
if (predicate(s)) {
tap(s);
return EMPTY;
}
return of (s);
}));
};
}

View File

@ -11,8 +11,8 @@ import {map, mergeMap} from 'rxjs/operators';
import {NavigationTransition} from '../router';
export function resolveData(
paramsInheritanceStrategy: 'emptyOnly' | 'always'): MonoTypeOperatorFunction<NavigationTransition> {
export function resolveData(paramsInheritanceStrategy: 'emptyOnly' | 'always'):
MonoTypeOperatorFunction<NavigationTransition> {
return function(source: Observable<NavigationTransition>) {
return source.pipe(mergeMap(t => {
if (!t.preActivation) {

View File

@ -1,26 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {MonoTypeOperatorFunction, Observable} from 'rxjs';
import {tap} from 'rxjs/operators';
export function throwIf<T>(
predicate: (value: T) => boolean,
errorFactory: (() => any) = defaultErrorFactory): MonoTypeOperatorFunction<T> {
return (source: Observable<T>) => {
return source.pipe(tap(s => {
if (predicate(s)) {
throw errorFactory();
}
}));
};
}
function defaultErrorFactory() {
return new Error();
}

View File

@ -19,7 +19,6 @@ import {afterPreactivation} from './operators/after_preactivation';
import {applyRedirects} from './operators/apply_redirects';
import {beforePreactivation} from './operators/before_preactivation';
import {checkGuards} from './operators/check_guards';
import {mergeMapIf} from './operators/mergeMapIf';
import {recognize} from './operators/recognize';
import {resolveData} from './operators/resolve_data';
import {PreActivation} from './pre_activation';
@ -30,7 +29,6 @@ import {ActivatedRoute, ActivatedRouteSnapshot, RouterState, RouterStateSnapshot
import {Params, isNavigationCancelingError} from './shared';
import {DefaultUrlHandlingStrategy, UrlHandlingStrategy} from './url_handling_strategy';
import {UrlSerializer, UrlTree, containsTree, createEmptyUrlTree} from './url_tree';
import {assertDefined} from './utils/assert';
import {forEach} from './utils/collection';
import {TreeNode, nodeChildrenAsMap} from './utils/tree';
@ -370,24 +368,24 @@ export class Router {
this.processNavigations();
}
// clang-format off
private setupNavigations(transitions: Observable<NavigationTransition>):
Observable<NavigationTransition> {
return transitions.pipe(
filter(t => t.id !== 0), mergeMap(t => Promise.resolve(t)),
// Extract URL
map(t => ({
...t, //
extractedUrl: this.urlHandlingStrategy.extract(t.rawUrl), //
} as NavigationTransition)),
...t,
extractedUrl: this.urlHandlingStrategy.extract(t.rawUrl)} as NavigationTransition)),
// Using switchMap so we cancel executing navigations when a new one comes in
switchMap(t => {
let completed = false;
let errored = false;
return of (t).pipe(
mergeMap(t => {
const urlTransition =
!this.navigated || t.extractedUrl.toString() !== this.currentUrlTree.toString();
return of (t).pipe(mergeMap(t => {
const urlTransition = !this.navigated ||
t.extractedUrl.toString() !== this.currentUrlTree.toString();
if ((this.onSameUrlNavigation === 'reload' ? true : urlTransition) &&
this.urlHandlingStrategy.shouldProcessUrl(t.rawUrl)) {
return of (t).pipe(
@ -397,11 +395,10 @@ export class Router {
// Fire NavigationStart event
mergeMap(t => {
const transition = this.transitions.getValue();
(this.events as Subject<Event>)
.next(new NavigationStart(
(this.events as Subject<Event>).next(new NavigationStart(
t.id, this.serializeUrl(t.extractedUrl), t.source, t.state));
if (transition !== this.transitions.getValue()) {
EMPTY;
return EMPTY;
}
return [t];
}),
@ -412,33 +409,28 @@ export class Router {
// ApplyRedirects
applyRedirects(
this.ngModule.injector, this.configLoader, this.urlSerializer,
this.config),
this.ngModule.injector, this.configLoader, this.urlSerializer,this.config),
// Recognize
recognize(
this.rootComponentType, this.config, (url) => this.serializeUrl(url),
this.paramsInheritanceStrategy),
// Throw if there's no snapshot
tap(t => assertDefined(t.targetSnapshot, 'snapshot must be defined')),
// Fire RoutesRecognized
tap(t => (this.events as Subject<Event>)
.next(new RoutesRecognized(
t.id, this.serializeUrl(t.extractedUrl),
this.serializeUrl(t.urlAfterRedirects),
t.targetSnapshot !))), );
tap(t => (this.events as Subject<Event>).next(new RoutesRecognized(
t.id, this.serializeUrl(t.extractedUrl), this.serializeUrl(t.urlAfterRedirects),
t.targetSnapshot !)))
);
} else if (
urlTransition && this.rawUrlTree &&
this.urlHandlingStrategy.shouldProcessUrl(this.rawUrlTree)) {
(this.events as Subject<Event>)
.next(new NavigationStart(
(this.events as Subject<Event>).next(new NavigationStart(
t.id, this.serializeUrl(t.extractedUrl), t.source, t.state));
return of ({
...t,
urlAfterRedirects: t.extractedUrl,
extras: {...t.extras, skipLocationChange: false, replaceUrl: false},
targetSnapshot:
createEmptyState(t.extractedUrl, this.rootComponentType).snapshot
targetSnapshot: createEmptyState(t.extractedUrl, this.rootComponentType).snapshot
});
} else {
this.rawUrlTree = t.rawUrl;
@ -449,6 +441,7 @@ export class Router {
// Before Preactivation
beforePreactivation(this.hooks.beforePreactivation),
// --- GUARDS ---
tap(t => this.triggerEvent(new GuardsCheckStart(
t.id, this.serializeUrl(t.extractedUrl),
@ -460,18 +453,23 @@ export class Router {
preActivation.initialize(this.rootContexts);
return {...t, preActivation};
}),
checkGuards(), tap(t => this.triggerEvent(new GuardsCheckEnd(
checkGuards(),
tap(t => this.triggerEvent(new GuardsCheckEnd(
t.id, this.serializeUrl(t.extractedUrl),
this.serializeUrl(t.urlAfterRedirects), t.targetSnapshot !,
!!t.guardsResult))),
mergeMapIf(
t => !t.guardsResult,
t => {
mergeMap(t => {
if (!t.guardsResult) {
this.resetUrlToCurrentUrlTree();
(this.events as Subject<Event>)
.next(new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), ''));
t.resolve(false);
return EMPTY;
}
return of(t);
}),
// --- RESOLVE ---
@ -496,6 +494,13 @@ export class Router {
return ({...t, targetRouterState});
}),
// Side effects of resetting Router instance
afterPreactivation(this.hooks.afterPreactivation), map(t => {
const targetRouterState = createRouterState(
this.routeReuseStrategy, t.targetSnapshot !, t.currentRouterState);
return ({...t, targetRouterState});
}),
// Side effects of resetting Router instance
tap(t => {
this.currentUrlTree = t.urlAfterRedirects;
@ -515,8 +520,7 @@ export class Router {
tap({next: () => completed = true, complete: () => completed = true}),
finalize(() => {
if (!completed && !errored) {
(this.events as Subject<Event>)
.next(new NavigationCancel(
(this.events as Subject<Event>).next(new NavigationCancel(
t.id, this.serializeUrl(t.extractedUrl),
`Navigation ID ${t.id} is not equal to the current navigation id ${this.navigationId}`));
t.resolve(false);
@ -527,13 +531,12 @@ export class Router {
if (isNavigationCancelingError(e)) {
this.navigated = true;
this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl);
(this.events as Subject<Event>)
.next(
(this.events as Subject<Event>).next(
new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), e.message));
} else {
this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl);
(this.events as Subject<Event>)
.next(new NavigationError(t.id, this.serializeUrl(t.extractedUrl), e));
(this.events as Subject<Event>).next(new NavigationError(
t.id, this.serializeUrl(t.extractedUrl), e));
try {
t.resolve(this.errorHandler(e));
} catch (ee) {
@ -557,6 +560,7 @@ export class Router {
};
}
}
// clang-format on
/**
* @internal

View File

@ -1,19 +0,0 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
export function assertDefined<T>(actual: T, msg: string) {
if (actual == null) {
throwError(msg);
}
}
function throwError(msg: string): never {
// tslint:disable-next-line
debugger; // Left intentionally for better debugger experience.
throw new Error(`ASSERTION ERROR: ${msg}`);
}

View File

@ -291,15 +291,7 @@ describe('Integration', () => {
});
// TODO(jasonaden): This test now fails because it relies on waiting on a guard to finish
// executing even after a new navigation has been scheduled. The previous implementation
// would do so, but ignore the result of any guards that are executing when a new navigation
// is scheduled.
// With new implementation, the current navigation will be unrolled and cleaned up so the
// new navigation can start immediately. This test therefore fails as it relies on that
// previous incorrect behavior.
xit('should execute navigations serialy',
it('should not wait for prior navigations to start a new navigation',
fakeAsync(inject([Router, Location], (router: Router) => {
const fixture = createRoot(router, RootCmp);
@ -316,23 +308,17 @@ describe('Integration', () => {
tick(100); // 200
fixture.detectChanges();
expect(log).toEqual(['trueRightAway', 'trueIn2Seconds-start']);
expect(log).toEqual(
['trueRightAway', 'trueIn2Seconds-start', 'trueRightAway', 'trueIn2Seconds-start']);
tick(2000); // 2200
fixture.detectChanges();
expect(log).toEqual([
'trueRightAway', 'trueIn2Seconds-start', 'trueIn2Seconds-end', 'trueRightAway',
'trueIn2Seconds-start'
'trueRightAway', 'trueIn2Seconds-start', 'trueRightAway', 'trueIn2Seconds-start',
'trueIn2Seconds-end', 'trueIn2Seconds-end'
]);
tick(2000); // 4200
fixture.detectChanges();
expect(log).toEqual([
'trueRightAway', 'trueIn2Seconds-start', 'trueIn2Seconds-end', 'trueRightAway',
'trueIn2Seconds-start', 'trueIn2Seconds-end'
]);
})));
});
@ -2971,7 +2957,8 @@ describe('Integration', () => {
]);
})));
it('should allow redirection in NavigationStart', fakeAsync(inject([Router], (router: Router) => {
it('should allow redirection in NavigationStart',
fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig([
@ -2983,13 +2970,11 @@ describe('Integration', () => {
const recordedEvents: any[] = [];
const navStart$ = router.events.pipe(
tap(e => recordedEvents.push(e)),
filter(e => e instanceof NavigationStart),
first()
);
tap(e => recordedEvents.push(e)), filter(e => e instanceof NavigationStart), first());
navStart$.subscribe((e: NavigationStart | NavigationError) => {
router.navigate(['/blank'], {queryParams: {state: 'redirected'}, queryParamsHandling: 'merge'});
router.navigate(
['/blank'], {queryParams: {state: 'redirected'}, queryParamsHandling: 'merge'});
advance(fixture);
});