fix(router): Ensure routes are processed in priority order and only i…

…f 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

packages/router/src/apply_redirects.ts

@@ -7,8 +7,8 @@
  */
 
 import {Injector, NgModuleRef} from '@angular/core';
-import {defer, EmptyError, Observable, Observer, of} from 'rxjs';
-import {catchError, concatAll, first, map, mergeMap, tap} from 'rxjs/operators';
+import {EmptyError, Observable, Observer, of} from 'rxjs';
+import {catchError, concatMap, first, map, mergeMap, tap} from 'rxjs/operators';
 
 import {LoadedRouterConfig, Route, Routes} from './config';
 import {CanLoadFn} from './interfaces';
@@ -149,7 +149,7 @@ class ApplyRedirects {
       segments: UrlSegment[], outlet: string,
       allowRedirects: boolean): Observable<UrlSegmentGroup> {
     return of(...routes).pipe(
-        map((r: any) => {
+        concatMap((r: any) => {
           const expanded$ = this.expandSegmentAgainstRoute(
               ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
           return expanded$.pipe(catchError((e: any) => {
@@ -161,7 +161,7 @@ class ApplyRedirects {
             throw e;
           }));
         }),
-        concatAll(), first((s: any) => !!s), catchError((e: any, _: any) => {
+        first((s: any) => !!s), catchError((e: any, _: any) => {
           if (e instanceof EmptyError || e.name === 'EmptyError') {
             if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
               return of(new UrlSegmentGroup([], {}));
@@ -247,12 +247,11 @@ class ApplyRedirects {
       segments: UrlSegment[]): Observable<UrlSegmentGroup> {
     if (route.path === '**') {
       if (route.loadChildren) {
-        return defer(
-            () => this.configLoader.load(ngModule.injector, route)
-                      .pipe(map((cfg: LoadedRouterConfig) => {
-                        route._loadedConfig = cfg;
-                        return new UrlSegmentGroup(segments, {});
-                      })));
+        return this.configLoader.load(ngModule.injector, route)
+            .pipe(map((cfg: LoadedRouterConfig) => {
+              route._loadedConfig = cfg;
+              return new UrlSegmentGroup(segments, {});
+            }));
       }
 
       return of(new UrlSegmentGroup(segments, {}));

packages/router/test/integration.spec.ts

@@ -4090,7 +4090,7 @@ describe('Integration', () => {
         return of(delayMs).pipe(delay(delayMs), mapTo(true));
       }
 
-      @NgModule()
+      @NgModule({imports: [RouterModule.forChild([{path: '', component: BlankCmp}])]})
       class LoadedModule {
       }
 
@@ -4119,6 +4119,15 @@ describe('Integration', () => {
                 return false;
               }
             },
+            {
+              provide: 'returnFalseAndNavigate',
+              useFactory: (router: Router) => () => {
+                log.push('returnFalseAndNavigate');
+                router.navigateByUrl('/redirected');
+                return false;
+              },
+              deps: [Router]
+            },
             {
               provide: 'returnUrlTree',
               useFactory: (router: Router) => () => {
@@ -4132,7 +4141,23 @@ describe('Integration', () => {
         });
       });
 
-      it('should wait for higher priority guards to be resolved',
+      it('should only execute canLoad guards of routes being activated', fakeAsync(() => {
+           const router = TestBed.inject(Router);
+
+           router.resetConfig([
+             {path: 'lazy', canLoad: ['guard1'], loadChildren: () => of(LoadedModule)},
+             {path: 'redirected', component: SimpleCmp},
+             // canLoad should not run for this route because 'lazy' activates first
+             {path: '', canLoad: ['returnFalseAndNavigate'], loadChildren: () => of(LoadedModule)},
+           ]);
+
+           router.navigateByUrl('/lazy');
+           tick(5);
+           expect(log.length).toEqual(1);
+           expect(log).toEqual(['guard1']);
+         }));
+
+      it('should execute canLoad guards',
          fakeAsync(inject(
              [Router, NgModuleFactoryLoader],
              (router: Router, loader: SpyNgModuleFactoryLoader) => {