diff --git a/docs/content/error/$compile/noctrl.ngdoc b/docs/content/error/$compile/noctrl.ngdoc new file mode 100644 index 000000000000..ce35974fb96a --- /dev/null +++ b/docs/content/error/$compile/noctrl.ngdoc @@ -0,0 +1,68 @@ +@ngdoc error +@name $compile:noctrl +@fullName Controller is required. +@description + +When using the `bindToController` feature of AngularJS, a directive is required +to have a Controller, in addition to a controller identifier. + +For example, the following directives are valid: + +```js +// OKAY, because controller is a string with a label component. +directive("okay", function() { + return { + bindToController: true, + controller: "myCtrl as $ctrl" + scope: { + text: "@text" + } + }; +}); + + +// OKAY, because the directive uses the controllerAs property to override +// the controller identifier. +directive("okay2", function() { + return { + bindToController: true, + controllerAs: "$ctrl", + controller: function() { + + }, + scope: { + text: "@text" + } + }; +}); +``` + +While the following are invalid: + +```js +// BAD, because the controller property is a string with no identifier. +directive("bad", function() { + return { + bindToController: true, + controller: "unlabeledCtrl", + scope: { + text: "@text" + } + }; +}); + + +// BAD because the controller is not a string (therefore has no identifier), +// and there is no controllerAs property. +directive("bad2", function() { + return { + bindToController: true, + controller: function noControllerAs() { + + }, + scope: { + text: "@text" + } + }; +}); +``` diff --git a/src/.jshintrc b/src/.jshintrc index 16db88d8501b..d0561483c9ad 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -152,6 +152,9 @@ "urlResolve": false, "urlIsSameOrigin": false, + /* ng/controller.js */ + "identifierForController": false, + /* ng/compile.js */ "directiveNormalize": false, diff --git a/src/ng/compile.js b/src/ng/compile.js index bef8873816d1..f1a355f624ce 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -712,7 +712,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // 'on' and be composed of only English letters. var EVENT_HANDLER_ATTR_REGEXP = /^(on[a-z]+|formaction)$/; - function parseIsolateBindings(scope, directiveName) { + function parseIsolateBindings(scope, directiveName, isController) { var LOCAL_REGEXP = /^\s*([@&]|=(\*?))(\??)\s*(\w*)\s*$/; var bindings = {}; @@ -722,9 +722,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (!match) { throw $compileMinErr('iscp', - "Invalid isolate scope definition for directive '{0}'." + + "Invalid {3} for directive '{0}'." + " Definition: {... {1}: '{2}' ...}", - directiveName, scopeName, definition); + directiveName, scopeName, definition, + (isController ? "controller bindings definition" : + "isolate scope definition")); } bindings[scopeName] = { @@ -738,6 +740,37 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return bindings; } + function parseDirectiveBindings(directive, directiveName) { + var bindings = { + isolateScope: null, + bindToController: null + }; + if (isObject(directive.scope)) { + if (directive.bindToController === true) { + bindings.bindToController = parseIsolateBindings(directive.scope, + directiveName, true); + bindings.isolateScope = {}; + } else { + bindings.isolateScope = parseIsolateBindings(directive.scope, + directiveName, false); + } + } + if (isObject(directive.bindToController)) { + bindings.bindToController = + parseIsolateBindings(directive.bindToController, directiveName, true); + } + if (isObject(bindings.bindToController)) { + var controller = directive.controller; + var controllerAs = directive.controllerAs; + if (!directive.controller || !identifierForController(controller, controllerAs)) { + throw $compileMinErr('noctrl', + "Cannot bind to controller without directive '{0}'s controller.", + directiveName); + } + } + return bindings; + } + /** * @ngdoc method * @name $compileProvider#directive @@ -775,8 +808,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { directive.name = directive.name || name; directive.require = directive.require || (directive.controller && directive.name); directive.restrict = directive.restrict || 'EA'; - if (isObject(directive.scope)) { - directive.$$isolateBindings = parseIsolateBindings(directive.scope, directive.name); + var bindings = directive.$$bindings = + parseDirectiveBindings(directive, directive.name); + if (isObject(bindings.isolateScope)) { + directive.$$isolateBindings = bindings.isolateScope; } directives.push(directive); } catch (e) { @@ -1338,6 +1373,16 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (nodeLinkFn.scope) { childScope = scope.$new(); compile.$$addScopeInfo(jqLite(node), childScope); + var onDestroyed = nodeLinkFn.$$onScopeDestroyed; + if (onDestroyed) { + nodeLinkFn.$$onScopeDestroyed = null; + childScope.$on('$destroyed', function() { + for (var i=0, ii = onDestroyed.length; i < ii; ++i) { + onDestroyed[i](); + } + onDestroyed = null; + }); + } } else { childScope = scope; } @@ -1357,7 +1402,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { childBoundTranscludeFn = null; } - nodeLinkFn(childLinkFn, childScope, node, $rootElement, childBoundTranscludeFn); + nodeLinkFn(childLinkFn, childScope, node, $rootElement, childBoundTranscludeFn, + nodeLinkFn); } else if (childLinkFn) { childLinkFn(scope, node.childNodes, undefined, parentBoundTranscludeFn); @@ -1838,7 +1884,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } - function nodeLinkFn(childLinkFn, scope, linkNode, $rootElement, boundTranscludeFn) { + function nodeLinkFn(childLinkFn, scope, linkNode, $rootElement, boundTranscludeFn, + thisLinkFn) { var i, ii, linkFn, controller, isolateScope, elementControllers, transcludeFn, $element, attrs; @@ -1895,89 +1942,29 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } if (newIsolateScopeDirective) { + // Initialize isolate scope bindings for new isolate scope directive. compile.$$addScopeInfo($element, isolateScope, true, !(templateDirective && (templateDirective === newIsolateScopeDirective || templateDirective === newIsolateScopeDirective.$$originalDirective))); compile.$$addScopeClass($element, true); - - var isolateScopeController = controllers && controllers[newIsolateScopeDirective.name]; - var isolateBindingContext = isolateScope; - if (isolateScopeController && isolateScopeController.identifier && - newIsolateScopeDirective.bindToController === true) { - isolateBindingContext = isolateScopeController.instance; - } - - forEach(isolateScope.$$isolateBindings = newIsolateScopeDirective.$$isolateBindings, function(definition, scopeName) { - var attrName = definition.attrName, - optional = definition.optional, - mode = definition.mode, // @, =, or & - lastValue, - parentGet, parentSet, compare; - - switch (mode) { - - case '@': - attrs.$observe(attrName, function(value) { - isolateBindingContext[scopeName] = value; - }); - attrs.$$observers[attrName].$$scope = scope; - 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); - } - break; - - case '=': - if (optional && !attrs[attrName]) { - return; - } - parentGet = $parse(attrs[attrName]); - if (parentGet.literal) { - compare = equals; - } else { - compare = function(a, b) { return a === b || (a !== a && b !== b); }; - } - parentSet = parentGet.assign || function() { - // reset the change, or we will throw this exception on every $digest - lastValue = isolateBindingContext[scopeName] = parentGet(scope); - throw $compileMinErr('nonassign', - "Expression '{0}' used with directive '{1}' is non-assignable!", - attrs[attrName], newIsolateScopeDirective.name); - }; - lastValue = isolateBindingContext[scopeName] = parentGet(scope); - var parentValueWatch = function parentValueWatch(parentValue) { - if (!compare(parentValue, isolateBindingContext[scopeName])) { - // we are out of sync and need to copy - if (!compare(parentValue, lastValue)) { - // parent changed and it has precedence - isolateBindingContext[scopeName] = parentValue; - } else { - // if the parent can be assigned then do so - parentSet(scope, parentValue = isolateBindingContext[scopeName]); - } - } - return lastValue = parentValue; - }; - parentValueWatch.$stateful = true; - var unwatch; - if (definition.collection) { - unwatch = scope.$watchCollection(attrs[attrName], parentValueWatch); - } else { - unwatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal); - } - isolateScope.$on('$destroy', unwatch); - break; - - case '&': - parentGet = $parse(attrs[attrName]); - isolateBindingContext[scopeName] = function(locals) { - return parentGet(scope, locals); - }; - break; - } - }); + isolateScope.$$isolateBindings = + newIsolateScopeDirective.$$isolateBindings; + initializeDirectiveBindings(scope, attrs, isolateScope, + isolateScope.$$isolateBindings, + newIsolateScopeDirective, isolateScope); } if (controllers) { + // Initialize bindToController bindings for new/isolate scopes + var scopeDirective = newIsolateScopeDirective || newScopeDirective; + if (scopeDirective && controllers[scopeDirective.name]) { + var bindings = scopeDirective.$$bindings.bindToController; + controller = controllers[scopeDirective.name]; + + if (controller && controller.identifier && bindings) { + thisLinkFn.$$onScopeDestroyed = + initializeDirectiveBindings(scope, attrs, controller.instance, + bindings, scopeDirective); + } + } forEach(controllers, function(controller) { controller(); }); @@ -2240,7 +2227,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { childBoundTranscludeFn = boundTranscludeFn; } afterTemplateNodeLinkFn(afterTemplateChildLinkFn, scope, linkNode, $rootElement, - childBoundTranscludeFn); + childBoundTranscludeFn, afterTemplateNodeLinkFn); } linkQueue = null; }); @@ -2257,7 +2244,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (afterTemplateNodeLinkFn.transcludeOnThisElement) { childBoundTranscludeFn = createBoundTranscludeFn(scope, afterTemplateNodeLinkFn.transclude, boundTranscludeFn); } - afterTemplateNodeLinkFn(afterTemplateChildLinkFn, scope, node, rootElement, childBoundTranscludeFn); + afterTemplateNodeLinkFn(afterTemplateChildLinkFn, scope, node, rootElement, childBoundTranscludeFn, + afterTemplateNodeLinkFn); } }; } @@ -2504,6 +2492,90 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { $exceptionHandler(e, startingTag($element)); } } + + + // Set up $watches for isolate scope and controller bindings. This process + // only occurs for isolate scopes and new scopes with controllerAs. + function initializeDirectiveBindings(scope, attrs, destination, bindings, + directive, newScope) { + var onNewScopeDestroyed; + forEach(bindings, function(definition, scopeName) { + var attrName = definition.attrName, + optional = definition.optional, + mode = definition.mode, // @, =, or & + lastValue, + parentGet, parentSet, compare; + + switch (mode) { + + case '@': + attrs.$observe(attrName, function(value) { + destination[scopeName] = value; + }); + attrs.$$observers[attrName].$$scope = scope; + 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 + destination[scopeName] = $interpolate(attrs[attrName])(scope); + } + break; + + case '=': + if (optional && !attrs[attrName]) { + return; + } + parentGet = $parse(attrs[attrName]); + if (parentGet.literal) { + compare = equals; + } else { + compare = function(a, b) { return a === b || (a !== a && b !== b); }; + } + parentSet = parentGet.assign || function() { + // reset the change, or we will throw this exception on every $digest + lastValue = destination[scopeName] = parentGet(scope); + throw $compileMinErr('nonassign', + "Expression '{0}' used with directive '{1}' is non-assignable!", + attrs[attrName], directive.name); + }; + lastValue = destination[scopeName] = parentGet(scope); + var parentValueWatch = function parentValueWatch(parentValue) { + if (!compare(parentValue, destination[scopeName])) { + // we are out of sync and need to copy + if (!compare(parentValue, lastValue)) { + // parent changed and it has precedence + destination[scopeName] = parentValue; + } else { + // if the parent can be assigned then do so + parentSet(scope, parentValue = destination[scopeName]); + } + } + return lastValue = parentValue; + }; + parentValueWatch.$stateful = true; + var unwatch; + if (definition.collection) { + unwatch = scope.$watchCollection(attrs[attrName], parentValueWatch); + } else { + unwatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal); + } + if (newScope) { + newScope.$on('$destroy', unwatch); + } else { + onNewScopeDestroyed = (onNewScopeDestroyed || []); + onNewScopeDestroyed.push(unwatch); + } + break; + + case '&': + parentGet = $parse(attrs[attrName]); + destination[scopeName] = function(locals) { + return parentGet(scope, locals); + }; + break; + } + }); + return onNewScopeDestroyed; + } }]; } diff --git a/src/ng/controller.js b/src/ng/controller.js index f7b2e5bcfa19..4ef9c39d3ced 100644 --- a/src/ng/controller.js +++ b/src/ng/controller.js @@ -2,6 +2,17 @@ var $controllerMinErr = minErr('$controller'); + +var CNTRL_REG = /^(\S+)(\s+as\s+(\w+))?$/; +function identifierForController(controller, ident) { + if (ident && isString(ident)) return ident; + if (isString(controller)) { + var match = CNTRL_REG.exec(controller); + if (match) return match[3]; + } +} + + /** * @ngdoc provider * @name $controllerProvider @@ -14,9 +25,7 @@ var $controllerMinErr = minErr('$controller'); */ function $ControllerProvider() { var controllers = {}, - globals = false, - CNTRL_REG = /^(\S+)(\s+as\s+(\w+))?$/; - + globals = false; /** * @ngdoc method diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 5df9bddb79c5..aebd8ab6ebee 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3809,6 +3809,180 @@ describe('$compile', function() { expect(controllerCalled).toBe(true); }); }); + + + it('should throw noctrl when missing controller', function() { + module(function($compileProvider) { + $compileProvider.directive('noCtrl', valueFn({ + templateUrl: 'test.html', + scope: { + 'data': '=dirData', + 'str': '@dirStr', + 'fn': '&dirFn' + }, + controllerAs: 'test', + bindToController: true + })); + }); + inject(function($compile, $rootScope) { + expect(function() { + $compile('
isolate
'); + $rootScope.fn = valueFn('called!'); + $rootScope.whom = 'world'; + $rootScope.remoteData = { + 'foo': 'bar', + 'baz': 'biz' + }; + element = $compile('')($rootScope); + $rootScope.$digest(); + expect(controllerCalled).toBe(true); + }); + }); + + + it('should bind to controller via object notation (new scope)', function() { + var controllerCalled = false; + module(function($compileProvider, $controllerProvider) { + $controllerProvider.register('myCtrl', function() { + expect(this.data).toEqualData({ + 'foo': 'bar', + 'baz': 'biz' + }); + expect(this.str).toBe('Hello, world!'); + expect(this.fn()).toBe('called!'); + controllerCalled = true; + }); + $compileProvider.directive('fooDir', valueFn({ + templateUrl: 'test.html', + bindToController: { + 'data': '=dirData', + 'str': '@dirStr', + 'fn': '&dirFn' + }, + scope: true, + controller: 'myCtrl as myCtrl' + })); + }); + 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); + }); + }); + + + it('should put controller in scope when labelled but not using controllerAs', function() { + var controllerCalled = false; + var myCtrl; + module(function($compileProvider, $controllerProvider) { + $controllerProvider.register('myCtrl', function() { + controllerCalled = true; + myCtrl = this; + }); + $compileProvider.directive('fooDir', valueFn({ + templateUrl: 'test.html', + bindToController: {}, + scope: true, + controller: 'myCtrl as theCtrl' + })); + }); + inject(function($compile, $rootScope, $templateCache) { + $templateCache.put('test.html', 'isolate
'); + element = $compile('