From f057d556a3d84da9863fd4bb82b88c9645f0ac42 Mon Sep 17 00:00:00 2001 From: Mike Ryan Date: Tue, 5 Apr 2016 18:18:45 -0500 Subject: [PATCH] feat(AsyncConfig): Refactor route config to use promises instead of callbacks 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 | 24 ++++++++-------- docs/getting-started/guards.md | 12 +++----- lib/component-renderer.ts | 31 ++++++-------------- lib/match-route.ts | 40 +++++++------------------- lib/resource-loader.ts | 23 +++++++++++++++ lib/route.ts | 8 +++--- lib/util.ts | 7 ----- spec/component-renderer.spec.ts | 16 +++++++---- spec/match-route.spec.ts | 11 ++++--- spec/util.spec.ts | 16 +---------- 10 files changed, 78 insertions(+), 110 deletions(-) create mode 100644 lib/resource-loader.ts diff --git a/docs/getting-started/code-splitting.md b/docs/getting-started/code-splitting.md index 3950885..ab52489 100644 --- a/docs/getting-started/code-splitting.md +++ b/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); }); - } + }) } ] ``` diff --git a/docs/getting-started/guards.md b/docs/getting-started/guards.md index 4493276..c4b8994 100644 --- a/docs/getting-started/guards.md +++ b/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), } ] ``` diff --git a/lib/component-renderer.ts b/lib/component-renderer.ts index 4eb3f0a..0279f95 100644 --- a/lib/component-renderer.ts +++ b/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(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 { - let promise: Promise; - - 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 { + return this._loader.load(route.component, route.loadComponent); } } diff --git a/lib/match-route.ts b/lib/match-route.ts index 11b82a7..1e7753c 100644 --- a/lib/match-route.ts +++ b/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(childRoutes => this._matchRoutes( childRoutes, pathname, @@ -151,33 +154,12 @@ export class RouteTraverser { }); } - private _loadChildRoutes(route: Route): Observable { - return this._loadResource(route.children, route.loadChildren, []); + private _loadChildRoutes(route: Route): Promise { + return this._loader.load(route.children, route.loadChildren, []); } - private _loadIndexRoute(route: Route): Observable { - return this._loadResource(route.indexRoute, route.loadIndexRoute, null); - } - - /** - * :-( - * - * We tried to use Observable.of and Observable.bindCallback, but zones... - */ - private _loadResource(sync: T, async: Callback, defaultValue: any): Observable { - let promise: Promise; - - if (!!sync) { - promise = Promise.resolve(sync); - } - else if (!!async) { - promise = new Promise(resolve => async(resolve)); - } - else { - promise = Promise.resolve(defaultValue); - } - - return Observable.fromPromise(promise); + private _loadIndexRoute(route: Route): Promise { + return this._loader.load(route.indexRoute, route.loadIndexRoute, null); } } diff --git a/lib/resource-loader.ts b/lib/resource-loader.ts new file mode 100644 index 0000000..0bef996 --- /dev/null +++ b/lib/resource-loader.ts @@ -0,0 +1,23 @@ +import { Injectable, Provider } from 'angular2/core'; + +export type Async = () => Promise; + +@Injectable() +export class ResourceLoader { + load(sync: T, async: Async, defaultValue?: any): Promise { + 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 }) +]; diff --git a/lib/route.ts b/lib/route.ts index f3edda0..e223de9 100644 --- a/lib/route.ts +++ b/lib/route.ts @@ -4,13 +4,13 @@ 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; export interface IndexRoute { component?: Type; - loadComponent?: Callback; + loadComponent?: Async; redirectTo?: string; } @@ -18,9 +18,9 @@ export interface Route extends IndexRoute { path?: string; guards?: Provider[]; indexRoute?: IndexRoute; - loadIndexRoute?: Callback; + loadIndexRoute?: Async; children?: Routes; - loadChildren?: Callback; + loadChildren?: Async; } export const ROUTES = new OpaqueToken('@ngrx/router Init Routes'); diff --git a/lib/util.ts b/lib/util.ts index 813850d..13681ee 100644 --- a/lib/util.ts +++ b/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 = (callback: (value: T) => void ) => void; - -export function fromCallback(fn: Callback) { - return Observable.bindCallback(fn)(); -} - export function createFactoryProvider( name: string, token = new OpaqueToken(name), diff --git a/spec/component-renderer.spec.ts b/spec/component-renderer.spec.ts index 139bf97..4ab74a1 100644 --- a/spec/component-renderer.spec.ts +++ b/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, 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); diff --git a/spec/match-route.spec.ts b/spec/match-route.spec.ts index 7500763..fdea1f7 100644 --- a/spec/match-route.spec.ts +++ b/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,9 +270,7 @@ describe('RouteTraverser', function() { if ( children ) { delete route.children; - route.loadChildren = function(done) { - setTimeout(() => done(children)); - }; + route.loadChildren = () => Promise.resolve(children); makeAsyncRouteConfig(children); } @@ -277,9 +278,7 @@ describe('RouteTraverser', function() { if ( indexRoute ) { delete route.indexRoute; - route.loadIndexRoute = function(done) { - setTimeout(() => done(indexRoute)); - }; + route.loadIndexRoute = () => Promise.resolve(indexRoute); } }); } diff --git a/spec/util.spec.ts b/spec/util.spec.ts index 415f090..ce2bd0c 100644 --- a/spec/util.spec.ts +++ b/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 = 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');