Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 2ee9dc2

Browse files
thorn0thorn0
committed
refactor($compile): remove preAssignBindingsEnabled leftovers
Now that we don't need to support `preAssignBindingsEnabled` (removed in #15782), complexity introduced in `$controller` by #7645 can be removed. One difference with the previous implementation is that all non-ES2015-class controller instances were available on the element before calling their constructors. Now it depends on the relative order of controllers. Controller constructors shouldn't be used to access other controllers (e.g. via `$element.controller(directiveName)`). The recommended way is to use the `require` property of the directive definition object and the life cycle hooks `$onChanges` or `$onInit`. See https://docs.angularjs.org/api/ng/service/$compile#-require- https://docs.angularjs.org/api/ng/service/$compile#life-cycle-hooks
1 parent 5262039 commit 2ee9dc2

File tree

5 files changed

+67
-167
lines changed

5 files changed

+67
-167
lines changed

‎src/auto/injector.js‎

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,8 @@ var FN_ARG = /^\s*(_?)(\S+?)\1\s*$/;
7070
var STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
7171
var $injectorMinErr = minErr('$injector');
7272

73-
function stringifyFn(fn) {
74-
return Function.prototype.toString.call(fn);
75-
}
76-
7773
function extractArgs(fn) {
78-
var fnText = stringifyFn(fn).replace(STRIP_COMMENTS, ''),
74+
var fnText = Function.prototype.toString.call(fn).replace(STRIP_COMMENTS, ''),
7975
args = fnText.match(ARROW_ARG) || fnText.match(FN_ARGS);
8076
return args;
8177
}
@@ -914,19 +910,6 @@ function createInjector(modulesToLoad, strictDi) {
914910
return args;
915911
}
916912

917-
function isClass(func) {
918-
// Support: IE 9-11 only
919-
// IE 9-11 do not support classes and IE9 leaks with the code below.
920-
if (msie || typeof func !== 'function') {
921-
return false;
922-
}
923-
var result = func.$$ngIsClass;
924-
if (!isBoolean(result)) {
925-
result = func.$$ngIsClass = /^class\b/.test(stringifyFn(func));
926-
}
927-
return result;
928-
}
929-
930913
function invoke(fn, self, locals, serviceName) {
931914
if (typeof locals === 'string') {
932915
serviceName = locals;
@@ -938,14 +921,9 @@ function createInjector(modulesToLoad, strictDi) {
938921
fn = fn[fn.length - 1];
939922
}
940923

941-
if (!isClass(fn)) {
942-
// http://jsperf.com/angularjs-invoke-apply-vs-switch
943-
// #5388
944-
return fn.apply(self, args);
945-
} else {
946-
args.unshift(null);
947-
return new (Function.prototype.bind.apply(fn, args))();
948-
}
924+
// http://jsperf.com/angularjs-invoke-apply-vs-switch
925+
// #5388
926+
return fn.apply(self, args);
949927
}
950928

951929

‎src/ng/compile.js‎

Lines changed: 57 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -2788,10 +2788,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
27882788
};
27892789
}
27902790

2791-
if (controllerDirectives) {
2792-
elementControllers = setupControllers($element, attrs, transcludeFn, controllerDirectives, isolateScope, scope, newIsolateScopeDirective);
2793-
}
2794-
27952791
if (newIsolateScopeDirective) {
27962792
// Initialize isolate scope bindings for new isolate scope directive.
27972793
compile.$$addScopeInfo($element, isolateScope, true, !(templateDirective && (templateDirective === newIsolateScopeDirective ||
@@ -2807,53 +2803,69 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
28072803
}
28082804
}
28092805

2810-
// Initialize bindToController bindings
2811-
for (var name in elementControllers) {
2812-
var controllerDirective = controllerDirectives[name];
2813-
var controller = elementControllers[name];
2814-
var bindings = controllerDirective.$$bindings.bindToController;
2806+
if (controllerDirectives) {
2807+
elementControllers = createMap();
2808+
for (var name in controllerDirectives) {
2809+
var directive = controllerDirectives[name];
2810+
var locals = {
2811+
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
2812+
$element: $element,
2813+
$attrs: attrs,
2814+
$transclude: transcludeFn
2815+
};
28152816

2816-
controller.instance = controller();
2817-
$element.data('$' + controllerDirective.name + 'Controller', controller.instance);
2818-
controller.bindingInfo =
2819-
initializeDirectiveBindings(controllerScope, attrs, controller.instance, bindings, controllerDirective);
2820-
}
2817+
var controllerConstructor = directive.controller;
2818+
if (controllerConstructor === '@') {
2819+
controllerConstructor = attrs[name];
2820+
}
28212821

2822-
// Bind the required controllers to the controller, if `require` is an object and `bindToController` is truthy
2823-
forEach(controllerDirectives, function(controllerDirective, name) {
2824-
var require = controllerDirective.require;
2825-
if (controllerDirective.bindToController && !isArray(require) && isObject(require)) {
2826-
extend(elementControllers[name].instance, getControllers(name, require, $element, elementControllers));
2822+
var instance = $controller(controllerConstructor, locals, directive.controllerAs);
2823+
2824+
$element.data('$' + name + 'Controller', instance);
2825+
2826+
// Initialize bindToController bindings
2827+
var bindings = directive.$$bindings.bindToController;
2828+
var bindingInfo = initializeDirectiveBindings(controllerScope, attrs, instance, bindings, directive);
2829+
2830+
elementControllers[name] = { instance: instance, bindingInfo: bindingInfo };
28272831
}
2828-
});
28292832

2830-
// Handle the init and destroy lifecycle hooks on all controllers that have them
2831-
forEach(elementControllers, function(controller) {
2832-
var controllerInstance = controller.instance;
2833-
if (isFunction(controllerInstance.$onChanges)) {
2834-
try {
2835-
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2836-
} catch (e) {
2837-
$exceptionHandler(e);
2833+
// Bind the required controllers to the controller, if `require` is an object and `bindToController` is truthy
2834+
forEach(controllerDirectives, function(controllerDirective, name) {
2835+
var require = controllerDirective.require;
2836+
if (controllerDirective.bindToController && !isArray(require) && isObject(require)) {
2837+
extend(elementControllers[name].instance, getControllers(name, require, $element, elementControllers));
28382838
}
2839-
}
2840-
if (isFunction(controllerInstance.$onInit)) {
2841-
try {
2842-
controllerInstance.$onInit();
2843-
} catch (e) {
2844-
$exceptionHandler(e);
2839+
});
2840+
2841+
// Handle the init and destroy lifecycle hooks on all controllers that have them
2842+
forEach(elementControllers, function(controller) {
2843+
var controllerInstance = controller.instance;
2844+
if (isFunction(controllerInstance.$onChanges)) {
2845+
try {
2846+
controllerInstance.$onChanges(controller.bindingInfo.initialChanges);
2847+
} catch (e) {
2848+
$exceptionHandler(e);
2849+
}
28452850
}
2846-
}
2847-
if (isFunction(controllerInstance.$doCheck)) {
2848-
controllerScope.$watch(function() { controllerInstance.$doCheck(); });
2849-
controllerInstance.$doCheck();
2850-
}
2851-
if (isFunction(controllerInstance.$onDestroy)) {
2852-
controllerScope.$on('$destroy', function callOnDestroyHook() {
2853-
controllerInstance.$onDestroy();
2854-
});
2855-
}
2856-
});
2851+
if (isFunction(controllerInstance.$onInit)) {
2852+
try {
2853+
controllerInstance.$onInit();
2854+
} catch (e) {
2855+
$exceptionHandler(e);
2856+
}
2857+
}
2858+
if (isFunction(controllerInstance.$doCheck)) {
2859+
controllerScope.$watch(function() { controllerInstance.$doCheck(); });
2860+
controllerInstance.$doCheck();
2861+
}
2862+
if (isFunction(controllerInstance.$onDestroy)) {
2863+
controllerScope.$on('$destroy', function callOnDestroyHook() {
2864+
controllerInstance.$onDestroy();
2865+
});
2866+
}
2867+
});
2868+
}
28572869

28582870
// PRELINKING
28592871
for (i = 0, ii = preLinkFns.length; i < ii; i++) {
@@ -2981,34 +2993,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
29812993
return value || null;
29822994
}
29832995

2984-
function setupControllers($element, attrs, transcludeFn, controllerDirectives, isolateScope, scope, newIsolateScopeDirective) {
2985-
var elementControllers = createMap();
2986-
for (var controllerKey in controllerDirectives) {
2987-
var directive = controllerDirectives[controllerKey];
2988-
var locals = {
2989-
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
2990-
$element: $element,
2991-
$attrs: attrs,
2992-
$transclude: transcludeFn
2993-
};
2994-
2995-
var controller = directive.controller;
2996-
if (controller === '@') {
2997-
controller = attrs[directive.name];
2998-
}
2999-
3000-
var controllerInstance = $controller(controller, locals, true, directive.controllerAs);
3001-
3002-
// For directives with element transclusion the element is a comment.
3003-
// In this case .data will not attach any data.
3004-
// Instead, we save the controllers for the element in a local hash and attach to .data
3005-
// later, once we have the actual element.
3006-
elementControllers[directive.name] = controllerInstance;
3007-
$element.data('$' + directive.name + 'Controller', controllerInstance.instance);
3008-
}
3009-
return elementControllers;
3010-
}
3011-
30122996
// Depending upon the context in which a directive finds itself it might need to have a new isolated
30132997
// or child scope created. For instance:
30142998
// * if the directive has been pulled into a template because another directive with a higher priority

‎src/ng/controller.js‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,11 @@ function $ControllerProvider() {
8181
* It's just a simple call to {@link auto.$injector $injector}, but extracted into
8282
* a service, so that one can override this service with [BC version](https://gist.github.com/1649788).
8383
*/
84-
return function $controller(expression, locals, later, ident) {
84+
return function $controller(expression, locals, ident) {
8585
// PRIVATE API:
86-
// param `later` --- indicates that the controller's constructor is invoked at a later time.
87-
// If true, $controller will allocate the object with the correct
88-
// prototype chain, but will not invoke the controller until a returned
89-
// callback is invoked.
9086
// param `ident` --- An optional label which overrides the label parsed from the controller
9187
// expression, if any.
9288
var instance, match, constructor, identifier;
93-
later = later === true;
9489
if (ident && isString(ident)) {
9590
identifier = ident;
9691
}
@@ -116,41 +111,6 @@ function $ControllerProvider() {
116111
assertArgFn(expression, constructor, true);
117112
}
118113

119-
if (later) {
120-
// Instantiate controller later:
121-
// This machinery is used to create an instance of the object before calling the
122-
// controller's constructor itself.
123-
//
124-
// This allows properties to be added to the controller before the constructor is
125-
// invoked. Primarily, this is used for isolate scope bindings in $compile.
126-
//
127-
// This feature is not intended for use by applications, and is thus not documented
128-
// publicly.
129-
// Object creation: http://jsperf.com/create-constructor/2
130-
var controllerPrototype = (isArray(expression) ?
131-
expression[expression.length - 1] : expression).prototype;
132-
instance = Object.create(controllerPrototype || null);
133-
134-
if (identifier) {
135-
addIdentifier(locals, identifier, instance, constructor || expression.name);
136-
}
137-
138-
return extend(function $controllerInit() {
139-
var result = $injector.invoke(expression, instance, locals, constructor);
140-
if (result !== instance && (isObject(result) || isFunction(result))) {
141-
instance = result;
142-
if (identifier) {
143-
// If result changed, re-assign controllerAs value to scope.
144-
addIdentifier(locals, identifier, instance, constructor || expression.name);
145-
}
146-
}
147-
return instance;
148-
}, {
149-
instance: instance,
150-
identifier: identifier
151-
});
152-
}
153-
154114
instance = $injector.instantiate(expression, locals, constructor);
155115

156116
if (identifier) {

‎src/ngMock/angular-mocks.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,14 +2332,11 @@ angular.mock.$RootElementProvider = function() {
23322332
*/
23332333
function createControllerDecorator() {
23342334
angular.mock.$ControllerDecorator = ['$delegate', function($delegate) {
2335-
return function(expression, locals, later, ident) {
2336-
if (later && typeof later === 'object') {
2337-
var instantiate = $delegate(expression, locals, true, ident);
2338-
var instance = instantiate();
2339-
angular.extend(instance, later);
2340-
return instance;
2341-
}
2342-
return $delegate(expression, locals, later, ident);
2335+
return function(expression, locals, bindings, ident) {
2336+
if (angular.isString(bindings)) ident = bindings;
2337+
var instance = $delegate(expression, locals, ident);
2338+
angular.extend(instance, bindings);
2339+
return instance;
23432340
};
23442341
}];
23452342

‎test/auto/injectorSpec.js‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -482,25 +482,6 @@ describe('injector', function() {
482482
expect(instance).toEqual(new Clazz('a-value'));
483483
expect(instance.aVal()).toEqual('a-value');
484484
});
485-
486-
they('should detect ES6 classes regardless of whitespace/comments ($prop)', [
487-
'class Test {}',
488-
'class Test{}',
489-
'class //<--ES6 stuff\nTest {}',
490-
'class//<--ES6 stuff\nTest {}',
491-
'class {}',
492-
'class{}',
493-
'class //<--ES6 stuff\n {}',
494-
'class//<--ES6 stuff\n {}',
495-
'class/* Test */{}',
496-
'class /* Test */ {}'
497-
], function(classDefinition) {
498-
// eslint-disable-next-line no-eval
499-
var Clazz = eval('(' + classDefinition + ')');
500-
var instance = injector.invoke(Clazz);
501-
502-
expect(instance).toEqual(jasmine.any(Clazz));
503-
});
504485
}
505486
});
506487

0 commit comments

Comments
 (0)