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

Commit 252e8b5

Browse files
IgorMinarIgorMinar
committed
revert: feat($compile): bind isolate scope properties to controller
This reverts commit 787c5a7. This change causes a regression at Google. We'll take a better look at it next week. Reopens #7645
1 parent 0c4997f commit 252e8b5

File tree

3 files changed

+52
-194
lines changed

3 files changed

+52
-194
lines changed

‎src/ng/compile.js‎

Lines changed: 43 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,6 @@
175175
* by calling the `localFn` as `localFn({amount: 22})`.
176176
*
177177
*
178-
* #### `bindToController`
179-
* When an isolate scope is used for a component (see above), and `controllerAs` is used, `bindToController` will
180-
* allow a component to have its properties bound to the controller, rather than to scope. When the controller
181-
* is instantiated, the initial values of the isolate scope bindings are already available.
182178
*
183179
* #### `controller`
184180
* Controller constructor function. The controller is instantiated before the
@@ -894,7 +890,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
894890

895891
if (transcludeControllers) {
896892
for (var controllerName in transcludeControllers) {
897-
$linkNode.data('$' + controllerName + 'Controller', transcludeControllers[controllerName].instance);
893+
$linkNode.data('$' + controllerName + 'Controller', transcludeControllers[controllerName]);
898894
}
899895
}
900896

@@ -1225,7 +1221,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
12251221
var terminalPriority = -Number.MAX_VALUE,
12261222
newScopeDirective,
12271223
controllerDirectives = previousCompileContext.controllerDirectives,
1228-
controllers,
12291224
newIsolateScopeDirective = previousCompileContext.newIsolateScopeDirective,
12301225
templateDirective = previousCompileContext.templateDirective,
12311226
nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective,
@@ -1463,9 +1458,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
14631458
value = null;
14641459

14651460
if (elementControllers && retrievalMethod === 'data') {
1466-
if (value = elementControllers[require]) {
1467-
value = value.instance;
1468-
}
1461+
value = elementControllers[require];
14691462
}
14701463
value = value || $element[retrievalMethod]('$' + require + 'Controller');
14711464

@@ -1497,62 +1490,22 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
14971490
attrs = new Attributes($element, templateAttrs);
14981491
}
14991492

1500-
if (newIsolateScopeDirective) {
1501-
isolateScope = scope.$new(true);
1502-
}
1503-
1504-
transcludeFn = boundTranscludeFn && controllersBoundTransclude;
1505-
if (controllerDirectives) {
1506-
// TODO: merge `controllers` and `elementControllers` into single object.
1507-
controllers = {};
1508-
elementControllers = {};
1509-
forEach(controllerDirectives, function(directive) {
1510-
var locals = {
1511-
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
1512-
$element: $element,
1513-
$attrs: attrs,
1514-
$transclude: transcludeFn
1515-
}, controllerInstance;
1516-
1517-
controller = directive.controller;
1518-
if (controller == '@') {
1519-
controller = attrs[directive.name];
1520-
}
1521-
1522-
controllerInstance = $controller(controller, locals, true, directive.controllerAs);
1523-
1524-
// For directives with element transclusion the element is a comment,
1525-
// but jQuery .data doesn't support attaching data to comment nodes as it's hard to
1526-
// clean up (http://bugs.jquery.com/ticket/8335).
1527-
// Instead, we save the controllers for the element in a local hash and attach to .data
1528-
// later, once we have the actual element.
1529-
elementControllers[directive.name] = controllerInstance;
1530-
if (!hasElementTranscludeDirective) {
1531-
$element.data('$' + directive.name + 'Controller', controllerInstance.instance);
1532-
}
1533-
1534-
controllers[directive.name] = controllerInstance;
1535-
});
1536-
}
1537-
15381493
if (newIsolateScopeDirective) {
15391494
var LOCAL_REGEXP = /^\s*([@=&])(\??)\s*(\w*)\s*$/;
15401495

1496+
isolateScope = scope.$new(true);
1497+
15411498
if (templateDirective && (templateDirective === newIsolateScopeDirective ||
15421499
templateDirective === newIsolateScopeDirective.$$originalDirective)) {
15431500
$element.data('$isolateScope', isolateScope);
15441501
} else {
15451502
$element.data('$isolateScopeNoTemplate', isolateScope);
15461503
}
15471504

1505+
1506+
15481507
safeAddClass($element, 'ng-isolate-scope');
15491508

1550-
var isolateScopeController = controllers && controllers[newIsolateScopeDirective.name];
1551-
var isolateBindingContext = isolateScope;
1552-
if (isolateScopeController && isolateScopeController.identifier &&
1553-
newIsolateScopeDirective.bindToController === true) {
1554-
isolateBindingContext = isolateScopeController.instance;
1555-
}
15561509
forEach(newIsolateScopeDirective.scope, function(definition, scopeName) {
15571510
var match = definition.match(LOCAL_REGEXP) || [],
15581511
attrName = match[3] || scopeName,
@@ -1573,7 +1526,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
15731526
if( attrs[attrName] ) {
15741527
// If the attribute has been provided then we trigger an interpolation to ensure
15751528
// the value is there for use in the link fn
1576-
isolateBindingContext[scopeName] = $interpolate(attrs[attrName])(scope);
1529+
isolateScope[scopeName] = $interpolate(attrs[attrName])(scope);
15771530
}
15781531
break;
15791532

@@ -1589,21 +1542,21 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
15891542
}
15901543
parentSet = parentGet.assign || function() {
15911544
// reset the change, or we will throw this exception on every $digest
1592-
lastValue = isolateBindingContext[scopeName] = parentGet(scope);
1545+
lastValue = isolateScope[scopeName] = parentGet(scope);
15931546
throw $compileMinErr('nonassign',
15941547
"Expression '{0}' used with directive '{1}' is non-assignable!",
15951548
attrs[attrName], newIsolateScopeDirective.name);
15961549
};
1597-
lastValue = isolateBindingContext[scopeName] = parentGet(scope);
1550+
lastValue = isolateScope[scopeName] = parentGet(scope);
15981551
var unwatch = scope.$watch($parse(attrs[attrName], function parentValueWatch(parentValue) {
1599-
if (!compare(parentValue, isolateBindingContext[scopeName])) {
1552+
if (!compare(parentValue, isolateScope[scopeName])) {
16001553
// we are out of sync and need to copy
16011554
if (!compare(parentValue, lastValue)) {
16021555
// parent changed and it has precedence
1603-
isolateBindingContext[scopeName] = parentValue;
1556+
isolateScope[scopeName] = parentValue;
16041557
} else {
16051558
// if the parent can be assigned then do so
1606-
parentSet(scope, parentValue = isolateBindingContext[scopeName]);
1559+
parentSet(scope, parentValue = isolateScope[scopeName]);
16071560
}
16081561
}
16091562
return lastValue = parentValue;
@@ -1613,7 +1566,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
16131566

16141567
case '&':
16151568
parentGet = $parse(attrs[attrName]);
1616-
isolateBindingContext[scopeName] = function(locals) {
1569+
isolateScope[scopeName] = function(locals) {
16171570
return parentGet(scope, locals);
16181571
};
16191572
break;
@@ -1626,11 +1579,37 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
16261579
}
16271580
});
16281581
}
1629-
if (controllers) {
1630-
forEach(controllers, function(controller) {
1631-
controller();
1582+
transcludeFn = boundTranscludeFn && controllersBoundTransclude;
1583+
if (controllerDirectives) {
1584+
elementControllers = {};
1585+
forEach(controllerDirectives, function(directive) {
1586+
var locals = {
1587+
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
1588+
$element: $element,
1589+
$attrs: attrs,
1590+
$transclude: transcludeFn
1591+
}, controllerInstance;
1592+
1593+
controller = directive.controller;
1594+
if (controller == '@') {
1595+
controller = attrs[directive.name];
1596+
}
1597+
1598+
controllerInstance = $controller(controller, locals);
1599+
// For directives with element transclusion the element is a comment,
1600+
// but jQuery .data doesn't support attaching data to comment nodes as it's hard to
1601+
// clean up (http://bugs.jquery.com/ticket/8335).
1602+
// Instead, we save the controllers for the element in a local hash and attach to .data
1603+
// later, once we have the actual element.
1604+
elementControllers[directive.name] = controllerInstance;
1605+
if (!hasElementTranscludeDirective) {
1606+
$element.data('$' + directive.name + 'Controller', controllerInstance);
1607+
}
1608+
1609+
if (directive.controllerAs) {
1610+
locals.$scope[directive.controllerAs] = controllerInstance;
1611+
}
16321612
});
1633-
controllers = null;
16341613
}
16351614

16361615
// PRELINKING

‎src/ng/controller.js‎

Lines changed: 9 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,13 @@ function $ControllerProvider() {
6868
* It's just a simple call to {@link auto.$injector $injector}, but extracted into
6969
* a service, so that one can override this service with [BC version](https://gist.github.com/1649788).
7070
*/
71-
return function(expression, locals, later, ident) {
72-
// PRIVATE API:
73-
// param `later` --- indicates that the controller's constructor is invoked at a later time.
74-
// If true, $controller will allocate the object with the correct
75-
// prototype chain, but will not invoke the controller until a returned
76-
// callback is invoked.
77-
// param `ident` --- An optional label which overrides the label parsed from the controller
78-
// expression, if any.
71+
return function(expression, locals) {
7972
var instance, match, constructor, identifier;
80-
later = later === true;
81-
if (ident && isString(ident)) {
82-
identifier = ident;
83-
}
8473

8574
if(isString(expression)) {
8675
match = expression.match(CNTRL_REG),
8776
constructor = match[1],
88-
identifier = identifier || match[3];
77+
identifier = match[3];
8978
expression = controllers.hasOwnProperty(constructor)
9079
? controllers[constructor]
9180
: getter(locals.$scope, constructor, true) ||
@@ -94,51 +83,19 @@ function $ControllerProvider() {
9483
assertArgFn(expression, constructor, true);
9584
}
9685

97-
if (later) {
98-
// Instantiate controller later:
99-
// This machinery is used to create an instance of the object before calling the
100-
// controller's constructor itself.
101-
//
102-
// This allows properties to be added to the controller before the constructor is
103-
// invoked. Primarily, this is used for isolate scope bindings in $compile.
104-
//
105-
// This feature is not intended for use by applications, and is thus not documented
106-
// publicly.
107-
var Constructor = function() {};
108-
Constructor.prototype = (isArray(expression) ?
109-
expression[expression.length - 1] : expression).prototype;
110-
instance = new Constructor();
111-
112-
if (identifier) {
113-
addIdentifier(locals, identifier, instance, constructor || expression.name);
114-
}
115-
116-
return extend(function() {
117-
$injector.invoke(expression, instance, locals, constructor);
118-
return instance;
119-
}, {
120-
instance: instance,
121-
identifier: identifier
122-
});
123-
}
124-
12586
instance = $injector.instantiate(expression, locals, constructor);
12687

12788
if (identifier) {
128-
addIdentifier(locals, identifier, instance, constructor || expression.name);
89+
if (!(locals && typeof locals.$scope === 'object')) {
90+
throw minErr('$controller')('noscp',
91+
"Cannot export controller '{0}' as '{1}'! No $scope object provided via `locals`.",
92+
constructor || expression.name, identifier);
93+
}
94+
95+
locals.$scope[identifier] = instance;
12996
}
13097

13198
return instance;
13299
};
133-
134-
function addIdentifier(locals, identifier, instance, name) {
135-
if (!(locals && isObject(locals.$scope))) {
136-
throw minErr('$controller')('noscp',
137-
"Cannot export controller '{0}' as '{1}'! No $scope object provided via `locals`.",
138-
name, identifier);
139-
}
140-
141-
locals.$scope[identifier] = instance;
142-
}
143100
}];
144101
}

‎test/ng/compileSpec.js‎

Lines changed: 0 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -3308,84 +3308,6 @@ describe('$compile', function() {
33083308
expect(componentScope.$$isolateBindings.exprAlias).toBe('&expr');
33093309

33103310
}));
3311-
3312-
3313-
it('should expose isolate scope variables on controller with controllerAs when bindToController is true', function() {
3314-
var controllerCalled = false;
3315-
module(function($compileProvider) {
3316-
$compileProvider.directive('fooDir', valueFn({
3317-
template: '<p>isolate</p>',
3318-
scope: {
3319-
'data': '=dirData',
3320-
'str': '@dirStr',
3321-
'fn': '&dirFn'
3322-
},
3323-
controller: function($scope) {
3324-
expect(this.data).toEqualData({
3325-
'foo': 'bar',
3326-
'baz': 'biz'
3327-
});
3328-
expect(this.str).toBe('Hello, world!');
3329-
expect(this.fn()).toBe('called!');
3330-
controllerCalled = true;
3331-
},
3332-
controllerAs: 'test',
3333-
bindToController: true
3334-
}));
3335-
});
3336-
inject(function($compile, $rootScope) {
3337-
$rootScope.fn = valueFn('called!');
3338-
$rootScope.whom = 'world';
3339-
$rootScope.remoteData = {
3340-
'foo': 'bar',
3341-
'baz': 'biz'
3342-
};
3343-
element = $compile('<div foo-dir dir-data="remoteData" ' +
3344-
'dir-str="Hello, {{whom}}!" ' +
3345-
'dir-fn="fn()"></div>')($rootScope);
3346-
expect(controllerCalled).toBe(true);
3347-
});
3348-
});
3349-
3350-
3351-
it('should expose isolate scope variables on controller with controllerAs when bindToController is true', function() {
3352-
var controllerCalled = false;
3353-
module(function($compileProvider) {
3354-
$compileProvider.directive('fooDir', valueFn({
3355-
templateUrl: 'test.html',
3356-
scope: {
3357-
'data': '=dirData',
3358-
'str': '@dirStr',
3359-
'fn': '&dirFn'
3360-
},
3361-
controller: function($scope) {
3362-
expect(this.data).toEqualData({
3363-
'foo': 'bar',
3364-
'baz': 'biz'
3365-
});
3366-
expect(this.str).toBe('Hello, world!');
3367-
expect(this.fn()).toBe('called!');
3368-
controllerCalled = true;
3369-
},
3370-
controllerAs: 'test',
3371-
bindToController: true
3372-
}));
3373-
});
3374-
inject(function($compile, $rootScope, $templateCache) {
3375-
$templateCache.put('test.html', '<p>isolate</p>');
3376-
$rootScope.fn = valueFn('called!');
3377-
$rootScope.whom = 'world';
3378-
$rootScope.remoteData = {
3379-
'foo': 'bar',
3380-
'baz': 'biz'
3381-
};
3382-
element = $compile('<div foo-dir dir-data="remoteData" ' +
3383-
'dir-str="Hello, {{whom}}!" ' +
3384-
'dir-fn="fn()"></div>')($rootScope);
3385-
$rootScope.$digest();
3386-
expect(controllerCalled).toBe(true);
3387-
});
3388-
});
33893311
});
33903312

33913313

0 commit comments

Comments
 (0)