diff --git a/docs/content/error/$injector/strictdi.ngdoc b/docs/content/error/$injector/strictdi.ngdoc new file mode 100644 index 000000000000..c7e97b1a26cb --- /dev/null +++ b/docs/content/error/$injector/strictdi.ngdoc @@ -0,0 +1,54 @@ +@ngdoc error +@name $injector:strictdi +@fullName Explicit annotation required +@description + +This error occurs when attempting to invoke a function or provider which +has not been explicitly annotated, while the application is running with +strict-di mode enabled. + +For example: + +``` +angular.module("myApp", []) + // BadController cannot be invoked, because + // the dependencies to be injected are not + // explicitly listed. + .controller("BadController", function($scope, $http, $filter) { + // ... + }); +``` + +To fix the error, explicitly annotate the function using either the inline +bracket notation, or with the $inject property: + +``` +function GoodController1($scope, $http, $filter) { + // ... +} +GoodController1.$inject = ["$scope", "$http", "$filter"]; + +angular.module("myApp", []) + // GoodController1 can be invoked because it + // had an $inject property, which is an array + // containing the dependency names to be + // injected. + .controller("GoodController1", GoodController1) + + // GoodController2 can also be invoked, because + // the dependencies to inject are listed, in + // order, in the array, with the function to be + // invoked trailing on the end. + .controller("GoodController2", [ + "$scope", + "$http", + "$filter", + function($scope, $http, $filter) { + // ... + } + ]); + +``` + +For more information about strict-di mode, see {@link ng.directive:ngApp ngApp} +and {@link api/angular.bootstrap angular.bootstrap}. diff --git a/src/Angular.js b/src/Angular.js index 98fa0058b1ff..315766eafc4f 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -1138,6 +1138,19 @@ function encodeUriQuery(val, pctEncodeSpaces) { replace(/%20/g, (pctEncodeSpaces ? '%20' : '+')); } +var ngAttrPrefixes = ['ng-', 'data-ng-', 'ng:', 'x-ng-']; + +function getNgAttribute(element, ngAttr) { + var attr, i, ii = ngAttrPrefixes.length, j, jj; + element = jqLite(element); + for (i=0; i * + * Using `ngStrictDi`, you would see something like this: + * + + +
+
+ I can add: {{a}} + {{b}} = {{ a+b }} + +

This renders because the controller does not fail to + instantiate, by using explicit annotation style (see + script.js for details) +

+
+ +
+ Name:
+ Hello, {{name}}! + +

This renders because the controller does not fail to + instantiate, by using explicit annotation style + (see script.js for details) +

+
+ +
+ I can add: {{a}} + {{b}} = {{ a+b }} + +

The controller could not be instantiated, due to relying + on automatic function annotations (which are disabled in + strict mode). As such, the content of this section is not + interpolated, and there should be an error in your web console. +

+
+
+ + + angular.module('ngAppStrictDemo', []) + // BadController will fail to instantiate, due to relying on automatic function annotation, + // rather than an explicit annotation + .controller('BadController', function($scope) { + $scope.a = 1; + $scope.b = 2; + }) + // Unlike BadController, GoodController1 and GoodController2 will not fail to be instantiated, + // due to using explicit annotations using the array style and $inject property, respectively. + .controller('GoodController1', ['$scope', function($scope) { + $scope.a = 1; + $scope.b = 2; + }]) + .controller('GoodController2', GoodController2); + function GoodController2($scope) { + $scope.name = "World"; + } + GoodController2.$inject = ['$scope']; + + + div[ng-controller] { + margin-bottom: 1em; + -webkit-border-radius: 4px; + border-radius: 4px; + border: 1px solid; + padding: .5em; + } + div[ng-controller^=Good] { + border-color: #d6e9c6; + background-color: #dff0d8; + color: #3c763d; + } + div[ng-controller^=Bad] { + border-color: #ebccd1; + background-color: #f2dede; + color: #a94442; + margin-bottom: 0; + } + + */ function angularInit(element, bootstrap) { var elements = [element], appElement, module, + config = {}, names = ['ng:app', 'ng-app', 'x-ng-app', 'data-ng-app'], + options = { + 'boolean': ['strict-di'] + }, NG_APP_CLASS_REGEXP = /\sng[:\-]app(:\s*([\w\d_]+);?)?\s/; function append(element) { @@ -1225,7 +1323,8 @@ function angularInit(element, bootstrap) { } }); if (appElement) { - bootstrap(appElement, module ? [module] : []); + config.strictDi = getNgAttribute(appElement, "strict-di") !== null; + bootstrap(appElement, module ? [module] : [], config); } } @@ -1271,9 +1370,20 @@ function angularInit(element, bootstrap) { * Each item in the array should be the name of a predefined module or a (DI annotated) * function that will be invoked by the injector as a run block. * See: {@link angular.module modules} + * @param {Object=} config an object for defining configuration options for the application. The + * following keys are supported: + * + * - `strictDi`: disable automatic function annotation for the application. This is meant to + * assist in finding bugs which break minified code. + * * @returns {auto.$injector} Returns the newly created injector for this app. */ -function bootstrap(element, modules) { +function bootstrap(element, modules, config) { + if (!isObject(config)) config = {}; + var defaultConfig = { + strictDi: false + }; + config = extend(defaultConfig, config); var doBootstrap = function() { element = jqLite(element); @@ -1287,7 +1397,7 @@ function bootstrap(element, modules) { $provide.value('$rootElement', element); }]); modules.unshift('ng'); - var injector = createInjector(modules); + var injector = createInjector(modules, config.strictDi); injector.invoke(['$rootScope', '$rootElement', '$compile', '$injector', '$animate', function(scope, element, compile, injector, animate) { scope.$apply(function() { diff --git a/src/auto/injector.js b/src/auto/injector.js index 303f36f2dbb7..36378a2c8912 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -66,7 +66,19 @@ var FN_ARG_SPLIT = /,/; var FN_ARG = /^\s*(_?)(\S+?)\1\s*$/; var STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg; var $injectorMinErr = minErr('$injector'); -function annotate(fn) { + +function anonFn(fn) { + // For anonymous functions, showing at the very least the function signature can help in + // debugging. + var fnText = fn.toString().replace(STRIP_COMMENTS, ''), + args = fnText.match(FN_ARGS); + if (args) { + return 'function(' + (args[1] || '').replace(/[\s\r\n]+/, ' ') + ')'; + } + return 'fn'; +} + +function annotate(fn, strictDi, name) { var $inject, fnText, argDecl, @@ -76,6 +88,13 @@ function annotate(fn) { if (!($inject = fn.$inject)) { $inject = []; if (fn.length) { + if (strictDi) { + if (!isString(name) || !name) { + name = fn.name || anonFn(fn); + } + throw $injectorMinErr('strictdi', + '{0} is not using explicit annotation and cannot be invoked in strict mode', name); + } fnText = fn.toString().replace(STRIP_COMMENTS, ''); argDecl = fnText.match(FN_ARGS); forEach(argDecl[1].split(FN_ARG_SPLIT), function(arg){ @@ -587,7 +606,8 @@ function annotate(fn) { */ -function createInjector(modulesToLoad) { +function createInjector(modulesToLoad, strictDi) { + strictDi = (strictDi === true); var INSTANTIATING = {}, providerSuffix = 'Provider', path = [], @@ -605,13 +625,13 @@ function createInjector(modulesToLoad) { providerInjector = (providerCache.$injector = createInternalInjector(providerCache, function() { throw $injectorMinErr('unpr', "Unknown provider: {0}", path.join(' <- ')); - })), + }, strictDi)), instanceCache = {}, instanceInjector = (instanceCache.$injector = createInternalInjector(instanceCache, function(servicename) { var provider = providerInjector.get(servicename + providerSuffix); - return instanceInjector.invoke(provider.$get, provider); - })); + return instanceInjector.invoke(provider.$get, provider, undefined, servicename); + }, strictDi)); forEach(loadModules(modulesToLoad), function(fn) { instanceInjector.invoke(fn || noop); }); @@ -743,9 +763,14 @@ function createInjector(modulesToLoad) { } } - function invoke(fn, self, locals){ + function invoke(fn, self, locals, serviceName){ + if (typeof locals === 'string') { + serviceName = locals; + locals = null; + } + var args = [], - $inject = annotate(fn), + $inject = annotate(fn, strictDi, serviceName), length, i, key; @@ -771,7 +796,7 @@ function createInjector(modulesToLoad) { return fn.apply(self, args); } - function instantiate(Type, locals) { + function instantiate(Type, locals, serviceName) { var Constructor = function() {}, instance, returnedValue; @@ -779,7 +804,7 @@ function createInjector(modulesToLoad) { // e.g. someModule.factory('greeter', ['$window', function(renamed$window) {}]); Constructor.prototype = (isArray(Type) ? Type[Type.length - 1] : Type).prototype; instance = new Constructor(); - returnedValue = invoke(Type, instance, locals); + returnedValue = invoke(Type, instance, locals, serviceName); return isObject(returnedValue) || isFunction(returnedValue) ? returnedValue : instance; } @@ -796,3 +821,4 @@ function createInjector(modulesToLoad) { } } +createInjector.$$annotate = annotate; diff --git a/src/ng/controller.js b/src/ng/controller.js index d3d1563308cc..a4ca767e58b7 100644 --- a/src/ng/controller.js +++ b/src/ng/controller.js @@ -71,7 +71,7 @@ function $ControllerProvider() { assertArgFn(expression, constructor, true); } - instance = $injector.instantiate(expression, locals); + instance = $injector.instantiate(expression, locals, constructor); if (identifier) { if (!(locals && typeof locals.$scope == 'object')) { diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index a4dc895bf2f0..4a19c64da015 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -767,7 +767,8 @@ angular.mock.animate = angular.module('ngAnimateMock', ['ng']) }; }); - $provide.decorator('$animate', function($delegate, $$asyncCallback) { + $provide.decorator('$animate', ['$delegate', '$$asyncCallback', + function($delegate, $$asyncCallback) { var animate = { queue : [], enabled : $delegate.enabled, @@ -795,7 +796,7 @@ angular.mock.animate = angular.module('ngAnimateMock', ['ng']) }); return animate; - }); + }]); }]); @@ -1636,7 +1637,7 @@ function MockXhr() { * that adds a "flush" and "verifyNoPendingTasks" methods. */ -angular.mock.$TimeoutDecorator = function($delegate, $browser) { +angular.mock.$TimeoutDecorator = ['$delegate', '$browser', function ($delegate, $browser) { /** * @ngdoc method @@ -1675,9 +1676,9 @@ angular.mock.$TimeoutDecorator = function($delegate, $browser) { } return $delegate; -}; +}]; -angular.mock.$RAFDecorator = function($delegate) { +angular.mock.$RAFDecorator = ['$delegate', function($delegate) { var queue = []; var rafFn = function(fn) { var index = queue.length; @@ -1703,9 +1704,9 @@ angular.mock.$RAFDecorator = function($delegate) { }; return rafFn; -}; +}]; -angular.mock.$AsyncCallbackDecorator = function($delegate) { +angular.mock.$AsyncCallbackDecorator = ['$delegate', function($delegate) { var callbacks = []; var addFn = function(fn) { callbacks.push(fn); @@ -1717,7 +1718,7 @@ angular.mock.$AsyncCallbackDecorator = function($delegate) { callbacks = []; }; return addFn; -}; +}]; /** * @@ -2151,14 +2152,28 @@ if(window.jasmine || window.mocha) { ///////////////////// function workFn() { var modules = currentSpec.$modules || []; - + var strictDi = !!currentSpec.$injectorStrict; modules.unshift('ngMock'); modules.unshift('ng'); var injector = currentSpec.$injector; if (!injector) { - injector = currentSpec.$injector = angular.injector(modules); + if (strictDi) { + // If strictDi is enabled, annotate the providerInjector blocks + angular.forEach(modules, function(moduleFn) { + if (typeof moduleFn === "function") { + angular.injector.$$annotate(moduleFn); + } + }); + } + injector = currentSpec.$injector = angular.injector(modules, strictDi); + currentSpec.$injectorStrict = strictDi; } for(var i = 0, ii = blockFns.length; i < ii; i++) { + if (currentSpec.$injectorStrict) { + // If the injector is strict / strictDi, and the spec wants to inject using automatic + // annotation, then annotate the function here. + injector.annotate(blockFns[i]); + } try { /* jshint -W040 *//* Jasmine explicitly provides a `this` object when calling functions */ injector.invoke(blockFns[i] || angular.noop, this); @@ -2174,4 +2189,20 @@ if(window.jasmine || window.mocha) { } } }; + + + angular.mock.inject.strictDi = function(value) { + value = arguments.length ? !!value : true; + return isSpecRunning() ? workFn() : workFn; + + function workFn() { + if (value !== currentSpec.$injectorStrict) { + if (currentSpec.$injector) { + throw new Error('Injector already created, can not modify strict annotations'); + } else { + currentSpec.$injectorStrict = value; + } + } + } + }; } diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 0f302e37b080..4000f413ecf1 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -513,7 +513,7 @@ function $RouteProvider(){ angular.forEach(locals, function(value, key) { locals[key] = angular.isString(value) ? - $injector.get(value) : $injector.invoke(value); + $injector.get(value) : $injector.invoke(value, null, null, key); }); if (angular.isDefined(template = next.template)) { diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 2eaa985ff1a2..f1b0b7203abb 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -687,7 +687,7 @@ describe('angular', function() { var appElement = jqLite('
')[0]; element.querySelectorAll['[ng-app]'] = [appElement]; angularInit(element, bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC']); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC'], jasmine.any(Object)); }); @@ -695,7 +695,7 @@ describe('angular', function() { var appElement = jqLite('
')[0]; jqLite(document.body).append(appElement); angularInit(element, bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC']); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC'], jasmine.any(Object)); }); @@ -703,7 +703,7 @@ describe('angular', function() { var appElement = jqLite('
')[0]; element.querySelectorAll['.ng\\:app'] = [appElement]; angularInit(element, bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC']); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC'], jasmine.any(Object)); }); @@ -711,14 +711,14 @@ describe('angular', function() { var appElement = jqLite('
')[0]; element.querySelectorAll['[ng\\:app]'] = [ appElement ]; angularInit(element, bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC']); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC'], jasmine.any(Object)); }); it('should bootstrap using class name', function() { var appElement = jqLite('
')[0]; angularInit(jqLite('
').append(appElement)[0], bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC']); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, ['ABC'], jasmine.any(Object)); }); @@ -726,21 +726,21 @@ describe('angular', function() { var appElement = jqLite('
')[0]; element.querySelectorAll['[x-ng-app]'] = [ appElement ]; angularInit(element, bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, []); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, [], jasmine.any(Object)); }); it('should bootstrap anonymously using class only', function() { var appElement = jqLite('
')[0]; angularInit(jqLite('
').append(appElement)[0], bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, []); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, [], jasmine.any(Object)); }); it('should bootstrap if the annotation is on the root element', function() { var appElement = jqLite('
')[0]; angularInit(appElement, bootstrapSpy); - expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, []); + expect(bootstrapSpy).toHaveBeenCalledOnceWith(appElement, [], jasmine.any(Object)); }); @@ -778,7 +778,24 @@ describe('angular', function() { ); dealoc(document); - }) + }); + + + it('should bootstrap in strict mode when ng-strict-di attribute is specified', function() { + bootstrapSpy = spyOn(angular, 'bootstrap').andCallThrough(); + var appElement = jqLite('
'); + angularInit(jqLite('
').append(appElement[0])[0], bootstrapSpy); + expect(bootstrapSpy).toHaveBeenCalledOnce(); + expect(bootstrapSpy.mostRecentCall.args[2].strictDi).toBe(true); + + var injector = appElement.injector(); + function testFactory($rootScope) {}; + expect(function() { + injector.instantiate(testFactory); + }).toThrowMinErr('$injector', 'strictdi'); + + dealoc(appElement); + }); }); diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index 4b9679784c0e..22686b98b0c3 100644 --- a/test/auto/injectorSpec.js +++ b/test/auto/injectorSpec.js @@ -864,3 +864,69 @@ describe('injector', function() { }); }); }); + +describe('strict-di injector', function() { + beforeEach(inject.strictDi(true)); + + describe('with ngMock', function() { + it('should not throw when calling mock.module() with "magic" annotations', function() { + expect(function() { + module(function($provide, $httpProvider, $compileProvider) { + // Don't throw! + }); + }).not.toThrow(); + }); + + + it('should not throw when calling mock.inject() with "magic" annotations', function() { + expect(function() { + inject(function($rootScope, $compile, $http) { + // Don't throw! + }); + }).not.toThrow(); + }); + }); + + + it('should throw if magic annotation is used by service', function() { + module(function($provide) { + $provide.service({ + '$test': function() { return this; }, + '$test2': function($test) { return this; } + }); + }); + inject(function($injector) { + expect (function() { + $injector.invoke(function($test2) {}); + }).toThrowMinErr('$injector', 'strictdi'); + }); + }); + + + it('should throw if magic annotation is used by provider', function() { + module(function($provide) { + $provide.provider({ + '$test': function() { this.$get = function($rootScope) { return $rootScope; }; }, + }); + }); + inject(function($injector) { + expect (function() { + $injector.invoke(['$test', function($test) {}]); + }).toThrowMinErr('$injector', 'strictdi'); + }); + }); + + + it('should throw if magic annotation is used by factory', function() { + module(function($provide) { + $provide.factory({ + '$test': function($rootScope) { return function() {} }, + }); + }); + inject(function($injector) { + expect(function() { + $injector.invoke(['$test', function(test) {}]); + }).toThrowMinErr('$injector', 'strictdi'); + }); + }); +});