From bd3849cf38fd8d317006df6d2ed8e743496b9664 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Thu, 21 Aug 2014 18:34:02 -0400 Subject: [PATCH] feat($compile): isolate scope properties in controller context via controllerAs It is now possible to ask the $compiler's isolate scope property machinery to reference properties by their isolate scope. The current syntax is to prefix the scope name with a '@', like so: scope: { "myData": "=someData", "myString": "@someInterpolation", "myExpr": "&someExpr" }, controllerAs: "someCtrl", bindtoController: true The putting of properties within the context of the controller will only occur if controllerAs is used for an isolate scope with the `bindToController` property of the directive definition object set to `true`. Closes #7635 Closes #7645 --- src/ng/compile.js | 107 ++++++++++++++++++++++++----------------- src/ng/controller.js | 61 +++++++++++++++++++---- test/ng/compileSpec.js | 78 ++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 52 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 950c036ecd4d..79c29df72631 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -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 @@ -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); } } @@ -1221,6 +1225,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { var terminalPriority = -Number.MAX_VALUE, newScopeDirective, controllerDirectives = previousCompileContext.controllerDirectives, + controllers, newIsolateScopeDirective = previousCompileContext.newIsolateScopeDirective, templateDirective = previousCompileContext.templateDirective, nonTlbTranscludeDirective = previousCompileContext.nonTlbTranscludeDirective, @@ -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'); @@ -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)) { @@ -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, @@ -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; @@ -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; @@ -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; @@ -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 diff --git a/src/ng/controller.js b/src/ng/controller.js index 0e215bdc8e77..a8f701adc636 100644 --- a/src/ng/controller.js +++ b/src/ng/controller.js @@ -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) { + // 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) || @@ -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; + } }]; } diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2ebe15c013ca..2968a97f5832 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -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: '

isolate

', + 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('
')($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', '

isolate

'); + $rootScope.fn = valueFn('called!'); + $rootScope.whom = 'world'; + $rootScope.remoteData = { + 'foo': 'bar', + 'baz': 'biz' + }; + element = $compile('
')($rootScope); + $rootScope.$digest(); + expect(controllerCalled).toBe(true); + }); + }); });