feat(AsyncConfig): Refactor route config to use promises instead of c…

…allbacks

Promises are more zone-aware than callbacks. They also work better with rx operators like mergeMap, switchMap, etc.



BREAKING CHANGE: Before loadComponent, loadChildren, and loadIndexRoute used a callback to handle async loading

  of code. These must be replaced with promise-returning functions.



  Before:



  ```ts

  {

    loadIndex(done) {

      System.import('./my-index-route', __moduleName)

        .then(module => done(module.indexRoute));

    },

    loadComponent(done) {

      System.import('./my-component', __moduleName)

        .then(module => done(module.MyComponent));

    },

    loadChildren(done) {

      System.import('./child-routes', __moduleName)

        .then(module => done(module.routes));

    }

  }

  ```



  After:



  ```

  {

    loadIndex: () => System.import('./my-index-route', __moduleName)

      .then(module => module.indexRoute),



    loadComponent: () => System.import('./my-component', __moduleName)

      .then(module => module.MyComponent),



    loadChildren: () => System.import('./child-routes', __moduleName)

      .then(module => module.routes)

  }

  ```

docs/getting-started/code-splitting.md

@@ -11,24 +11,24 @@ import { Routes } from 'ngrx/router';
 export const routes: Routes = [
   {
     path: '/',
-    loadComponent(done) {
+    loadComponent: () => new Promise(resolve => {
       require.ensure([], require => {
-        done(require('./pages/home').HomePage);
+        resolve(require('./pages/home').HomePage);
       })
-    }
+    })
   },
   {
     path: '/blog',
-    loadComponent(done) {
+    loadComponent: () => new Promise(resolve => {
       require.ensure([], require => {
-        done(require('./pages/blog').BlogPage);
+        resolve(require('./pages/blog').BlogPage);
       })
-    },
-    loadChildren(done) {
+    }),
+    loadChildren: () => new Promise(resolve => {
       require.ensure([], require => {
-        done(require('./blog-routes').blogRoutes);
+        resolve(require('./blog-routes').blogRoutes);
       })
-    }
+    })
   }
 ]
 ```
@@ -40,11 +40,11 @@ import { Routes } from 'ngrx/router';
 export const blogRoutes: Routes = [
   {
     path: ':id',
-    loadComponent(done) {
+    loadComponent: () => new Promise(resolve => {
       require.ensure([], require => {
-        done(require('./pages/post').PostPage);
+        resolve(require('./pages/post').PostPage);
       });
-    }
+    })
   }
 ]
 ```

docs/getting-started/guards.md

@@ -39,14 +39,10 @@ const routes: Routes = [
   {
     path: '/account',
     guards: [ authGuard ],
-    loadComponent(done) {
-      System.load('/pages/account')
-        .then(module => done(module.AccountPage));
-    },
-    loadChildren(done) {
-      System.load('/routes/account')
-        .then(module => done(module.accountRoutes));
-    }
+    loadComponent: () => System.import('/pages/account', __moduleName)
+      .then(module => module.AccountPage),
+    loadChildren: () => System.import('/routes/account', __moduleName))
+      .then(module => module.accountRoutes),
   }
 ]
 ```

lib/component-renderer.ts

@@ -1,4 +1,3 @@
-import 'rxjs/add/observable/fromPromise';
 import 'rxjs/add/observable/of';
 import 'rxjs/add/operator/let';
 import 'rxjs/add/operator/map';
@@ -17,7 +16,8 @@ import {
 } from 'angular2/core';
 import { Observable } from 'rxjs/Observable';
 
-import { fromCallback, compose } from './util';
+import { Async, ResourceLoader } from './resource-loader';
+import { compose } from './util';
 import { Route } from './route';
 import { Middleware, identity, provideMiddlewareForToken } from './middleware';
 
@@ -53,6 +53,7 @@ export interface ResolvedInstruction {
 @Injectable()
 export class ComponentRenderer {
   constructor(
+    private _loader: ResourceLoader,
     @Inject(PRE_RENDER_MIDDLEWARE) private _preMiddleware: Middleware[],
     @Inject(POST_RENDER_MIDDLEWARE) private _postMiddleware: Middleware[]
   ) { }
@@ -64,7 +65,8 @@ export class ComponentRenderer {
     dcl: DynamicComponentLoader,
     providers: Provider[]
   ) {
-    return this._loadComponent(route)
+    return Observable.of(route)
+      .mergeMap(route => this._loadComponent(route))
       .map<RenderInstruction>(component => {
         return { component, injector, providers };
       })
@@ -80,28 +82,11 @@ export class ComponentRenderer {
   }
 
   renderComponent({ component, providers, ref, dcl }: ResolvedInstruction) {
-    return Observable.fromPromise(dcl.loadNextToLocation(
-      component, ref, providers
-    ));
+    return dcl.loadNextToLocation(component, ref, providers);
   }
 
-  /**
-   * :-(
-   *
-   * We tried to use Observable.of and Observable.bindCallback, but zones...
-   */
-  private _loadComponent(route: Route): Observable<Type> {
-    let promise: Promise<Type>;
-
-    if ( !!route.component ) {
-      promise = Promise.resolve(route.component);
-    }
-
-    else if ( !!route.loadComponent ) {
-      promise = new Promise(resolve => route.loadComponent(resolve));
-    }
-
-    return Observable.fromPromise(promise);
+  private _loadComponent(route: Route): Promise<Type> {
+    return this._loader.load(route.component, route.loadComponent);
   }
 }
 

lib/match-route.ts

@@ -5,7 +5,6 @@
 */
 import 'rxjs/add/observable/of';
 import 'rxjs/add/observable/from';
-import 'rxjs/add/observable/fromPromise';
 import 'rxjs/add/operator/mergeMap';
 import 'rxjs/add/operator/let';
 import 'rxjs/add/operator/catch';
@@ -15,7 +14,8 @@ import 'rxjs/add/operator/take';
 import { Observable } from 'rxjs/Observable';
 import { OpaqueToken, Provider, Inject, Injectable } from 'angular2/core';
 
-import { Callback, compose } from './util';
+import { ResourceLoader, Async } from './resource-loader';
+import { compose } from './util';
 import { matchPattern } from './match-pattern';
 import { Route, IndexRoute, Routes, ROUTES } from './route';
 import { Middleware, provideMiddlewareForToken, identity } from './middleware';
@@ -43,6 +43,7 @@ export interface TraversalCandidate {
 @Injectable()
 export class RouteTraverser {
   constructor(
+    private _loader: ResourceLoader,
     @Inject(TRAVERSAL_MIDDLEWARE) private _middleware: Middleware[],
     @Inject(ROUTES) private _routes: Routes
   ) { }
@@ -121,7 +122,8 @@ export class RouteTraverser {
             params
           };
 
-          return this._loadIndexRoute(route)
+          return Observable.of(route)
+            .mergeMap(route => this._loadIndexRoute(route))
             .map(indexRoute => {
               if ( !!indexRoute ) {
                 match.routes.push(indexRoute);
@@ -131,7 +133,8 @@ export class RouteTraverser {
             });
         }
 
-        return this._loadChildRoutes(route)
+        return Observable.of(route)
+          .mergeMap(route => this._loadChildRoutes(route))
           .mergeMap<Match>(childRoutes => this._matchRoutes(
             childRoutes,
             pathname,
@@ -151,33 +154,12 @@ export class RouteTraverser {
       });
   }
 
-  private _loadChildRoutes(route: Route): Observable<Routes> {
-    return this._loadResource(route.children, route.loadChildren, []);
+  private _loadChildRoutes(route: Route): Promise<Routes> {
+    return this._loader.load(route.children, route.loadChildren, []);
   }
 
-  private _loadIndexRoute(route: Route): Observable<Route> {
-    return this._loadResource(route.indexRoute, route.loadIndexRoute, null);
-  }
-
-  /**
-   * :-(
-   *
-   * We tried to use Observable.of and Observable.bindCallback, but zones...
-   */
-  private _loadResource<T>(sync: T, async: Callback<T>, defaultValue: any): Observable<T> {
-    let promise: Promise<T>;
-
-    if (!!sync) {
-      promise = Promise.resolve(sync);
-    }
-    else if (!!async) {
-      promise = new Promise<T>(resolve => async(resolve));
-    }
-    else {
-      promise = Promise.resolve(defaultValue);
-    }
-
-    return Observable.fromPromise(promise);
+  private _loadIndexRoute(route: Route): Promise<Route> {
+    return this._loader.load(route.indexRoute, route.loadIndexRoute, null);
   }
 }
 

lib/resource-loader.ts

@@ -0,0 +1,23 @@
+import { Injectable, Provider } from 'angular2/core';
+
+export type Async<T> = () => Promise<T>;
+
+@Injectable()
+export class ResourceLoader {
+  load<T>(sync: T, async: Async<T>, defaultValue?: any): Promise<T> {
+    if (!!sync) {
+      return Promise.resolve(sync);
+    }
+
+    else if (!!async) {
+      return Promise.resolve(async());
+    }
+
+    return Promise.resolve(defaultValue);
+  }
+}
+
+
+export const RESOURCE_LOADER_PROVIDERS = [
+  new Provider(ResourceLoader, { useClass: ResourceLoader })
+];

lib/route.ts

@@ -4,23 +4,23 @@
 import { Observable } from 'rxjs/Observable';
 import { Provider, Type, OpaqueToken } from 'angular2/core';
 
-import { Callback } from './util';
+import { Async } from './resource-loader';
 
 export type Routes = Array<Route>;
 
 export interface IndexRoute {
   component?: Type;
-  loadComponent?: Callback<Type>;
+  loadComponent?: Async<Type>;
   redirectTo?: string;
 }
 
 export interface Route extends IndexRoute {
   path?: string;
   guards?: Provider[];
   indexRoute?: IndexRoute;
-  loadIndexRoute?: Callback<IndexRoute>;
+  loadIndexRoute?: Async<IndexRoute>;
   children?: Routes;
-  loadChildren?: Callback<Routes>;
+  loadChildren?: Async<Routes>;
 }
 
 export const ROUTES = new OpaqueToken('@ngrx/router Init Routes');

lib/util.ts

@@ -1,14 +1,7 @@
-import 'rxjs/add/observable/bindCallback';
 import { Observable } from 'rxjs/Observable';
 import { Subscriber } from 'rxjs/Subscriber';
 import { provide, Provider, OpaqueToken } from 'angular2/core';
 
-export type Callback<T> = (callback: (value: T) => void ) => void;
-
-export function fromCallback<T>(fn: Callback<T>) {
-  return Observable.bindCallback(fn)();
-}
-
 export function createFactoryProvider<T>(
   name: string,
   token = new OpaqueToken(name),

spec/component-renderer.spec.ts

@@ -2,6 +2,8 @@ import 'rxjs/add/operator/toPromise';
 import { Subject } from 'rxjs/Subject';
 import { Observable } from 'rxjs/Observable';
 import { Injector, provide, DynamicComponentLoader, ElementRef } from 'angular2/core';
+
+import { RESOURCE_LOADER_PROVIDERS } from '../lib/resource-loader';
 import { Location } from '../lib/location';
 import {
   ComponentRenderer,
@@ -35,7 +37,8 @@ describe('Component Renderer', function() {
         ComponentRenderer,
         provide(PRE_RENDER_MIDDLEWARE, { useValue: [] }),
         provide(POST_RENDER_MIDDLEWARE, { useValue: [] }),
-        provide(DynamicComponentLoader, {useClass: MockDCL})
+        provide(DynamicComponentLoader, {useClass: MockDCL}),
+        RESOURCE_LOADER_PROVIDERS
       ]);
 
       renderer = injector.get(ComponentRenderer);
@@ -54,10 +57,9 @@ describe('Component Renderer', function() {
 
     it('should render a loaded component', (done) => {
       route = {
-        loadComponent(cb) {
-          cb(TestComponent);
-        }
+        loadComponent: () => Promise.resolve(TestComponent)
       };
+
       let render = renderer.render(route, injector, <ElementRef>elementRef, loader, providers);
 
       render.subscribe(() => {
@@ -95,7 +97,8 @@ describe('Component Renderer', function() {
         ComponentRenderer,
         usePreRenderMiddleware(renderMiddleware),
         provide(POST_RENDER_MIDDLEWARE, {useValue: []}),
-        provide(DynamicComponentLoader, {useClass: MockDCL})
+        provide(DynamicComponentLoader, {useClass: MockDCL}),
+        RESOURCE_LOADER_PROVIDERS
       ]);
 
       renderer = injector.get(ComponentRenderer);
@@ -152,7 +155,8 @@ describe('Component Renderer', function() {
         ComponentRenderer,
         usePostRenderMiddleware(renderMiddleware),
         provide(PRE_RENDER_MIDDLEWARE, {useValue: []}),
-        provide(DynamicComponentLoader, {useClass: MockDCL})
+        provide(DynamicComponentLoader, {useClass: MockDCL}),
+        RESOURCE_LOADER_PROVIDERS
       ]);
 
       renderer = injector.get(ComponentRenderer);

spec/match-route.spec.ts

@@ -1,4 +1,6 @@
 import { Injector, provide } from 'angular2/core';
+
+import { RESOURCE_LOADER_PROVIDERS } from '../lib/resource-loader';
 import { Routes, Route, ROUTES } from '../lib/route';
 import { RouteTraverser, MATCH_ROUTE_PROVIDERS } from '../lib/match-route';
 
@@ -70,6 +72,7 @@ describe('RouteTraverser', function() {
   beforeEach(function() {
     const injector = Injector.resolveAndCreate([
       MATCH_ROUTE_PROVIDERS,
+      RESOURCE_LOADER_PROVIDERS,
       provide(ROUTES, { useValue: routes })
     ]);
     traverser = injector.get(RouteTraverser);
@@ -267,19 +270,15 @@ describe('RouteTraverser', function() {
         if ( children ) {
           delete route.children;
 
-          route.loadChildren = function(done) {
-            setTimeout(() => done(children));
-          };
+          route.loadChildren = () => Promise.resolve(children);
 
           makeAsyncRouteConfig(children);
         }
 
         if ( indexRoute ) {
           delete route.indexRoute;
 
-          route.loadIndexRoute = function(done) {
-            setTimeout(() => done(indexRoute));
-          };
+          route.loadIndexRoute = () => Promise.resolve(indexRoute);
         }
       });
     }

spec/util.spec.ts

@@ -1,22 +1,8 @@
 import { Injector, Provider, OpaqueToken } from 'angular2/core';
-import { Callback, fromCallback, createFactoryProvider, compose } from '../lib/util';
+import { createFactoryProvider, compose } from '../lib/util';
 
 
 describe('Router Utilities', function() {
-  describe('fromCallback Helper', function() {
-    it('should wrap a callback function in an observable', function(done) {
-      const callback: Callback<number> = function(done) {
-        done(123);
-      };
-
-      fromCallback(callback).subscribe(value => {
-        expect(value).toBe(123);
-
-        done();
-      });
-    });
-  });
-
   describe('createFactoryProvider Helper', function() {
     it('should create a factory function for anonymous providers', function() {
       const factory = createFactoryProvider('test');