Sitelet https://github.com/angular/angular/commit/9c51ba3
Skip to content

Commit 9c51ba3

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
fix(router): Ensure routes are processed in priority order and only if needed (#38780)
There is a slight difference between `map`...`concatAll` and `concatMap` in that the latter (`concatMap`) will ensure that the computations are executed in-order and only if needed while the former may execute the `map` body of all items if they do not emit immediately. That is, if the stream is `from([a, b, c]).pipe(map(v => of(v).pipe(delay(1))), concatAll(), first())` the `map` body will execute for all of `a`, `b`, and `c`. However, the following will only execute the `concatMap` body for `a` `from([a, b, c]).pipe(concatMap(v => of(v).pipe(delay(1))), first())` See https://stackblitz.com/edit/rxjs-cvwxyx fixes #38691 PR Close #38780
1 parent d8714d0 commit 9c51ba3

2 files changed

Lines changed: 36 additions & 12 deletions

File tree

‎packages/router/src/apply_redirects.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88

99
import {Injector, NgModuleRef} from '@angular/core';
10-
import {defer, EmptyError, Observable, Observer, of} from 'rxjs';
11-
import {catchError, concatAll, first, map, mergeMap, tap} from 'rxjs/operators';
10+
import {EmptyError, Observable, Observer, of} from 'rxjs';
11+
import {catchError, concatMap, first, map, mergeMap, tap} from 'rxjs/operators';
1212

1313
import {LoadedRouterConfig, Route, Routes} from './config';
1414
import {CanLoadFn} from './interfaces';
@@ -149,7 +149,7 @@ class ApplyRedirects {
149149
segments: UrlSegment[], outlet: string,
150150
allowRedirects: boolean): Observable<UrlSegmentGroup> {
151151
return of(...routes).pipe(
152-
map((r: any) => {
152+
concatMap((r: any) => {
153153
const expanded$ = this.expandSegmentAgainstRoute(
154154
ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
155155
return expanded$.pipe(catchError((e: any) => {
@@ -161,7 +161,7 @@ class ApplyRedirects {
161161
throw e;
162162
}));
163163
}),
164-
concatAll(), first((s: any) => !!s), catchError((e: any, _: any) => {
164+
first((s: any) => !!s), catchError((e: any, _: any) => {
165165
if (e instanceof EmptyError || e.name === 'EmptyError') {
166166
if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
167167
return of(new UrlSegmentGroup([], {}));
@@ -247,12 +247,11 @@ class ApplyRedirects {
247247
segments: UrlSegment[]): Observable<UrlSegmentGroup> {
248248
if (route.path === '**') {
249249
if (route.loadChildren) {
250-
return defer(
251-
() => this.configLoader.load(ngModule.injector, route)
252-
.pipe(map((cfg: LoadedRouterConfig) => {
253-
route._loadedConfig = cfg;
254-
return new UrlSegmentGroup(segments, {});
255-
})));
250+
return this.configLoader.load(ngModule.injector, route)
251+
.pipe(map((cfg: LoadedRouterConfig) => {
252+
route._loadedConfig = cfg;
253+
return new UrlSegmentGroup(segments, {});
254+
}));
256255
}
257256

258257
return of(new UrlSegmentGroup(segments, {}));

‎packages/router/test/integration.spec.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4090,7 +4090,7 @@ describe('Integration', () => {
40904090
return of(delayMs).pipe(delay(delayMs), mapTo(true));
40914091
}
40924092

4093-
@NgModule()
4093+
@NgModule({imports: [RouterModule.forChild([{path: '', component: BlankCmp}])]})
40944094
class LoadedModule {
40954095
}
40964096

@@ -4119,6 +4119,15 @@ describe('Integration', () => {
41194119
return false;
41204120
}
41214121
},
4122+
{
4123+
provide: 'returnFalseAndNavigate',
4124+
useFactory: (router: Router) => () => {
4125+
log.push('returnFalseAndNavigate');
4126+
router.navigateByUrl('/redirected');
4127+
return false;
4128+
},
4129+
deps: [Router]
4130+
},
41224131
{
41234132
provide: 'returnUrlTree',
41244133
useFactory: (router: Router) => () => {
@@ -4132,7 +4141,23 @@ describe('Integration', () => {
41324141
});
41334142
});
41344143

4135-
it('should wait for higher priority guards to be resolved',
4144+
it('should only execute canLoad guards of routes being activated', fakeAsync(() => {
4145+
const router = TestBed.inject(Router);
4146+
4147+
router.resetConfig([
4148+
{path: 'lazy', canLoad: ['guard1'], loadChildren: () => of(LoadedModule)},
4149+
{path: 'redirected', component: SimpleCmp},
4150+
// canLoad should not run for this route because 'lazy' activates first
4151+
{path: '', canLoad: ['returnFalseAndNavigate'], loadChildren: () => of(LoadedModule)},
4152+
]);
4153+
4154+
router.navigateByUrl('/lazy');
4155+
tick(5);
4156+
expect(log.length).toEqual(1);
4157+
expect(log).toEqual(['guard1']);
4158+
}));
4159+
4160+
it('should execute canLoad guards',
41364161
fakeAsync(inject(
41374162
[Router, NgModuleFactoryLoader],
41384163
(router: Router, loader: SpyNgModuleFactoryLoader) => {

0 commit comments

Comments
 (0)