From 885fb0dd0743859a8985c23e4d0c1855a2be711e Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 22 May 2012 21:12:19 -0700 Subject: [PATCH] feat($route): resolve local route promises Resolve all promises on route before we fire $afterRouteChange which then renders the ngView. --- src/ng/directive/ngView.js | 64 ++++---- src/ng/route.js | 106 +++++++++++-- test/ng/directive/ngViewSpec.js | 22 +-- test/ng/routeSpec.js | 254 ++++++++++++++++++++++++++++++-- 4 files changed, 367 insertions(+), 79 deletions(-) diff --git a/src/ng/directive/ngView.js b/src/ng/directive/ngView.js index 7c7377653b4..4924ed1aa3e 100644 --- a/src/ng/directive/ngView.js +++ b/src/ng/directive/ngView.js @@ -112,8 +112,7 @@ var ngViewDirective = ['$http', '$templateCache', '$route', '$anchorScroll', '$c restrict: 'ECA', terminal: true, link: function(scope, element, attr) { - var changeCounter = 0, - lastScope, + var lastScope, onloadExp = attr.onload || ''; scope.$on('$afterRouteChange', update); @@ -127,43 +126,36 @@ var ngViewDirective = ['$http', '$templateCache', '$route', '$anchorScroll', '$c } } + function clearContent() { + element.html(''); + destroyLastScope(); + } + function update() { - var template = $route.current && $route.current.template, - thisChangeId = ++changeCounter; - - function clearContent() { - // ignore callback if another route change occured since - if (thisChangeId === changeCounter) { - element.html(''); - destroyLastScope(); - } - } + var locals = $route.current && $route.current.locals, + template = locals && locals.$template; if (template) { - $http.get(template, {cache: $templateCache}).success(function(response) { - // ignore callback if another route change occured since - if (thisChangeId === changeCounter) { - element.html(response); - destroyLastScope(); - - var link = $compile(element.contents()), - current = $route.current, - controller; - - lastScope = current.scope = scope.$new(); - if (current.controller) { - controller = $controller(current.controller, {$scope: lastScope}); - element.contents().data('$ngControllerController', controller); - } - - link(lastScope); - lastScope.$emit('$viewContentLoaded'); - lastScope.$eval(onloadExp); - - // $anchorScroll might listen on event... - $anchorScroll(); - } - }).error(clearContent); + element.html(template); + destroyLastScope(); + + var link = $compile(element.contents()), + current = $route.current, + controller; + + lastScope = current.scope = scope.$new(); + if (current.controller) { + locals.$scope = lastScope; + controller = $controller(current.controller, locals); + element.contents().data('$ngControllerController', controller); + } + + link(lastScope); + lastScope.$emit('$viewContentLoaded'); + lastScope.$eval(onloadExp); + + // $anchorScroll might listen on event... + $anchorScroll(); } else { clearContent(); } diff --git a/src/ng/route.js b/src/ng/route.js index fd54b1c51ad..5386e12a1fe 100644 --- a/src/ng/route.js +++ b/src/ng/route.js @@ -19,7 +19,7 @@ function $RouteProvider(){ * @methodOf angular.module.ng.$routeProvider * * @param {string} path Route path (matched against `$location.path`). If `$location.path` - * contains redudant trailing slash or is missing one, the route will still match and the + * contains redundant trailing slash or is missing one, the route will still match and the * `$location.path` will be updated to add or drop the trailing slash to exacly match the * route definition. * @param {Object} route Mapping information to be assigned to `$route.current` on route @@ -32,6 +32,17 @@ function $RouteProvider(){ * - `template` – `{string=}` – path to an html template that should be used by * {@link angular.module.ng.$compileProvider.directive.ngView ngView} or * {@link angular.module.ng.$compileProvider.directive.ngInclude ngInclude} directives. + * - `resolve` - `{Object.=}` - An optional map of dependencies which should + * be injected into the controller. If any of these dependencies are promises, they will be + * resolved and converted to a value before the controller is instantiated and the + * `$aftreRouteChange` event is fired. The map object is: + * + * - `key` – `{string}`: a name of a dependency to be injected into the controller. + * - `factory` - `{string|function}`: If `string` then it is an alias for a service. + * Otherwise if function, then it is {@link api/angular.module.AUTO.$injector#invoke injected} + * and the return value is treated as the dependency. If the result is a promise, it is resolved + * before its value is injected into the controller. + * * - `redirectTo` – {(string|function())=} – value to update * {@link angular.module.ng.$location $location} path with and trigger route redirection. * @@ -89,8 +100,8 @@ function $RouteProvider(){ }; - this.$get = ['$rootScope', '$location', '$routeParams', - function( $rootScope, $location, $routeParams) { + this.$get = ['$rootScope', '$location', '$routeParams', '$q', '$injector', '$http', '$templateCache', + function( $rootScope, $location, $routeParams, $q, $injector, $http, $templateCache) { /** * @ngdoc object @@ -99,6 +110,16 @@ function $RouteProvider(){ * @requires $routeParams * * @property {Object} current Reference to the current route definition. + * The route definition contains: + * + * - `controller`: The controller constructor as define in route definition. + * - `locals`: A map of locals which is used by {@link angular.module.ng.$controller $controller} service for + * controller instantiation. The `locals` contain + * the resolved values of the `resolve` map. Additionally the `locals` also contain: + * + * - `$scope` - The current route scope. + * - `$template` - The current route template HTML. + * * @property {Array.} routes Array of all configured routes. * * @description @@ -153,7 +174,15 @@ function $RouteProvider(){ angular.module('ngView', [], function($routeProvider, $locationProvider) { $routeProvider.when('/Book/:bookId', { template: 'book.html', - controller: BookCntl + controller: BookCntl, + resolve: { + // I will cause a 1 second delay + delay: function($q, $timeout) { + var delay = $q.defer(); + $timeout(delay.resolve, 1000); + return delay.promise; + } + } }); $routeProvider.when('/Book/:bookId/ch/:chapterId', { template: 'chapter.html', @@ -190,6 +219,7 @@ function $RouteProvider(){ expect(content).toMatch(/Chapter Id\: 1/); element('a:contains("Scarlet")').click(); + sleep(2); // promises are not part of scenario waiting content = element('.doc-example-live [ng-view]').text(); expect(content).toMatch(/controller\: BookCntl/); expect(content).toMatch(/Book Id\: Scarlet/); @@ -204,7 +234,11 @@ function $RouteProvider(){ * @eventOf angular.module.ng.$route * @eventType broadcast on root scope * @description - * Broadcasted before a route change. + * Broadcasted before a route change. At this point the route services starts + * resolving all of the dependencies needed for the route change to occurs. + * Typically this involves fetching the view template as well as any dependencies + * defined in `resolve` route property. Once all of the dependencies are resolved + * `$afterRouteChange` is fired. * * @param {Route} next Future route information. * @param {Route} current Current route information. @@ -216,12 +250,27 @@ function $RouteProvider(){ * @eventOf angular.module.ng.$route * @eventType broadcast on root scope * @description - * Broadcasted after a route change. + * Broadcasted after a route dependencies are resolved. + * {@link angular.module.ng.$compileProvider.directive.ngView ngView} listens for the directive + * to instantiate the controller and render the view. * * @param {Route} current Current route information. * @param {Route} previous Previous route information. */ + /** + * @ngdoc event + * @name angular.module.ng.$route#$routeChangeError + * @eventOf angular.module.ng.$route + * @eventType broadcast on root scope + * @description + * Broadcasted if any of the resolve promises are rejected. + * + * @param {Route} current Current route information. + * @param {Route} previous Previous route information. + * @param {Route} rejection Rejection of the promise. Usually the error of the failed promise. + */ + /** * @ngdoc event * @name angular.module.ng.$route#$routeUpdate @@ -245,7 +294,7 @@ function $RouteProvider(){ * @methodOf angular.module.ng.$route * * @description - * Causes `$route` service to reload theR current route even if + * Causes `$route` service to reload the current route even if * {@link angular.module.ng.$location $location} hasn't changed. * * As a result of that, {@link angular.module.ng.$compileProvider.directive.ngView ngView} @@ -309,11 +358,48 @@ function $RouteProvider(){ $location.url(next.redirectTo(next.pathParams, $location.path(), $location.search())) .replace(); } - } else { - copy(next.params, $routeParams); } } - $rootScope.$broadcast('$afterRouteChange', next, last); + + $q.when(next). + then(function() { + if (next) { + var keys = [], + values = []; + + forEach(next.resolve || {}, function(value, key) { + keys.push(key); + values.push(isFunction(value) ? $injector.invoke(value) : $injector.get(value)); + }); + if (next.template) { + keys.push('$template'); + values.push($http. + get(next.template, {cache: $templateCache}). + then(function(response) { return response.data; })); + } + return $q.all(values).then(function(values) { + var locals = {}; + forEach(values, function(value, index) { + locals[keys[index]] = value; + }); + return locals; + }); + } + }). + // after route change + then(function(locals) { + if (next == $route.current) { + if (next) { + next.locals = locals; + copy(next.params, $routeParams); + } + $rootScope.$broadcast('$afterRouteChange', next, last); + } + }, function(error) { + if (next == $route.current) { + $rootScope.$broadcast('$routeChangeError', next, last, error); + } + }); } } diff --git a/test/ng/directive/ngViewSpec.js b/test/ng/directive/ngViewSpec.js index 00fc6827900..7524884fcd0 100644 --- a/test/ng/directive/ngViewSpec.js +++ b/test/ng/directive/ngViewSpec.js @@ -229,24 +229,6 @@ describe('ngView', function() { }); - it('should clear the content when error during xhr request', function() { - module(function($routeProvider) { - $routeProvider.when('/foo', {controller: noop, template: 'myUrl1'}); - }); - - inject(function($route, $location, $rootScope, $httpBackend) { - $location.path('/foo'); - $httpBackend.expect('GET', 'myUrl1').respond(404, ''); - element.text('content'); - - $rootScope.$digest(); - $httpBackend.flush(); - - expect(element.text()).toBe(''); - }); - }); - - it('should be async even if served from cache', function() { module(function($routeProvider) { $routeProvider.when('/foo', {controller: noop, template: 'myUrl1'}); @@ -293,8 +275,8 @@ describe('ngView', function() { $rootScope.$digest(); expect(element.text()).toBe('bound-value'); - expect(log).toEqual(['$beforeRouteChange', '$afterRouteChange', 'init-ctrl', - '$viewContentLoaded']); + expect(log).toEqual([ + '$beforeRouteChange', 'init-ctrl', '$viewContentLoaded', '$afterRouteChange' ]); }); }); diff --git a/test/ng/routeSpec.js b/test/ng/routeSpec.js index b66cbb8edee..3097d72dcfb 100644 --- a/test/ng/routeSpec.js +++ b/test/ng/routeSpec.js @@ -1,6 +1,19 @@ 'use strict'; describe('$route', function() { + var $httpBackend; + + beforeEach(module(function() { + return function(_$httpBackend_) { + $httpBackend = _$httpBackend_; + $httpBackend.when('GET', 'Chapter.html').respond('chapter'); + $httpBackend.when('GET', 'test.html').respond('test'); + $httpBackend.when('GET', 'foo.html').respond('foo'); + $httpBackend.when('GET', 'baz.html').respond('baz'); + $httpBackend.when('GET', 'bar.html').respond('bar'); + $httpBackend.when('GET', '404.html').respond('not found'); + }; + })); it('should route and fire change event', function() { var log = '', @@ -28,6 +41,7 @@ describe('$route', function() { $location.path('/Book/Moby/Chapter/Intro').search('p=123'); $rootScope.$digest(); + $httpBackend.flush(); expect(log).toEqual('before();after();'); expect($route.current.params).toEqual({book:'Moby', chapter:'Intro', p:'123'}); @@ -165,27 +179,241 @@ describe('$route', function() { }); - it('should not fire $after/beforeRouteChange during bootstrap (if no route)', function() { - var routeChangeSpy = jasmine.createSpy('route change'); + describe('events', function() { + it('should not fire $after/beforeRouteChange during bootstrap (if no route)', function() { + var routeChangeSpy = jasmine.createSpy('route change'); - module(function($routeProvider) { - $routeProvider.when('/one', {}); // no otherwise defined + module(function($routeProvider) { + $routeProvider.when('/one', {}); // no otherwise defined + }); + + inject(function($rootScope, $route, $location) { + $rootScope.$on('$beforeRouteChange', routeChangeSpy); + $rootScope.$on('$afterRouteChange', routeChangeSpy); + + $rootScope.$digest(); + expect(routeChangeSpy).not.toHaveBeenCalled(); + + $location.path('/no-route-here'); + $rootScope.$digest(); + expect(routeChangeSpy).not.toHaveBeenCalled(); + }); }); - inject(function($rootScope, $route, $location) { - $rootScope.$on('$beforeRouteChange', routeChangeSpy); - $rootScope.$on('$afterRouteChange', routeChangeSpy); + it('should fire $beforeRouteChange and resolve promises', function() { + var deferA, + deferB; - $rootScope.$digest(); - expect(routeChangeSpy).not.toHaveBeenCalled(); + module(function($provide, $routeProvider) { + $provide.factory('b', function($q) { + deferB = $q.defer(); + return deferB.promise; + }); + $routeProvider.when('/path', { template: 'foo.html', resolve: { + a: function($q) { + deferA = $q.defer(); + return deferA.promise; + }, + b: 'b' + } }); + }); - $location.path('/no-route-here'); - $rootScope.$digest(); - expect(routeChangeSpy).not.toHaveBeenCalled(); + inject(function($location, $route, $rootScope, $httpBackend) { + var log = ''; + + $httpBackend.expectGET('foo.html').respond('FOO'); + + $location.path('/path'); + $rootScope.$digest(); + expect(log).toEqual(''); + $httpBackend.flush(); + expect(log).toEqual(''); + deferA.resolve(); + $rootScope.$digest(); + expect(log).toEqual(''); + deferB.resolve(); + $rootScope.$digest(); + expect($route.current.locals.$template).toEqual('FOO'); + }); }); - }); + it('should fire $routeChangeError event on resolution error', function() { + var deferA; + + module(function($provide, $routeProvider) { + $routeProvider.when('/path', { template: 'foo', resolve: { + a: function($q) { + deferA = $q.defer(); + return deferA.promise; + } + } }); + }); + + inject(function($location, $route, $rootScope) { + var log = ''; + + $rootScope.$on('$beforeRouteChange', function() { log += 'before();'; }); + $rootScope.$on('$routeChangeError', function(e, n, l, reason) { log += 'failed(' + reason + ');'; }); + + $location.path('/path'); + $rootScope.$digest(); + expect(log).toEqual('before();'); + + deferA.reject('MyError'); + $rootScope.$digest(); + expect(log).toEqual('before();failed(MyError);'); + }); + }); + + + it('should fetch templates', function() { + module(function($routeProvider) { + $routeProvider. + when('/r1', { template: 'r1.html' }). + when('/r2', { template: 'r2.html' }); + }); + + inject(function($route, $httpBackend, $location, $rootScope) { + var log = ''; + $rootScope.$on('$beforeRouteChange', function(e, next) { log += '$before(' + next.template + ');'}); + $rootScope.$on('$afterRouteChange', function(e, next) { log += '$after(' + next.template + ');'}); + + $httpBackend.expectGET('r1.html').respond('R1'); + $httpBackend.expectGET('r2.html').respond('R2'); + + $location.path('/r1'); + $rootScope.$digest(); + expect(log).toBe('$before(r1.html);'); + + $location.path('/r2'); + $rootScope.$digest(); + expect(log).toBe('$before(r1.html);$before(r2.html);'); + + $httpBackend.flush(); + expect(log).toBe('$before(r1.html);$before(r2.html);$after(r2.html);'); + expect(log).not.toContain('$after(r1.html);'); + }); + }); + + + it('should not update $routeParams until $afterRouteChange', function() { + module(function($routeProvider) { + $routeProvider. + when('/r1/:id', { template: 'r1.html' }). + when('/r2/:id', { template: 'r2.html' }); + }); + + inject(function($route, $httpBackend, $location, $rootScope, $routeParams) { + var log = ''; + $rootScope.$on('$beforeRouteChange', function(e, next) { log += '$before' + toJson($routeParams) + ';'}); + $rootScope.$on('$afterRouteChange', function(e, next) { log += '$after' + toJson($routeParams) + ';'}); + + $httpBackend.whenGET('r1.html').respond('R1'); + $httpBackend.whenGET('r2.html').respond('R2'); + + $location.path('/r1/1'); + $rootScope.$digest(); + expect(log).toBe('$before{};'); + $httpBackend.flush(); + expect(log).toBe('$before{};$after{"id":"1"};'); + + log = ''; + + $location.path('/r2/2'); + $rootScope.$digest(); + expect(log).toBe('$before{"id":"1"};'); + $httpBackend.flush(); + expect(log).toBe('$before{"id":"1"};$after{"id":"2"};'); + }); + }); + + + it('should drop in progress route change when new route change occurs', function() { + module(function($routeProvider) { + $routeProvider. + when('/r1', { template: 'r1.html' }). + when('/r2', { template: 'r2.html' }); + }); + + inject(function($route, $httpBackend, $location, $rootScope) { + var log = ''; + $rootScope.$on('$beforeRouteChange', function(e, next) { log += '$before(' + next.template + ');'}); + $rootScope.$on('$afterRouteChange', function(e, next) { log += '$after(' + next.template + ');'}); + + $httpBackend.expectGET('r1.html').respond('R1'); + $httpBackend.expectGET('r2.html').respond('R2'); + + $location.path('/r1'); + $rootScope.$digest(); + expect(log).toBe('$before(r1.html);'); + + $location.path('/r2'); + $rootScope.$digest(); + expect(log).toBe('$before(r1.html);$before(r2.html);'); + + $httpBackend.flush(); + expect(log).toBe('$before(r1.html);$before(r2.html);$after(r2.html);'); + expect(log).not.toContain('$after(r1.html);'); + }); + }); + + + it('should drop in progress route change when new route change occurs and old fails', function() { + module(function($routeProvider) { + $routeProvider. + when('/r1', { templateUrl: 'r1.html' }). + when('/r2', { templateUrl: 'r2.html' }); + }); + + inject(function($route, $httpBackend, $location, $rootScope) { + var log = ''; + $rootScope.$on('$routeChangeError', function(e, next, last, error) { + log += '$failed(' + next.templateUrl + ', ' + error.status + ');'; + }); + $rootScope.$on('$beforeRouteChange', function(e, next) { log += '$before(' + next.templateUrl + ');'}); + $rootScope.$on('$afterRouteChange', function(e, next) { log += '$after(' + next.templateUrl + ');'}); + + $httpBackend.expectGET('r1.html').respond(404, 'R1'); + $httpBackend.expectGET('r2.html').respond('R2'); + + $location.path('/r1'); + $rootScope.$digest(); + expect(log).toBe('$before(r1.html);'); + + $location.path('/r2'); + $rootScope.$digest(); + expect(log).toBe('$before(r1.html);$before(r2.html);'); + + $httpBackend.flush(); + expect(log).toBe('$before(r1.html);$before(r2.html);$after(r2.html);'); + expect(log).not.toContain('$after(r1.html);'); + }); + }); + + + it('should catch local factory errors', function() { + var myError = new Error('MyError'); + module(function($routeProvider, $exceptionHandlerProvider) { + $exceptionHandlerProvider.mode('log'); + $routeProvider.when('/locals', { + resolve: { + a: function($q) { + throw myError; + } + } + }); + }); + + inject(function($location, $route, $rootScope, $exceptionHandler) { + $location.path('/locals'); + $rootScope.$digest(); + expect($exceptionHandler.errors).toEqual([myError]); + }); + }); + }); + + it('should match route with and without trailing slash', function() { module(function($routeProvider){ $routeProvider.when('/foo', {template: 'foo.html'});