Skip to content

Commit

Permalink
revert: feat($compile): bind isolate scope properties to controller
Browse files Browse the repository at this point in the history 
This reverts commit 787c5a7.

This change causes a regression at Google. We'll take a better look at it
next week.

Reopens angular#7645
  • Loading branch information
@IgorMinar
IgorMinar committed Aug 22, 2014
1 parent 0c4997f commit 252e8b5
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 194 deletions.
107 changes: 43 additions & 64 deletions src/ng/compile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,6 @@
* by calling the `localFn` as `localFn({amount: 22})`.
*
*
* #### `bindToController`
* When an isolate scope is used for a component (see above), and `controllerAs` is used, `bindToController` will
* allow a component to have its properties bound to the controller, rather than to scope. When the controller
* is instantiated, the initial values of the isolate scope bindings are already available.
*
* #### `controller`
* Controller constructor function. The controller is instantiated before the
Expand Down Expand Up @@ -894,7 +890,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

if (transcludeControllers) {
for (var controllerName in transcludeControllers) {
$linkNode.data('$' + controllerName + 'Controller', transcludeControllers[controllerName].instance);
$linkNode.data('$' + controllerName + 'Controller', transcludeControllers[controllerName]);
}
}

Expand Down Expand Up @@ -1225,7 +1221,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
var terminalPriority = -Number.MAX_VALUE,
newScopeDirective,
controllerDirectives = previousCompileContext.controllerDirectives,
controllers,
newIsolateScopeDirective = previousCompileContext.newIsolateScopeDirective,
templateDirective = previousCompileContext.templateDirective,
nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective,
Expand Down Expand Up @@ -1463,9 +1458,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
value = null;

if (elementControllers && retrievalMethod === 'data') {
if (value = elementControllers[require]) {
value = value.instance;
}
value = elementControllers[require];
}
value = value || $element[retrievalMethod]('$' + require + 'Controller');

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

if (newIsolateScopeDirective) {
isolateScope = scope.$new(true);
}

transcludeFn = boundTranscludeFn && controllersBoundTransclude;
if (controllerDirectives) {
// TODO: merge `controllers` and `elementControllers` into single object.
controllers = {};
elementControllers = {};
forEach(controllerDirectives, function(directive) {
var locals = {
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
$element: $element,
$attrs: attrs,
$transclude: transcludeFn
}, controllerInstance;

controller = directive.controller;
if (controller == '@') {
controller = attrs[directive.name];
}

controllerInstance = $controller(controller, locals, true, directive.controllerAs);

// For directives with element transclusion the element is a comment,
// but jQuery .data doesn't support attaching data to comment nodes as it's hard to
// clean up (http://bugs.jquery.com/ticket/8335).
// Instead, we save the controllers for the element in a local hash and attach to .data
// later, once we have the actual element.
elementControllers[directive.name] = controllerInstance;
if (!hasElementTranscludeDirective) {
$element.data('$' + directive.name + 'Controller', controllerInstance.instance);
}

controllers[directive.name] = controllerInstance;
});
}

if (newIsolateScopeDirective) {
var LOCAL_REGEXP = /^\s*([@=&])(\??)\s*(\w*)\s*$/;

isolateScope = scope.$new(true);

if (templateDirective && (templateDirective === newIsolateScopeDirective ||
templateDirective === newIsolateScopeDirective.$$originalDirective)) {
$element.data('$isolateScope', isolateScope);
} else {
$element.data('$isolateScopeNoTemplate', isolateScope);
}



safeAddClass($element, 'ng-isolate-scope');

var isolateScopeController = controllers && controllers[newIsolateScopeDirective.name];
var isolateBindingContext = isolateScope;
if (isolateScopeController && isolateScopeController.identifier &&
newIsolateScopeDirective.bindToController === true) {
isolateBindingContext = isolateScopeController.instance;
}
forEach(newIsolateScopeDirective.scope, function(definition, scopeName) {
var match = definition.match(LOCAL_REGEXP) || [],
attrName = match[3] || scopeName,
Expand All @@ -1573,7 +1526,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
if( attrs[attrName] ) {
// If the attribute has been provided then we trigger an interpolation to ensure
// the value is there for use in the link fn
isolateBindingContext[scopeName] = $interpolate(attrs[attrName])(scope);
isolateScope[scopeName] = $interpolate(attrs[attrName])(scope);
}
break;

Expand All @@ -1589,21 +1542,21 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
parentSet = parentGet.assign || function() {
// reset the change, or we will throw this exception on every $digest
lastValue = isolateBindingContext[scopeName] = parentGet(scope);
lastValue = isolateScope[scopeName] = parentGet(scope);
throw $compileMinErr('nonassign',
"Expression '{0}' used with directive '{1}' is non-assignable!",
attrs[attrName], newIsolateScopeDirective.name);
};
lastValue = isolateBindingContext[scopeName] = parentGet(scope);
lastValue = isolateScope[scopeName] = parentGet(scope);
var unwatch = scope.$watch($parse(attrs[attrName], function parentValueWatch(parentValue) {
if (!compare(parentValue, isolateBindingContext[scopeName])) {
if (!compare(parentValue, isolateScope[scopeName])) {
// we are out of sync and need to copy
if (!compare(parentValue, lastValue)) {
// parent changed and it has precedence
isolateBindingContext[scopeName] = parentValue;
isolateScope[scopeName] = parentValue;
} else {
// if the parent can be assigned then do so
parentSet(scope, parentValue = isolateBindingContext[scopeName]);
parentSet(scope, parentValue = isolateScope[scopeName]);
}
}
return lastValue = parentValue;
Expand All @@ -1613,7 +1566,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

case '&':
parentGet = $parse(attrs[attrName]);
isolateBindingContext[scopeName] = function(locals) {
isolateScope[scopeName] = function(locals) {
return parentGet(scope, locals);
};
break;
Expand All @@ -1626,11 +1579,37 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
});
}
if (controllers) {
forEach(controllers, function(controller) {
controller();
transcludeFn = boundTranscludeFn && controllersBoundTransclude;
if (controllerDirectives) {
elementControllers = {};
forEach(controllerDirectives, function(directive) {
var locals = {
$scope: directive === newIsolateScopeDirective || directive.$$isolateScope ? isolateScope : scope,
$element: $element,
$attrs: attrs,
$transclude: transcludeFn
}, controllerInstance;

controller = directive.controller;
if (controller == '@') {
controller = attrs[directive.name];
}

controllerInstance = $controller(controller, locals);
// For directives with element transclusion the element is a comment,
// but jQuery .data doesn't support attaching data to comment nodes as it's hard to
// clean up (http://bugs.jquery.com/ticket/8335).
// Instead, we save the controllers for the element in a local hash and attach to .data
// later, once we have the actual element.
elementControllers[directive.name] = controllerInstance;
if (!hasElementTranscludeDirective) {
$element.data('$' + directive.name + 'Controller', controllerInstance);
}

if (directive.controllerAs) {
locals.$scope[directive.controllerAs] = controllerInstance;
}
});
controllers = null;
}

// PRELINKING
Expand Down
61 changes: 9 additions & 52 deletions src/ng/controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,13 @@ function $ControllerProvider() {
* It's just a simple call to {@link auto.$injector $injector}, but extracted into
* a service, so that one can override this service with [BC version](https://gist.github.com/1649788).
*/
return function(expression, locals, later, ident) {
// PRIVATE API:
// param `later` --- indicates that the controller's constructor is invoked at a later time.
// If true, $controller will allocate the object with the correct
// prototype chain, but will not invoke the controller until a returned
// callback is invoked.
// param `ident` --- An optional label which overrides the label parsed from the controller
// expression, if any.
return function(expression, locals) {
var instance, match, constructor, identifier;
later = later === true;
if (ident && isString(ident)) {
identifier = ident;
}

if(isString(expression)) {
match = expression.match(CNTRL_REG),
constructor = match[1],
identifier = identifier || match[3];
identifier = match[3];
expression = controllers.hasOwnProperty(constructor)
? controllers[constructor]
: getter(locals.$scope, constructor, true) ||
Expand All @@ -94,51 +83,19 @@ function $ControllerProvider() {
assertArgFn(expression, constructor, true);
}

if (later) {
// Instantiate controller later:
// This machinery is used to create an instance of the object before calling the
// controller's constructor itself.
//
// This allows properties to be added to the controller before the constructor is
// invoked. Primarily, this is used for isolate scope bindings in $compile.
//
// This feature is not intended for use by applications, and is thus not documented
// publicly.
var Constructor = function() {};
Constructor.prototype = (isArray(expression) ?
expression[expression.length - 1] : expression).prototype;
instance = new Constructor();

if (identifier) {
addIdentifier(locals, identifier, instance, constructor || expression.name);
}

return extend(function() {
$injector.invoke(expression, instance, locals, constructor);
return instance;
}, {
instance: instance,
identifier: identifier
});
}

instance = $injector.instantiate(expression, locals, constructor);

if (identifier) {
addIdentifier(locals, identifier, instance, constructor || expression.name);
if (!(locals && typeof locals.$scope === 'object')) {
throw minErr('$controller')('noscp',
"Cannot export controller '{0}' as '{1}'! No $scope object provided via `locals`.",
constructor || expression.name, identifier);
}

locals.$scope[identifier] = instance;
}

return instance;
};

function addIdentifier(locals, identifier, instance, name) {
if (!(locals && isObject(locals.$scope))) {
throw minErr('$controller')('noscp',
"Cannot export controller '{0}' as '{1}'! No $scope object provided via `locals`.",
name, identifier);
}

locals.$scope[identifier] = instance;
}
}];
}
78 changes: 0 additions & 78 deletions test/ng/compileSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3308,84 +3308,6 @@ describe('$compile', function() {
expect(componentScope.$$isolateBindings.exprAlias).toBe('&expr');

}));


it('should expose isolate scope variables on controller with controllerAs when bindToController is true', function() {
var controllerCalled = false;
module(function($compileProvider) {
$compileProvider.directive('fooDir', valueFn({
template: '<p>isolate</p>',
scope: {
'data': '=dirData',
'str': '@dirStr',
'fn': '&dirFn'
},
controller: function($scope) {
expect(this.data).toEqualData({
'foo': 'bar',
'baz': 'biz'
});
expect(this.str).toBe('Hello, world!');
expect(this.fn()).toBe('called!');
controllerCalled = true;
},
controllerAs: 'test',
bindToController: true
}));
});
inject(function($compile, $rootScope) {
$rootScope.fn = valueFn('called!');
$rootScope.whom = 'world';
$rootScope.remoteData = {
'foo': 'bar',
'baz': 'biz'
};
element = $compile('<div foo-dir dir-data="remoteData" ' +
'dir-str="Hello, {{whom}}!" ' +
'dir-fn="fn()"></div>')($rootScope);
expect(controllerCalled).toBe(true);
});
});


it('should expose isolate scope variables on controller with controllerAs when bindToController is true', function() {
var controllerCalled = false;
module(function($compileProvider) {
$compileProvider.directive('fooDir', valueFn({
templateUrl: 'test.html',
scope: {
'data': '=dirData',
'str': '@dirStr',
'fn': '&dirFn'
},
controller: function($scope) {
expect(this.data).toEqualData({
'foo': 'bar',
'baz': 'biz'
});
expect(this.str).toBe('Hello, world!');
expect(this.fn()).toBe('called!');
controllerCalled = true;
},
controllerAs: 'test',
bindToController: true
}));
});
inject(function($compile, $rootScope, $templateCache) {
$templateCache.put('test.html', '<p>isolate</p>');
$rootScope.fn = valueFn('called!');
$rootScope.whom = 'world';
$rootScope.remoteData = {
'foo': 'bar',
'baz': 'biz'
};
element = $compile('<div foo-dir dir-data="remoteData" ' +
'dir-str="Hello, {{whom}}!" ' +
'dir-fn="fn()"></div>')($rootScope);
$rootScope.$digest();
expect(controllerCalled).toBe(true);
});
});
});


Expand Down

0 comments on commit 252e8b5

Please sign in to comment.