fixup! fix(router): support lazy loading for named outlets with empty…

… path
asvishnyakov · Sep 15, 2019 · Aug 4, 2020 · Aug 6, 2020 · Aug 6, 2020 · 7f5f43b846d18d2baa3093adecd27dab576fcfb4
commit 7f5f43b846d18d2baa3093adecd27dab576fcfb4
diff --git a/packages/router/src/apply_redirects.ts b/packages/router/src/apply_redirects.ts
@@ -7,7 +7,7 @@
  */
 
 import {Injector, NgModuleRef} from '@angular/core';
-import {combineLatest, EmptyError, Observable, Observer, of} from 'rxjs';
+import {combineLatest, EmptyError, Observable, Observer, of, throwError} from 'rxjs';
 import {catchError, first, map, mergeMap, tap} from 'rxjs/operators';
 
 import {LoadedRouterConfig, Route, Routes} from './config';
@@ -148,20 +148,25 @@ class ApplyRedirects {
       ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[],
       segments: UrlSegment[], outlet: string,
       allowRedirects: boolean): Observable<UrlSegmentGroup> {
-    return combineLatest(routes.map((r: any) => {
-             const expanded$ = this.expandSegmentAgainstRoute(
-                 ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
-             return expanded$.pipe(catchError((e: any) => {
-               if (e instanceof NoMatch) {
-                 // TODO(i): this return type doesn't match the declared Observable<UrlSegmentGroup>
-                 // talk to Jason
-                 return of(null) as any;
-               }
-               throw e;
-             }));
-           }))
+    const expandedSegments$ = routes.map((r: any) => {
+      try {
+        const expanded$ = this.expandSegmentAgainstRoute(
+            ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
+        return expanded$.pipe(catchError((e: any) => {
+          if (e instanceof NoMatch) {
+            return of(null);
+          }
+          throw e;
+        }));
+      } catch (e) {
+        // Ensure any errors thrown outside of an Observable are converted to Observable errors.
+        return throwError(e);
+      }
+    });
+    return combineLatest(expandedSegments$)
         .pipe(
-            map(x => x.find((s: any) => !!s)), first((s: any) => !!s),
+            map(resolved => resolved.find((s): s is UrlSegmentGroup => s !== null)),
+            first((s): s is UrlSegmentGroup => s !== undefined),
             catchError((e: any, _: any) => {
               if (e instanceof EmptyError || e.name === 'EmptyError') {
                 if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
@@ -170,7 +175,8 @@ class ApplyRedirects {
                 throw new NoMatch(segmentGroup);
               }
               throw e;
-            }));
+            }),
+        );
   }
 
   private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string):
@@ -181,6 +187,8 @@ class ApplyRedirects {
   private expandSegmentAgainstRoute(
       ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route,
       paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> {
+    // Empty string segments are special because multiple outlets can match a single config, i.e.
+    // `[{path: '', component: B}, {path: '', loadChildren: () => {}, outlet: "about"}]`
     if (getOutlet(route) !== outlet && route.path !== '') {
       return noMatch(segmentGroup);
     }

diff --git a/packages/router/test/apply_redirects.spec.ts b/packages/router/test/apply_redirects.spec.ts
@@ -7,8 +7,9 @@
  */
 
 import {NgModuleRef} from '@angular/core';
-import {TestBed} from '@angular/core/testing';
+import {fakeAsync, TestBed, tick} from '@angular/core/testing';
 import {Observable, of} from 'rxjs';
+import {delay, tap} from 'rxjs/operators';
 
 import {applyRedirects} from '../src/apply_redirects';
 import {LoadedRouterConfig, Route, Routes} from '../src/config';
@@ -463,26 +464,33 @@ describe('applyRedirects', () => {
           });
     });
 
-    it('should load the configuration of empty root path if the entry point is an auxiliary outlet',
-       () => {
+    it('should load all matching configurations of empty path, including an auxiliary outlets',
+       fakeAsync(() => {
          const loadedConfig =
              new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
+         let loadCalls = 0;
+         let loaded: string[] = [];
          const loader = {
-           load: (injector: any, p: any) => {
-             return of(loadedConfig);
+           load: (injector: any, p: Route) => {
+             loadCalls++;
+             return of(loadedConfig)
+                 .pipe(
+                     delay(100 * loadCalls),
+                     tap(() => loaded.push(p.loadChildren! as string)),
+                 );
            }
          };
 
-         const config: Routes = [
-           {path: '', loadChildren: 'root'}, {path: 'modal', loadChildren: 'aux', outlet: 'popup'}
-         ];
+         const config: Routes =
+             [{path: '', loadChildren: 'root'}, {path: '', loadChildren: 'aux', outlet: 'popup'}];
 
-         applyRedirects(testModule.injector, <any>loader, serializer, tree('(popup:modal)'), config)
-             .forEach(r => {
-               expectTreeToBe(r, '/(popup:modal)');
-               expect((config[0] as any)._loadedConfig).toBe(loadedConfig);
-             });
-       });
+         applyRedirects(testModule.injector, <any>loader, serializer, tree(''), config).subscribe();
+         expect(loadCalls).toBe(2);
+         tick(100);
+         expect(loaded).toEqual(['root']);
+         tick(100);
+         expect(loaded).toEqual(['root', 'aux']);
+       }));
   });
 
   describe('empty paths', () => {