fix(router): should run resolvers for the same route concurrently

Fixes angular#14279
DzmitryShylovich · Mar 29, 2017 · 0c1c390 · 0c1c390
1 parent 4916278
commit 0c1c390
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 7 deletions.
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -17,6 +17,7 @@ import {of } from 'rxjs/observable/of';
 import {concatMap} from 'rxjs/operator/concatMap';
 import {every} from 'rxjs/operator/every';
 import {first} from 'rxjs/operator/first';
+import {last} from 'rxjs/operator/last';
 import {map} from 'rxjs/operator/map';
 import {mergeMap} from 'rxjs/operator/mergeMap';
 import {reduce} from 'rxjs/operator/reduce';
@@ -991,11 +992,24 @@ export class PreActivation {
   }
 
   private resolveNode(resolve: ResolveData, future: ActivatedRouteSnapshot): Observable<any> {
-    return waitForMap(resolve, (k, v) => {
-      const resolver = this.getToken(v, future);
-      return resolver.resolve ? wrapIntoObservable(resolver.resolve(future, this.future)) :
-                                wrapIntoObservable(resolver(future, this.future));
+    const data: {[k: string]: any} = {};
+    const keys = Object.keys(resolve);
+    if (keys.length === 0) {
+      return of (data);
+    }
+    const runningResolvers$ = mergeMap.call(from(keys), (key: string) => {
+      return map.call(this.getResolver(resolve[key], future), (value: any) => {
+        data[key] = value;
+        return value;
+      });
     });
+    return map.call(last.call(runningResolvers$), () => data);
+  }
+
+  private getResolver(injectionToken: any, future: ActivatedRouteSnapshot): Observable<any> {
+    const resolver = this.getToken(injectionToken, future);
+    return resolver.resolve ? wrapIntoObservable(resolver.resolve(future, this.future)) :
+                              wrapIntoObservable(resolver(future, this.future));
   }
 
   private getToken(token: any, snapshot: ActivatedRouteSnapshot): any {

diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -844,24 +844,37 @@ describe('Integration', () => {
      })));
 
   describe('data', () => {
+    let log: string[];
     class ResolveSix implements Resolve<number> {
       resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): number { return 6; }
     }
 
     beforeEach(() => {
+      log = [];
       TestBed.configureTestingModule({
         providers: [
           {provide: 'resolveTwo', useValue: (a: any, b: any) => 2},
           {provide: 'resolveFour', useValue: (a: any, b: any) => 4},
           {provide: 'resolveSix', useClass: ResolveSix},
           {provide: 'resolveError', useValue: (a: any, b: any) => Promise.reject('error')},
-          {provide: 'numberOfUrlSegments', useValue: (a: any, b: any) => a.url.length}
+          {provide: 'numberOfUrlSegments', useValue: (a: any, b: any) => a.url.length},
+          {
+            provide: 'resolver1',
+            useValue: () => delayPromise(10).then(() => log.push('resolver1'))
+          },
+          {provide: 'resolver2', useValue: () => delayPromise(5).then(() => log.push('resolver2'))},
         ]
       });
     });
 
-    it('should provide resolved data',
-       fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
+    function delayPromise(delay: number): Promise<void> {
+      let resolve: () => void;
+      const promise = new Promise<void>(res => resolve = res);
+      setTimeout(() => resolve(), delay);
+      return promise;
+    }
+
+    it('should provide resolved data', fakeAsync(inject([Router], (router: Router) => {
          const fixture = createRoot(router, RootCmpWithTwoOutlets);
 
          router.resetConfig([{
@@ -966,6 +979,26 @@ describe('Integration', () => {
 
          expect(cmp.route.snapshot.data).toEqual({numberOfUrlSegments: 3});
        })));
+
+    it('should run resolvers for the same route concurrently',
+       fakeAsync(inject([Router], (router: Router) => {
+         const fixture = createRoot(router, RootCmp);
+
+         router.resetConfig([{
+           path: 'a',
+           resolve: {
+             one: 'resolver1',
+             two: 'resolver2',
+           },
+           component: SimpleCmp
+         }]);
+
+         router.navigateByUrl('/a');
+         advance(fixture);
+         tick(10);
+
+         expect(log).toEqual(['resolver2', 'resolver1']);
+       })));
   });
 
   describe('router links', () => {