fix(router): cancel navigation when at least one resolver completes with no "next" emission (#24621)

This change aligns behavior for resolvers which return EMPTY. Currently EMPTY resolvers have inconsistent behavior:
- One resolver that returns EMPTY => won't navigate and just ends on ResolveStart router event.
- Two resolvers where both return EMPTY => throws "Error: Uncaught (in promise): EmptyError: no elements in sequence"
- Two resolvers where one returns a value and the other one returns EMPTY => Navigates successfully.
With this change any EMPTY resolver will cancel navigation.

BREAKING CHANGE: Any resolver which return EMPTY will cancel navigation.
If you want to allow the navigation to continue, you will need to update the resolvers to emit
some value, (i.e. defaultIfEmpty(...), of(...), etc).
PR Close #24195

PR Close #24621
This commit is contained in:
Martin Sikora 2020-04-17 11:48:19 +02:00 committed by Alex Rickabaugh
parent 12fcc7cafe
commit d9c4840a9c
5 changed files with 296 additions and 25 deletions

View File

@ -2956,7 +2956,7 @@ Update the `CrisisDetailComponent` to get the crisis from the `ActivatedRoute.d
<code-example path="router/src/app/crisis-center/crisis-detail/crisis-detail.component.ts" header="src/app/crisis-center/crisis-detail/crisis-detail.component.ts (ngOnInit v2)" region="ngOnInit"></code-example>
Note the following two important points:
Note the following three important points:
1. The router's `Resolve` interface is optional.
The `CrisisDetailResolverService` doesn't inherit from a base class.
@ -2964,6 +2964,8 @@ The router looks for that method and calls it if found.
1. The router calls the resolver in any case where the the user could navigate away so you don't have to code for each use case.
1. Returning an empty `Observable` in at least one resolver will cancel navigation.
The relevant Crisis Center code for this milestone follows.
<code-tabs>

View File

@ -7,8 +7,8 @@
*/
import {Injector} from '@angular/core';
import {from, MonoTypeOperatorFunction, Observable, of} from 'rxjs';
import {concatMap, last, map, mergeMap, reduce} from 'rxjs/operators';
import {EMPTY, from, MonoTypeOperatorFunction, Observable, of} from 'rxjs';
import {concatMap, map, mergeMap, takeLast, tap} from 'rxjs/operators';
import {ResolveData} from '../config';
import {NavigationTransition} from '../router';
@ -26,13 +26,16 @@ export function resolveData(
if (!canActivateChecks.length) {
return of(t);
}
let canActivateChecksResolved = 0;
return from(canActivateChecks)
.pipe(
concatMap(
check => runResolve(
check.route, targetSnapshot!, paramsInheritanceStrategy, moduleInjector)),
reduce((_: any, __: any) => _), map(_ => t));
tap(() => canActivateChecksResolved++),
takeLast(1),
mergeMap(_ => canActivateChecksResolved === canActivateChecks.length ? of(t) : EMPTY),
);
}));
};
}
@ -59,22 +62,23 @@ function resolveNode(
if (keys.length === 0) {
return of({});
}
if (keys.length === 1) {
const key = keys[0];
return getResolver(resolve[key], futureARS, futureRSS, moduleInjector)
.pipe(map((value: any) => {
return {[key]: value};
}));
}
const data: {[k: string]: any} = {};
const runningResolvers$ = from(keys).pipe(mergeMap((key: string) => {
return getResolver(resolve[key], futureARS, futureRSS, moduleInjector)
.pipe(map((value: any) => {
data[key] = value;
return value;
}));
}));
return runningResolvers$.pipe(last(), map(() => data));
return from(keys).pipe(
mergeMap(
(key: string) => getResolver(resolve[key], futureARS, futureRSS, moduleInjector)
.pipe(tap((value: any) => {
data[key] = value;
}))),
takeLast(1),
mergeMap(() => {
// Ensure all resolvers returned values, otherwise don't emit any "next" and just complete
// the chain which will cancel navigation
if (Object.keys(data).length === keys.length) {
return of(data);
}
return EMPTY;
}),
);
}
function getResolver(
@ -83,4 +87,4 @@ function getResolver(
const resolver = getToken(injectionToken, futureARS, moduleInjector);
return resolver.resolve ? wrapIntoObservable(resolver.resolve(futureARS, futureRSS)) :
wrapIntoObservable(resolver(futureARS, futureRSS));
}
}

View File

@ -639,9 +639,25 @@ export class Router {
this.serializeUrl(t.urlAfterRedirects), t.targetSnapshot!);
this.triggerEvent(resolveStart);
}),
resolveData(
this.paramsInheritanceStrategy,
this.ngModule.injector), //
switchMap(t => {
let dataResolved = false;
return of(t).pipe(
resolveData(
this.paramsInheritanceStrategy, this.ngModule.injector),
tap({
next: () => dataResolved = true,
complete: () => {
if (!dataResolved) {
const navCancel = new NavigationCancel(
t.id, this.serializeUrl(t.extractedUrl),
`At least one route resolver didn't emit any value.`);
eventsSubject.next(navCancel);
t.resolve(false);
}
}
}),
);
}),
tap(t => {
const resolveEnd = new ResolveEnd(
t.id, this.serializeUrl(t.extractedUrl),

View File

@ -13,7 +13,7 @@ import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/
import {By} from '@angular/platform-browser/src/dom/debug/by';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DefaultUrlSerializer, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, Navigation, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, ParamMap, Params, PreloadAllModules, PreloadingStrategy, PRIMARY_OUTLET, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, Router, RouteReuseStrategy, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlSerializer, UrlTree} from '@angular/router';
import {Observable, Observer, of, Subscription} from 'rxjs';
import {EMPTY, Observable, Observer, of, Subscription} from 'rxjs';
import {filter, first, map, tap} from 'rxjs/operators';
import {forEach} from '../src/utils/collection';
@ -1564,6 +1564,7 @@ describe('Integration', () => {
{provide: 'resolveSix', useClass: ResolveSix},
{provide: 'resolveError', useValue: (a: any, b: any) => Promise.reject('error')},
{provide: 'resolveNullError', useValue: (a: any, b: any) => Promise.reject(null)},
{provide: 'resolveEmpty', useValue: (a: any, b: any) => EMPTY},
{provide: 'numberOfUrlSegments', useValue: (a: any, b: any) => a.url.length},
]
});
@ -1647,6 +1648,152 @@ describe('Integration', () => {
expect(e).toEqual(null);
})));
it('should not navigate when all resolvers return empty result',
fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig([
{path: 'simple', component: SimpleCmp, resolve: {e1: 'resolveEmpty', e2: 'resolveEmpty'}}
]);
const recordedEvents: any[] = [];
router.events.subscribe(e => e instanceof RouterEvent && recordedEvents.push(e));
let e: any = null;
router.navigateByUrl('/simple').catch(error => e = error);
advance(fixture);
expectEvents(recordedEvents, [
[NavigationStart, '/simple'],
[RoutesRecognized, '/simple'],
[GuardsCheckStart, '/simple'],
[GuardsCheckEnd, '/simple'],
[ResolveStart, '/simple'],
[NavigationCancel, '/simple'],
]);
expect(e).toEqual(null);
})));
it('should not navigate when at least one resolver returns empty result',
fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig([
{path: 'simple', component: SimpleCmp, resolve: {e1: 'resolveTwo', e2: 'resolveEmpty'}}
]);
const recordedEvents: any[] = [];
router.events.subscribe(e => e instanceof RouterEvent && recordedEvents.push(e));
let e: any = null;
router.navigateByUrl('/simple').catch(error => e = error);
advance(fixture);
expectEvents(recordedEvents, [
[NavigationStart, '/simple'],
[RoutesRecognized, '/simple'],
[GuardsCheckStart, '/simple'],
[GuardsCheckEnd, '/simple'],
[ResolveStart, '/simple'],
[NavigationCancel, '/simple'],
]);
expect(e).toEqual(null);
})));
it('should not navigate when all resolvers for a child route from forChild() returns empty result',
fakeAsync(inject(
[Router, Location, NgModuleFactoryLoader],
(router: Router, location: Location, loader: SpyNgModuleFactoryLoader) => {
const fixture = createRoot(router, RootCmp);
@Component({selector: 'lazy-cmp', template: 'lazy-loaded-1'})
class LazyComponent1 {
}
router.resetConfig([{path: 'lazy', loadChildren: 'expected1'}]);
@NgModule({
declarations: [LazyComponent1],
imports: [
RouterModule.forChild([{
path: 'loaded',
component: LazyComponent1,
resolve: {e1: 'resolveEmpty', e2: 'resolveEmpty'}
}]),
],
})
class LoadedModule {
}
loader.stubbedModules = {expected1: LoadedModule};
const recordedEvents: any[] = [];
router.events.subscribe(e => e instanceof RouterEvent && recordedEvents.push(e));
let e: any = null;
router.navigateByUrl('lazy/loaded').catch(error => e = error);
advance(fixture);
expectEvents(recordedEvents, [
[NavigationStart, '/lazy/loaded'],
[RoutesRecognized, '/lazy/loaded'],
[GuardsCheckStart, '/lazy/loaded'],
[GuardsCheckEnd, '/lazy/loaded'],
[ResolveStart, '/lazy/loaded'],
[NavigationCancel, '/lazy/loaded'],
]);
expect(e).toEqual(null);
})));
it('should not navigate when at least one resolver for a child route from forChild() returns empty result',
fakeAsync(inject(
[Router, Location, NgModuleFactoryLoader],
(router: Router, location: Location, loader: SpyNgModuleFactoryLoader) => {
const fixture = createRoot(router, RootCmp);
@Component({selector: 'lazy-cmp', template: 'lazy-loaded-1'})
class LazyComponent1 {
}
router.resetConfig([{path: 'lazy', loadChildren: 'expected1'}]);
@NgModule({
declarations: [LazyComponent1],
imports: [
RouterModule.forChild([{
path: 'loaded',
component: LazyComponent1,
resolve: {e1: 'resolveTwo', e2: 'resolveEmpty'}
}]),
],
})
class LoadedModule {
}
loader.stubbedModules = {expected1: LoadedModule};
const recordedEvents: any[] = [];
router.events.subscribe(e => e instanceof RouterEvent && recordedEvents.push(e));
let e: any = null;
router.navigateByUrl('lazy/loaded').catch(error => e = error);
advance(fixture);
expectEvents(recordedEvents, [
[NavigationStart, '/lazy/loaded'],
[RoutesRecognized, '/lazy/loaded'],
[GuardsCheckStart, '/lazy/loaded'],
[GuardsCheckEnd, '/lazy/loaded'],
[ResolveStart, '/lazy/loaded'],
[NavigationCancel, '/lazy/loaded'],
]);
expect(e).toEqual(null);
})));
it('should preserve resolved data', fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp);

View File

@ -0,0 +1,102 @@
/**
* @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 {Injector} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {EMPTY, of} from 'rxjs';
import {TestScheduler} from 'rxjs/testing';
import {resolveData} from '../../src/operators/resolve_data';
describe('resolveData operator', () => {
let testScheduler: TestScheduler;
let injector: Injector;
beforeEach(() => {
TestBed.configureTestingModule({
providers: [
{provide: 'resolveTwo', useValue: (a: any, b: any) => of(2)},
{provide: 'resolveFour', useValue: (a: any, b: any) => 4},
{provide: 'resolveEmpty', useValue: (a: any, b: any) => EMPTY},
]
});
});
beforeEach(() => {
testScheduler = new TestScheduler(assertDeepEquals);
});
beforeEach(() => {
injector = TestBed.inject<Injector>(Injector);
});
it('should re-emit updated value from source after all resolvers emit and complete', () => {
testScheduler.run(({hot, cold, expectObservable}) => {
const transition: any = createTransition({e1: 'resolveTwo'}, {e2: 'resolveFour'});
const source = cold('-(t|)', {t: deepClone(transition)});
const expected = '-(t|)';
const outputTransition = deepClone(transition);
outputTransition.guards.canActivateChecks[0].route._resolvedData = {e1: 2};
outputTransition.guards.canActivateChecks[1].route._resolvedData = {e2: 4};
expectObservable(source.pipe(resolveData('emptyOnly', injector))).toBe(expected, {
t: outputTransition
});
});
});
it('should re-emit value from source when there are no resolvers', () => {
testScheduler.run(({hot, cold, expectObservable}) => {
const transition: any = createTransition({});
const source = cold('-(t|)', {t: deepClone(transition)});
const expected = '-(t|)';
const outputTransition = deepClone(transition);
outputTransition.guards.canActivateChecks[0].route._resolvedData = {};
expectObservable(source.pipe(resolveData('emptyOnly', injector))).toBe(expected, {
t: outputTransition
});
});
});
it('should not emit when there\'s one resolver that doesn\'t emit', () => {
testScheduler.run(({hot, cold, expectObservable}) => {
const transition: any = createTransition({e2: 'resolveEmpty'});
const source = cold('-(t|)', {t: deepClone(transition)});
const expected = '-|';
expectObservable(source.pipe(resolveData('emptyOnly', injector))).toBe(expected);
});
});
it('should not emit if at least one resolver doesn\'t emit', () => {
testScheduler.run(({hot, cold, expectObservable}) => {
const transition: any = createTransition({e1: 'resolveTwo'}, {e2: 'resolveEmpty'});
const source = cold('-(t|)', {t: deepClone(transition)});
const expected = '-|';
expectObservable(source.pipe(resolveData('emptyOnly', injector))).toBe(expected);
});
});
});
function assertDeepEquals(a: any, b: any) {
return expect(a).toEqual(b);
}
function createTransition(...resolvers: {[key: string]: string}[]) {
return {
targetSnapshot: {},
guards: {
canActivateChecks:
resolvers.map(resolver => ({
route: {_resolve: resolver, pathFromRoot: [{url: '/'}], data: {}},
})),
},
};
}
function deepClone(obj: any) {
return JSON.parse(JSON.stringify(obj));
}