From 4361efb03b79e71bf0cea92b94ff377ed718bad4 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Wed, 16 May 2012 22:21:08 -0700 Subject: [PATCH 1/8] feat($injector): provide API for retrieving function annotations --- src/auto/injector.js | 142 ++++++++++++++++++++++++++++++-------- test/auto/injectorSpec.js | 27 +++++--- 2 files changed, 131 insertions(+), 38 deletions(-) diff --git a/src/auto/injector.js b/src/auto/injector.js index 250f05c5b826..49247ff4ee2a 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -42,19 +42,32 @@ var FN_ARGS = /^function\s*[^\(]*\(\s*([^\)]*)\)/m; var FN_ARG_SPLIT = /,/; var FN_ARG = /^\s*(_?)(.+?)\1\s*$/; var STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg; -function inferInjectionArgs(fn) { - assertArgFn(fn); - if (!fn.$inject) { - var args = fn.$inject = []; - var fnText = fn.toString().replace(STRIP_COMMENTS, ''); - var argDecl = fnText.match(FN_ARGS); - forEach(argDecl[1].split(FN_ARG_SPLIT), function(arg){ - arg.replace(FN_ARG, function(all, underscore, name){ - args.push(name); +function annotate(fn) { + var $inject, + fnText, + argDecl, + last; + + if (typeof fn == 'function') { + if (!($inject = fn.$inject)) { + $inject = []; + fnText = fn.toString().replace(STRIP_COMMENTS, ''); + argDecl = fnText.match(FN_ARGS); + forEach(argDecl[1].split(FN_ARG_SPLIT), function(arg){ + arg.replace(FN_ARG, function(all, underscore, name){ + $inject.push(name); + }); }); - }); + fn.$inject = $inject; + } + } else if (isArray(fn)) { + last = fn.length - 1; + assertArgFn(fn[last], 'fn') + $inject = fn.slice(0, last); + } else { + assertArgFn(fn, 'fn', true); } - return fn.$inject; + return $inject; } /////////////////////////////////////// @@ -152,6 +165,87 @@ function inferInjectionArgs(fn) { * @returns {Object} new instance of `Type`. */ +/** + * @ngdoc method + * @name angular.module.AUTO.$injector#annotate + * @methodOf angular.module.AUTO.$injector + * + * @description + * Returns an array of service names which the function is requesting for injection. This API is used by the injector + * to determine which services need to be injected into the function when the function is invoked. There are three + * ways in which the function can be annotated with the needed dependencies. + * + * # Argument names + * + * The simplest form is to extract the dependencies from the arguments of the function. This is done by converting + * the function into a string using `toString()` method and extracting the argument names. + * 
+ *   // Given
+ *   function MyController($scope, $route) {
+ *     // ...
+ *   }
+ *
+ *   // Then
+ *   expect(injector.annotate(MyController)).toEqual(['$scope', '$route']);
+ *
+ * + * This method does not work with code minfication / obfuscation. For this reason the following annotation strategies + * are supported. + * + * # The `$injector` property + * + * If a function has an `$inject` property and its value is an array of strings, then the strings represent names of + * services to be injected into the function. + * 
+ *   // Given
+ *   var MyController = function(obfuscatedScope, obfuscatedRoute) {
+ *     // ...
+ *   }
+ *   // Define function dependencies
+ *   MyController.$inject = ['$scope', '$route'];
+ *
+ *   // Then
+ *   expect(injector.annotate(MyController)).toEqual(['$scope', '$route']);
+ *
+ * + * # The array notation + * + * It is often desirable to inline Injected functions and that's when setting the `$inject` property is very + * inconvenient. In these situations using the array notation to specify the dependencies in a way that survives + * minification is a better choice: + * + * 
+ *   // We wish to write this (not minification / obfuscation safe)
+ *   injector.invoke(function($compile, $rootScope) {
+ *     // ...
+ *   });
+ *
+ *   // We are forced to write break inlining
+ *   var tmpFn = function(obfuscatedCompile, obfuscatedRootScope) {
+ *     // ...
+ *   };
+ *   tmpFn.$inject = ['$compile', '$rootScope'];
+ *   injector.invoke(tempFn);
+ *
+ *   // To better support inline function the inline annotation is supported
+ *   injector.invoke(['$compile', '$rootScope', function(obfCompile, obfRootScope) {
+ *     // ...
+ *   }]);
+ *
+ *   // Therefore
+ *   expect(injector.annotate(
+ *      ['$compile', '$rootScope', function(obfus_$compile, obfus_$rootScope) {}])
+ *    ).toEqual(['$compile', '$rootScope']);
+ *
+ * + * @param {function|Array.} fn Function for which dependent service names need to be retrieved as described + * above. + * + * @returns {Array.} The names of the services which the function requires. + */ + + + /** * @ngdoc object @@ -454,23 +548,11 @@ function createInjector(modulesToLoad) { function invoke(fn, self, locals){ var args = [], - $inject, - length, + $inject = annotate(fn), + length, i, key; - if (typeof fn == 'function') { - $inject = inferInjectionArgs(fn); - length = $inject.length; - } else { - if (isArray(fn)) { - $inject = fn; - length = $inject.length - 1; - fn = $inject[length]; - } - assertArgFn(fn, 'fn'); - } - - for(var i = 0; i < length; i++) { + for(i = 0, length = $inject.length; i < length; i++) { key = $inject[i]; args.push( locals && locals.hasOwnProperty(key) @@ -478,6 +560,11 @@ function createInjector(modulesToLoad) { : getService(key, path) ); } + if (!fn.$inject) { + // this means that we must be an array. + fn = fn[length]; + } + // Performance optimization: http://jsperf.com/apply-vs-call-vs-invoke switch (self ? -1 : args.length) { @@ -510,7 +597,8 @@ function createInjector(modulesToLoad) { return { invoke: invoke, instantiate: instantiate, - get: getService + get: getService, + annotate: annotate }; } } diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index 83bd3d1f81e2..33fecac616de 100644 --- a/test/auto/injectorSpec.js +++ b/test/auto/injectorSpec.js @@ -123,11 +123,11 @@ describe('injector', function() { it('should return $inject', function() { function fn() {} fn.$inject = ['a']; - expect(inferInjectionArgs(fn)).toBe(fn.$inject); - expect(inferInjectionArgs(function() {})).toEqual([]); - expect(inferInjectionArgs(function () {})).toEqual([]); - expect(inferInjectionArgs(function () {})).toEqual([]); - expect(inferInjectionArgs(function /* */ () {})).toEqual([]); + expect(annotate(fn)).toBe(fn.$inject); + expect(annotate(function() {})).toEqual([]); + expect(annotate(function () {})).toEqual([]); + expect(annotate(function () {})).toEqual([]); + expect(annotate(function /* */ () {})).toEqual([]); }); @@ -142,43 +142,48 @@ describe('injector', function() { */ _c, /* {some type} */ d) { extraParans();} - expect(inferInjectionArgs($f_n0)).toEqual(['$a', 'b_', '_c', 'd']); + expect(annotate($f_n0)).toEqual(['$a', 'b_', '_c', 'd']); expect($f_n0.$inject).toEqual(['$a', 'b_', '_c', 'd']); }); it('should strip leading and trailing underscores from arg name during inference', function() { function beforeEachFn(_foo_) { /* foo = _foo_ */ }; - expect(inferInjectionArgs(beforeEachFn)).toEqual(['foo']); + expect(annotate(beforeEachFn)).toEqual(['foo']); }); it('should handle no arg functions', function() { function $f_n0() {} - expect(inferInjectionArgs($f_n0)).toEqual([]); + expect(annotate($f_n0)).toEqual([]); expect($f_n0.$inject).toEqual([]); }); it('should handle no arg functions with spaces in the arguments list', function() { function fn( ) {} - expect(inferInjectionArgs(fn)).toEqual([]); + expect(annotate(fn)).toEqual([]); expect(fn.$inject).toEqual([]); }); it('should handle args with both $ and _', function() { function $f_n0($a_) {} - expect(inferInjectionArgs($f_n0)).toEqual(['$a_']); + expect(annotate($f_n0)).toEqual(['$a_']); expect($f_n0.$inject).toEqual(['$a_']); }); it('should throw on non function arg', function() { expect(function() { - inferInjectionArgs({}); + annotate({}); }).toThrow(); }); + + + it('should publish annotate API', function() { + expect(injector.annotate).toBe(annotate); + }); }); From 885fb0dd0743859a8985c23e4d0c1855a2be711e Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Tue, 22 May 2012 21:12:19 -0700 Subject: [PATCH 2/8] 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 7c7377653b40..4924ed1aa3e7 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 fd54b1c51ad1..5386e12a1feb 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 00fc68279009..7524884fcd03 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 b66cbb8edeec..3097d72dcfb4 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'}); From 7c2428218893f59c6a4499667488009ca67f3385 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 1 Jun 2012 11:31:10 -0700 Subject: [PATCH 3/8] chore($route): rename events BREAKING CHANGE rename $beforeRouteChange to $routeChangeStart rename $afterRouteChange to $routeChangeSuccess --- ...guide.services.managing_dependencies.ngdoc | 2 +- src/ng/directive/ngView.js | 2 +- src/ng/route.js | 10 ++-- test/ng/directive/ngViewSpec.js | 6 +-- test/ng/routeSpec.js | 53 ++++++++++--------- 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/docs/content/guide/dev_guide.services.managing_dependencies.ngdoc b/docs/content/guide/dev_guide.services.managing_dependencies.ngdoc index bccc653930a5..01c261469a57 100644 --- a/docs/content/guide/dev_guide.services.managing_dependencies.ngdoc +++ b/docs/content/guide/dev_guide.services.managing_dependencies.ngdoc @@ -76,7 +76,7 @@ provided by Angular's web framework: $provide.factory('routeTemplateMonitor', ['$route', 'batchLog', '$rootScope', function($route, batchLog, $rootScope) { - $rootScope.$on('$afterRouteChange', function() { + $rootScope.$on('$routeChangeSuccess', function() { batchLog($route.current ? $route.current.template : null); }); }]); diff --git a/src/ng/directive/ngView.js b/src/ng/directive/ngView.js index 4924ed1aa3e7..1d2a5d29c6ce 100644 --- a/src/ng/directive/ngView.js +++ b/src/ng/directive/ngView.js @@ -115,7 +115,7 @@ var ngViewDirective = ['$http', '$templateCache', '$route', '$anchorScroll', '$c var lastScope, onloadExp = attr.onload || ''; - scope.$on('$afterRouteChange', update); + scope.$on('$routeChangeSuccess', update); update(); diff --git a/src/ng/route.js b/src/ng/route.js index 5386e12a1feb..ab19818877f4 100644 --- a/src/ng/route.js +++ b/src/ng/route.js @@ -230,7 +230,7 @@ function $RouteProvider(){ /** * @ngdoc event - * @name angular.module.ng.$route#$beforeRouteChange + * @name angular.module.ng.$route#$routeChangeStart * @eventOf angular.module.ng.$route * @eventType broadcast on root scope * @description @@ -238,7 +238,7 @@ function $RouteProvider(){ * 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. + * `$routeChangeSuccess` is fired. * * @param {Route} next Future route information. * @param {Route} current Current route information. @@ -246,7 +246,7 @@ function $RouteProvider(){ /** * @ngdoc event - * @name angular.module.ng.$route#$afterRouteChange + * @name angular.module.ng.$route#$routeChangeSuccess * @eventOf angular.module.ng.$route * @eventType broadcast on root scope * @description @@ -347,7 +347,7 @@ function $RouteProvider(){ $rootScope.$broadcast('$routeUpdate', last); } else if (next || last) { forceReload = false; - $rootScope.$broadcast('$beforeRouteChange', next, last); + $rootScope.$broadcast('$routeChangeStart', next, last); $route.current = next; if (next) { if (next.redirectTo) { @@ -393,7 +393,7 @@ function $RouteProvider(){ next.locals = locals; copy(next.params, $routeParams); } - $rootScope.$broadcast('$afterRouteChange', next, last); + $rootScope.$broadcast('$routeChangeSuccess', next, last); } }, function(error) { if (next == $route.current) { diff --git a/test/ng/directive/ngViewSpec.js b/test/ng/directive/ngViewSpec.js index 7524884fcd03..efdefae882ef 100644 --- a/test/ng/directive/ngViewSpec.js +++ b/test/ng/directive/ngViewSpec.js @@ -266,8 +266,8 @@ describe('ngView', function() { }); inject(function($templateCache, $rootScope, $location) { - $rootScope.$on('$beforeRouteChange', logger('$beforeRouteChange')); - $rootScope.$on('$afterRouteChange', logger('$afterRouteChange')); + $rootScope.$on('$routeChangeStart', logger('$routeChangeStart')); + $rootScope.$on('$routeChangeSuccess', logger('$routeChangeSuccess')); $rootScope.$on('$viewContentLoaded', logger('$viewContentLoaded')); $templateCache.put('tpl.html', [200, '{{value}}', {}]); @@ -276,7 +276,7 @@ describe('ngView', function() { expect(element.text()).toBe('bound-value'); expect(log).toEqual([ - '$beforeRouteChange', 'init-ctrl', '$viewContentLoaded', '$afterRouteChange' ]); + '$routeChangeStart', 'init-ctrl', '$viewContentLoaded', '$routeChangeSuccess' ]); }); }); diff --git a/test/ng/routeSpec.js b/test/ng/routeSpec.js index 3097d72dcfb4..c3f7b357eea5 100644 --- a/test/ng/routeSpec.js +++ b/test/ng/routeSpec.js @@ -26,13 +26,13 @@ describe('$route', function() { $routeProvider.when('/Blank', {}); }); inject(function($route, $location, $rootScope) { - $rootScope.$on('$beforeRouteChange', function(event, next, current) { + $rootScope.$on('$routeChangeStart', function(event, next, current) { log += 'before();'; expect(current).toBe($route.current); lastRoute = current; nextRoute = next; }); - $rootScope.$on('$afterRouteChange', function(event, current, last) { + $rootScope.$on('$routeChangeSuccess', function(event, current, last) { log += 'after();'; expect(current).toBe($route.current); expect(lastRoute).toBe(last); @@ -93,7 +93,7 @@ describe('$route', function() { inject(function($route, $location, $rootScope) { var callback = jasmine.createSpy('onRouteChange'); - $rootScope.$on('$beforeRouteChange', callback); + $rootScope.$on('$routeChangeStart', callback); $location.path('/test'); $rootScope.$digest(); callback.reset(); @@ -114,7 +114,7 @@ describe('$route', function() { inject(function($route, $location, $rootScope) { var onChangeSpy = jasmine.createSpy('onChange'); - $rootScope.$on('$beforeRouteChange', onChangeSpy); + $rootScope.$on('$routeChangeStart', onChangeSpy); expect($route.current).toBeUndefined(); expect(onChangeSpy).not.toHaveBeenCalled(); @@ -139,7 +139,7 @@ describe('$route', function() { inject(function($route, $location, $rootScope) { var onChangeSpy = jasmine.createSpy('onChange'); - $rootScope.$on('$beforeRouteChange', onChangeSpy); + $rootScope.$on('$routeChangeStart', onChangeSpy); expect($route.current).toBeUndefined(); expect(onChangeSpy).not.toHaveBeenCalled(); @@ -188,8 +188,8 @@ describe('$route', function() { }); inject(function($rootScope, $route, $location) { - $rootScope.$on('$beforeRouteChange', routeChangeSpy); - $rootScope.$on('$afterRouteChange', routeChangeSpy); + $rootScope.$on('$routeChangeStart', routeChangeSpy); + $rootScope.$on('$routeChangeSuccess', routeChangeSpy); $rootScope.$digest(); expect(routeChangeSpy).not.toHaveBeenCalled(); @@ -200,7 +200,7 @@ describe('$route', function() { }); }); - it('should fire $beforeRouteChange and resolve promises', function() { + it('should fire $routeChangeStart and resolve promises', function() { var deferA, deferB; @@ -253,8 +253,13 @@ describe('$route', function() { inject(function($location, $route, $rootScope) { var log = ''; +<<<<<<< HEAD $rootScope.$on('$beforeRouteChange', function() { log += 'before();'; }); $rootScope.$on('$routeChangeError', function(e, n, l, reason) { log += 'failed(' + reason + ');'; }); +======= + $rootScope.$on('$routeChangeStart', function() { log += 'before();'; }); + $rootScope.$on('$routeChangeFailed', function(e, n, l, reason) { log += 'failed(' + reason + ');'; }); +>>>>>>> ebebe46... chore($route): rename events $location.path('/path'); $rootScope.$digest(); @@ -276,8 +281,8 @@ describe('$route', function() { 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 + ');'}); + $rootScope.$on('$routeChangeStart', function(e, next) { log += '$before(' + next.template + ');'}); + $rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after(' + next.template + ');'}); $httpBackend.expectGET('r1.html').respond('R1'); $httpBackend.expectGET('r2.html').respond('R2'); @@ -297,7 +302,7 @@ describe('$route', function() { }); - it('should not update $routeParams until $afterRouteChange', function() { + it('should not update $routeParams until $routeChangeSuccess', function() { module(function($routeProvider) { $routeProvider. when('/r1/:id', { template: 'r1.html' }). @@ -306,8 +311,8 @@ describe('$route', function() { 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) + ';'}); + $rootScope.$on('$routeChangeStart', function(e, next) { log += '$before' + toJson($routeParams) + ';'}); + $rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after' + toJson($routeParams) + ';'}); $httpBackend.whenGET('r1.html').respond('R1'); $httpBackend.whenGET('r2.html').respond('R2'); @@ -338,8 +343,8 @@ describe('$route', function() { 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 + ');'}); + $rootScope.$on('$routeChangeStart', function(e, next) { log += '$before(' + next.template + ');'}); + $rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after(' + next.template + ');'}); $httpBackend.expectGET('r1.html').respond('R1'); $httpBackend.expectGET('r2.html').respond('R2'); @@ -371,8 +376,8 @@ describe('$route', function() { $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 + ');'}); + $rootScope.$on('$routeChangeStart', function(e, next) { log += '$before(' + next.templateUrl + ');'}); + $rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after(' + next.templateUrl + ');'}); $httpBackend.expectGET('r1.html').respond(404, 'R1'); $httpBackend.expectGET('r2.html').respond('R2'); @@ -457,7 +462,7 @@ describe('$route', function() { inject(function($route, $location, $rootScope) { var onChangeSpy = jasmine.createSpy('onChange'); - $rootScope.$on('$beforeRouteChange', onChangeSpy); + $rootScope.$on('$routeChangeStart', onChangeSpy); expect($route.current).toBeUndefined(); expect(onChangeSpy).not.toHaveBeenCalled(); @@ -563,7 +568,7 @@ describe('$route', function() { }); inject(function($route, $location, $rootScope, $routeParams) { - $rootScope.$on('$beforeRouteChange', reloaded); + $rootScope.$on('$routeChangeStart', reloaded); $location.path('/foo'); $rootScope.$digest(); expect(reloaded).toHaveBeenCalled(); @@ -588,8 +593,8 @@ describe('$route', function() { }); inject(function($route, $location, $rootScope) { - $rootScope.$on('$beforeRouteChange', routeChange); - $rootScope.$on('$afterRouteChange', routeChange); + $rootScope.$on('$routeChangeStart', routeChange); + $rootScope.$on('$routeChangeSuccess', routeChange); $rootScope.$on('$routeUpdate', routeUpdate); expect(routeChange).not.toHaveBeenCalled(); @@ -618,8 +623,8 @@ describe('$route', function() { }); inject(function($route, $location, $rootScope) { - $rootScope.$on('$beforeRouteChange', routeChange); - $rootScope.$on('$afterRouteChange', routeChange); + $rootScope.$on('$routeChangeStart', routeChange); + $rootScope.$on('$routeChangeSuccess', routeChange); expect(routeChange).not.toHaveBeenCalled(); @@ -693,7 +698,7 @@ describe('$route', function() { }); inject(function($route, $location, $rootScope, $routeParams) { - $rootScope.$on('$afterRouteChange', routeChangeSpy); + $rootScope.$on('$routeChangeSuccess', routeChangeSpy); $location.path('/bar/123'); $rootScope.$digest(); From 0a6e464a93d9a1e76a624b356054ce9ca4015f55 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Thu, 17 May 2012 16:36:18 -0700 Subject: [PATCH 4/8] feat($route): rename template -> tempalteUrl and add support for inline templates BREAKING CHANGE: template in $route definition is now templateUrl To migrate just rename `template` to `templateUrl`. --- docs/content/cookbook/deeplinking.ngdoc | 4 +- docs/content/tutorial/step_07.ngdoc | 4 +- example/temp.html | 4 +- src/ng/directive/ngView.js | 4 +- src/ng/route.js | 27 +++++--- test/ng/directive/ngViewSpec.js | 65 ++++++++++++------ test/ng/routeSpec.js | 91 ++++++++++++------------- 7 files changed, 113 insertions(+), 86 deletions(-) diff --git a/docs/content/cookbook/deeplinking.ngdoc b/docs/content/cookbook/deeplinking.ngdoc index 491e3b799a7d..1e9c01616cfe 100644 --- a/docs/content/cookbook/deeplinking.ngdoc +++ b/docs/content/cookbook/deeplinking.ngdoc @@ -35,8 +35,8 @@ In this example we have a simple app which consist of two screens: angular.module('deepLinking', ['ngSanitize']) .config(function($routeProvider) { $routeProvider. - when("/welcome", {template:'welcome.html', controller:WelcomeCntl}). - when("/settings", {template:'settings.html', controller:SettingsCntl}); + when("/welcome", {templateUrl:'welcome.html', controller:WelcomeCntl}). + when("/settings", {templateUrl:'settings.html', controller:SettingsCntl}); }); AppCntl.$inject = ['$scope', '$route'] diff --git a/docs/content/tutorial/step_07.ngdoc b/docs/content/tutorial/step_07.ngdoc index f8b5cf527072..f0812278445e 100644 --- a/docs/content/tutorial/step_07.ngdoc +++ b/docs/content/tutorial/step_07.ngdoc @@ -72,8 +72,8 @@ __`app/js/app.js`:__ angular.module('phonecat', []). config(['$routeProvider', function($routeProvider) { $routeProvider. - when('/phones', {template: 'partials/phone-list.html', controller: PhoneListCtrl}). - when('/phones/:phoneId', {template: 'partials/phone-detail.html', controller: PhoneDetailCtrl}). + when('/phones', {templateUrl: 'partials/phone-list.html', controller: PhoneListCtrl}). + when('/phones/:phoneId', {templateUrl: 'partials/phone-detail.html', controller: PhoneDetailCtrl}). otherwise({redirectTo: '/phones'}); }]); diff --git a/example/temp.html b/example/temp.html index 22eb2de7da31..da92c68ccf3c 100644 --- a/example/temp.html +++ b/example/temp.html @@ -6,8 +6,8 @@