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

feat($compile): isolate scope properties in controller context via controllerAs #7645

Closed
wants to merge 1 commit into from
Closed

feat($compile): isolate scope properties in controller context via controllerAs #7645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 64 additions & 43 deletions src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@
* 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 @@ -890,7 +894,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {

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

Expand Down Expand Up @@ -1221,6 +1225,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
var terminalPriority = -Number.MAX_VALUE,
newScopeDirective,
controllerDirectives = previousCompileContext.controllerDirectives,
controllers,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having a separate object for this is suspect... I think elementControllers could probably be used

newIsolateScopeDirective = previousCompileContext.newIsolateScopeDirective,
templateDirective = previousCompileContext.templateDirective,
nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective,
Expand Down Expand Up @@ -1458,7 +1463,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
value = null;

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

Expand Down Expand Up @@ -1491,9 +1498,45 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}

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

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*$/;

if (templateDirective && (templateDirective === newIsolateScopeDirective ||
templateDirective === newIsolateScopeDirective.$$originalDirective)) {
Expand All @@ -1502,10 +1545,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
$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 @@ -1526,7 +1573,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
isolateScope[scopeName] = $interpolate(attrs[attrName])(scope);
isolateBindingContext[scopeName] = $interpolate(attrs[attrName])(scope);
}
break;

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

case '&':
parentGet = $parse(attrs[attrName]);
isolateScope[scopeName] = function(locals) {
isolateBindingContext[scopeName] = function(locals) {
return parentGet(scope, locals);
};
break;
Expand All @@ -1579,37 +1626,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
});
}
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;
}
if (controllers) {
forEach(controllers, function(controller) {
controller();
});
controllers = null;
}

// PRELINKING
Expand Down
61 changes: 52 additions & 9 deletions src/ng/controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,24 @@ 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) {
return function(expression, locals, later, ident) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the extra parameters are undocumented as they're not intended for public use, but basically what they are:

  1. instantiate controller later (which the compiler needs to do)
  2. optional identifier to override existing identifier (comes from directive.controllerAs)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add this as private comment into the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean in addition to the big comment on lines 90-98?

// 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.
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 = match[3];
identifier = identifier || match[3];
expression = controllers.hasOwnProperty(constructor)
? controllers[constructor]
: getter(locals.$scope, constructor, true) ||
Expand All @@ -83,19 +94,51 @@ function $ControllerProvider() {
assertArgFn(expression, constructor, true);
}

instance = $injector.instantiate(expression, locals, constructor);
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) {
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);
if (identifier) {
addIdentifier(locals, identifier, instance, constructor || expression.name);
}

locals.$scope[identifier] = instance;
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);
}

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: 78 additions & 0 deletions test/ng/compileSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3308,6 +3308,84 @@ 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