feat($controller): disable using global controller constructors

With the exception of simple demos, it is not helpful to use globals
for controller constructors. This adds a new method to `$controllerProvider`
to re-enable the old behavior, but disables this feature by default.

BREAKING CHANGE:
`$controller` will no longer look for controllers on `window`.
The old behavior of looking on `window` for controllers was originally intended
for use in examples, demos, and toy apps. We found that allowing global controller
functions encouraged poor practices, so we resolved to disable this behavior by
default.

To migrate, register your controllers with modules rather than exposing them
as globals:

Before:

```javascript
function MyController() {
  // ...
}
```

After:

```javascript
angular.module('myApp', []).controller('MyController', [function() {
  // ...
}]);
```

Although it's not recommended, you can re-enable the old behavior like this:

```javascript
angular.module('myModule').config(['$controllerProvider', function($controllerProvider) {
  // this option might be handy for migrating old apps, but please don't use it
  // in new ones!
  $controllerProvider.allowGlobals();
}]);
```
This commit is contained in:
Brian Ford
2014-07-01 10:56:55 -07:00
parent d5305d5652
commit 3f2232b5a1
3 changed files with 44 additions and 16 deletions

View File

@@ -12,6 +12,7 @@
*/
function $ControllerProvider() {
var controllers = {},
globals = false,
CNTRL_REG = /^(\S+)(\s+as\s+(\w+))?$/;
@@ -32,6 +33,15 @@ function $ControllerProvider() {
}
};
/**
* @ngdoc method
* @name $controllerProvider#allowGlobals
* @description If called, allows `$controller` to find controller constructors on `window`
*/
this.allowGlobals = function() {
globals = true;
};
this.$get = ['$injector', '$window', function($injector, $window) {
@@ -46,7 +56,8 @@ function $ControllerProvider() {
*
* * check if a controller with given name is registered via `$controllerProvider`
* * check if evaluating the string on the current scope returns a constructor
* * check `window[constructor]` on the global `window` object
* * if $controllerProvider#allowGlobals, check `window[constructor]` on the global
* `window` object (not recommended)
*
* @param {Object} locals Injection locals for Controller.
* @return {Object} Instance of given controller.
@@ -66,7 +77,8 @@ function $ControllerProvider() {
identifier = match[3];
expression = controllers.hasOwnProperty(constructor)
? controllers[constructor]
: getter(locals.$scope, constructor, true) || getter($window, constructor, true);
: getter(locals.$scope, constructor, true) ||
(globals ? getter($window, constructor, true) : undefined);
assertArgFn(expression, constructor, true);
}

View File

@@ -64,6 +64,21 @@ describe('$controller', function() {
$controllerProvider.register('hasOwnProperty', function($scope) {});
}).toThrowMinErr('ng', 'badname', "hasOwnProperty is not a valid controller name");
});
it('should instantiate a controller defined on window if allowGlobals is set',
inject(function($window) {
var scope = {};
var Foo = function() {};
$controllerProvider.allowGlobals();
$window.a = {Foo: Foo};
var foo = $controller('a.Foo', {$scope: scope});
expect(foo).toBeDefined();
expect(foo instanceof Foo).toBe(true);
}));
});
@@ -97,15 +112,15 @@ describe('$controller', function() {
});
it('should instantiate controller defined on window', inject(function($window) {
it('should not instantiate a controller defined on window', inject(function($window) {
var scope = {};
var Foo = function() {};
$window.a = {Foo: Foo};
var foo = $controller('a.Foo', {$scope: scope});
expect(foo).toBeDefined();
expect(foo instanceof Foo).toBe(true);
expect(function () {
$controller('a.Foo', {$scope: scope});
}).toThrow();
}));

View File

@@ -7,9 +7,8 @@ describe('ngController', function() {
$controllerProvider.register('PublicModule', function() {
this.mark = 'works';
});
}));
beforeEach(inject(function($window) {
$window.Greeter = function($scope) {
var Greeter = function($scope) {
// private stuff (not exported to scope)
this.prefix = 'Hello ';
@@ -22,20 +21,22 @@ describe('ngController', function() {
$scope.protoGreet = bind(this, this.protoGreet);
};
$window.Greeter.prototype = {
Greeter.prototype = {
suffix: '!',
protoGreet: function(name) {
return this.prefix + name + this.suffix;
}
};
$controllerProvider.register('Greeter', Greeter);
$window.Child = function($scope) {
$controllerProvider.register('Child', function($scope) {
$scope.name = 'Adam';
};
});
$window.Public = function() {
$controllerProvider.register('Public', function($scope) {
this.mark = 'works';
};
});
}));
afterEach(function() {
@@ -77,11 +78,11 @@ describe('ngController', function() {
it('should instantiate controller defined on scope', inject(function($compile, $rootScope) {
$rootScope.Greeter = function($scope) {
$rootScope.VojtaGreeter = function($scope) {
$scope.name = 'Vojta';
};
element = $compile('<div ng-controller="Greeter">{{name}}</div>')($rootScope);
element = $compile('<div ng-controller="VojtaGreeter">{{name}}</div>')($rootScope);
$rootScope.$digest();
expect(element.text()).toBe('Vojta');
}));