From 437f9ace1de8a3ce715f41ac0b9784288a1e8e2c Mon Sep 17 00:00:00 2001 From: David Rodenas Pico Date: Sun, 10 Jan 2016 20:08:23 +0100 Subject: [PATCH 1/5] feat(ngMock.$componentController): use $injector instead of adding new code to angular.min.js #13683 --- src/ng/compile.js | 5 +---- src/ngMock/angular-mocks.js | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 7a6121f6e98f..1049aa6c8c53 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -928,7 +928,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return this; }; - this.$$componentControllers = createMap(); /** * @ngdoc method * @name $compileProvider#component @@ -1054,8 +1053,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { */ this.component = function registerComponent(name, options) { var controller = options.controller || function() {}; - var ident = identifierForController(options.controller) || options.controllerAs || '$ctrl'; - this.$$componentControllers[name] = { controller: controller, ident: ident}; function factory($injector) { function makeInjectable(fn) { @@ -1071,7 +1068,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { var template = (!options.template && !options.templateUrl ? '' : options.template); return { controller: controller, - controllerAs: ident, + controllerAs: identifierForController(options.controller) || options.controllerAs || '$ctrl', template: makeInjectable(template), templateUrl: makeInjectable(options.templateUrl), transclude: options.transclude, diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 6a391832c638..a0a787770990 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -2183,10 +2183,10 @@ angular.mock.$ControllerDecorator = ['$delegate', function($delegate) { */ angular.mock.$ComponentControllerProvider = ['$compileProvider', function($compileProvider) { return { - $get: ['$controller', function($controller) { + $get: ['$controller','$injector', function($controller,$injector) { return function $componentController(componentName, locals, bindings, ident) { - var controllerInfo = $compileProvider.$$componentControllers[componentName]; - return $controller(controllerInfo.controller, locals, bindings, ident || controllerInfo.ident); + var directiveInfo = $injector.get(componentName + 'Directive')[0]; + return $controller(directiveInfo.controller, locals, bindings, ident || directiveInfo.controllerAs); }; }] }; From 44a5e902a089d249ca4858c58fbdba2b0bcd5d99 Mon Sep 17 00:00:00 2001 From: David Rodenas Pico Date: Mon, 11 Jan 2016 22:45:03 +0100 Subject: [PATCH 2/5] feat(ngMock.$componentController): handle multiple directives with the same name #13683 --- src/ngMock/angular-mocks.js | 17 ++++- test/ngMock/angular-mocksSpec.js | 110 +++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index a0a787770990..500aa9ac8782 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -2185,7 +2185,22 @@ angular.mock.$ComponentControllerProvider = ['$compileProvider', function($compi return { $get: ['$controller','$injector', function($controller,$injector) { return function $componentController(componentName, locals, bindings, ident) { - var directiveInfo = $injector.get(componentName + 'Directive')[0]; + // get all directives associated to the component name + var directives = $injector.get(componentName + 'Directive'); + // look for those directives that are components + var candidateDirectives = directives.filter(function(directiveInfo) { + // components have controller, controllerAs and restrict:'E' compatible + return directiveInfo.controller && directiveInfo.controllerAs && directiveInfo.restrict.indexOf('E') >= 0; + }); + // check if valid directives found + if (candidateDirectives.length === 0) { + throw new Error('No component found'); + } + if (candidateDirectives.length > 1) { + throw new Error('Too many components found'); + } + // get the info of the component + var directiveInfo = candidateDirectives[0]; return $controller(directiveInfo.controller, locals, bindings, ident || directiveInfo.controllerAs); }; }] diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index b7d5fa9441b9..30b096a35b69 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -1938,6 +1938,116 @@ describe('ngMock', function() { expect($scope.testCtrl).toBe(ctrl); }); }); + + it('should instantiate the controller of the restrict:\'E\' component if there are more directives with the same name but not \'E\' restrictness', function() { + function TestController() { + this.r = 6779; + } + module(function($compileProvider) { + $compileProvider.directive('test', function() { + return { restrict: 'A' }; + }); + $compileProvider.component('test', { + controller: TestController + }); + }); + inject(function($componentController, $rootScope) { + var ctrl = $componentController('test', { $scope: {} }); + expect(ctrl).toEqual({ r: 6779 }); + }); + }); + + it('should instantiate the controller of the restrict:\'E\' component if there are more directives with the same name and \'E\' restrictness but no controller', function() { + function TestController() { + this.r = 22926; + } + module(function($compileProvider) { + $compileProvider.directive('test', function() { + return { restrict: 'E' }; + }); + $compileProvider.component('test', { + controller: TestController + }); + }); + inject(function($componentController, $rootScope) { + var ctrl = $componentController('test', { $scope: {} }); + expect(ctrl).toEqual({ r: 22926 }); + }); + }); + + it('should instantiate the controller of the directive with controller if there are more directives', function() { + function TestController() { + this.r = 18842; + } + module(function($compileProvider) { + $compileProvider.directive('test', function() { + return { }; + }); + $compileProvider.directive('test', function() { + return { + controller: TestController, + controllerAs: '$ctrl' + }; + }); + }); + inject(function($componentController, $rootScope) { + var ctrl = $componentController('test', { $scope: {} }); + expect(ctrl).toEqual({ r: 18842 }); + }); + }); + + it('should fail if there is no directive with restrict:\'E\' compatible and controller', function() { + function TestController() { + this.r = 31145; + } + module(function($compileProvider) { + $compileProvider.directive('test', function() { + return { + restrict: 'AC', + controller: TestController + }; + }); + $compileProvider.directive('test', function() { + return { + restrict: 'E', + controller: TestController + }; + }); + $compileProvider.directive('test', function() { + return { restrict: 'E' }; + }); + }); + inject(function($componentController, $rootScope) { + expect(function() { + $componentController('test', { $scope: {} }); + }).toThrow('No component found'); + }); + }); + + it('should fail if there more than two components with same name', function() { + function TestController($scope, a, b) { + this.$scope = $scope; + this.a = a; + this.b = b; + } + module(function($compileProvider) { + $compileProvider.directive('test', function() { + return { + controller: TestController, + controllerAs: '$ctrl' + }; + }); + $compileProvider.component('test', { + controller: TestController + }); + }); + inject(function($componentController, $rootScope) { + expect(function() { + var $scope = {}; + $componentController('test', { $scope: $scope, a: 'A', b: 'B' }, { x: 'X', y: 'Y' }); + }).toThrow('Too many components found'); + }); + }); }); }); From 4c0bc80ea301d602051864efc3700f6c916de14e Mon Sep 17 00:00:00 2001 From: David Rodenas Pico Date: Tue, 12 Jan 2016 06:37:23 +0100 Subject: [PATCH 3/5] feat(ngMock.$componentController): fix jscs extra space #13683 --- test/ngMock/angular-mocksSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index 30b096a35b69..82203821d4c4 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -1984,7 +1984,7 @@ describe('ngMock', function() { return { }; }); $compileProvider.directive('test', function() { - return { + return { controller: TestController, controllerAs: '$ctrl' }; From fa6f5afad75c8ebf4cc96ad680562d8d9888e208 Mon Sep 17 00:00:00 2001 From: David Rodenas Pico Date: Tue, 12 Jan 2016 07:10:35 +0100 Subject: [PATCH 4/5] refactor(ngMock.$componentController): expect components to be restricted to 'E' #13683 --- src/ngMock/angular-mocks.js | 4 ++-- test/ngMock/angular-mocksSpec.js | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 500aa9ac8782..f0792f044c07 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -2189,8 +2189,8 @@ angular.mock.$ComponentControllerProvider = ['$compileProvider', function($compi var directives = $injector.get(componentName + 'Directive'); // look for those directives that are components var candidateDirectives = directives.filter(function(directiveInfo) { - // components have controller, controllerAs and restrict:'E' compatible - return directiveInfo.controller && directiveInfo.controllerAs && directiveInfo.restrict.indexOf('E') >= 0; + // components have controller, controllerAs and restrict:'E' + return directiveInfo.controller && directiveInfo.controllerAs && directiveInfo.restrict === 'E'; }); // check if valid directives found if (candidateDirectives.length === 0) { diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index 82203821d4c4..281b0763abea 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -1939,7 +1939,7 @@ describe('ngMock', function() { }); }); - it('should instantiate the controller of the restrict:\'E\' component if there are more directives with the same name but not \'E\' restrictness', function() { + it('should instantiate the controller of the restrict:\'E\' component if there are more directives with the same name but not restricted to \'E\'', function() { function TestController() { this.r = 6779; } @@ -1957,7 +1957,7 @@ describe('ngMock', function() { }); }); - it('should instantiate the controller of the restrict:\'E\' component if there are more directives with the same name and \'E\' restrictness but no controller', function() { + it('should instantiate the controller of the restrict:\'E\' component if there are more directives with the same name and restricted to \'E\' but no controller', function() { function TestController() { this.r = 22926; } @@ -1975,7 +1975,7 @@ describe('ngMock', function() { }); }); - it('should instantiate the controller of the directive with controller if there are more directives', function() { + it('should instantiate the controller of the directive with controller, controllerAs and restrict:\'E\' if there are more directives', function() { function TestController() { this.r = 18842; } @@ -1985,6 +1985,7 @@ describe('ngMock', function() { }); $compileProvider.directive('test', function() { return { + restrict: 'E', controller: TestController, controllerAs: '$ctrl' }; @@ -1996,7 +1997,7 @@ describe('ngMock', function() { }); }); - it('should fail if there is no directive with restrict:\'E\' compatible and controller', function() { + it('should fail if there is no directive with restrict:\'E\' and controller', function() { function TestController() { this.r = 31145; } @@ -2013,6 +2014,13 @@ describe('ngMock', function() { controller: TestController }; }); + $compileProvider.directive('test', function() { + return { + restrict: 'EA', + controller: TestController, + controllerAs: '$ctrl' + }; + }); $compileProvider.directive('test', function() { return { restrict: 'E' }; }); @@ -2033,6 +2041,7 @@ describe('ngMock', function() { module(function($compileProvider) { $compileProvider.directive('test', function() { return { + restrict: 'E', controller: TestController, controllerAs: '$ctrl' }; From 4aa6002f69d1dd48a9dcc220eff429b811ff6001 Mon Sep 17 00:00:00 2001 From: David Rodenas Pico Date: Tue, 12 Jan 2016 09:07:39 +0100 Subject: [PATCH 5/5] refactor(ngMock.$componentController): add minErr to throw statements (fail due to TypeError: Cannot read property 'nocomp' of undefined) at /Users/drodenas/Projects/drpicox/angular.js/docs/config/processors/error-docs.js:43:76) --- .../error/$componentController/multicomp.ngdoc | 17 +++++++++++++++++ .../error/$componentController/nocomp.ngdoc | 11 +++++++++++ src/ngMock/angular-mocks.js | 5 +++-- test/ngMock/angular-mocksSpec.js | 4 ++-- 4 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 docs/content/error/$componentController/multicomp.ngdoc create mode 100644 docs/content/error/$componentController/nocomp.ngdoc diff --git a/docs/content/error/$componentController/multicomp.ngdoc b/docs/content/error/$componentController/multicomp.ngdoc new file mode 100644 index 000000000000..ad69e528c333 --- /dev/null +++ b/docs/content/error/$componentController/multicomp.ngdoc @@ -0,0 +1,17 @@ +@ngdoc error +@name $componentController:multicomp +@fullName Multiple components defined +@description + +This error occurs when multiple components or component-like directives +are defined with the same name, and processing them would result in a +collision or an unsupported configuration. + + +To resolve this issue remove one of the directives or controllers which is causing the collision. + +Component-like directives are those that: + +* Have a controller +* Have defined controllerAs +* Have restrict to 'E' diff --git a/docs/content/error/$componentController/nocomp.ngdoc b/docs/content/error/$componentController/nocomp.ngdoc new file mode 100644 index 000000000000..46ac5dd81274 --- /dev/null +++ b/docs/content/error/$componentController/nocomp.ngdoc @@ -0,0 +1,11 @@ +@ngdoc error +@name $componentController:nocomp +@fullName No component available +@description + +This error occurs when it is requested a component that it is not defined. + + +To resolve this issue create the component which the requested name or change +the name to match an existing component name. + diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index f0792f044c07..b26a79bab6a4 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -2182,6 +2182,7 @@ angular.mock.$ControllerDecorator = ['$delegate', function($delegate) { * @return {Object} Instance of requested controller. */ angular.mock.$ComponentControllerProvider = ['$compileProvider', function($compileProvider) { + var minErr = angular.$$minErr('$componentController'); return { $get: ['$controller','$injector', function($controller,$injector) { return function $componentController(componentName, locals, bindings, ident) { @@ -2194,10 +2195,10 @@ angular.mock.$ComponentControllerProvider = ['$compileProvider', function($compi }); // check if valid directives found if (candidateDirectives.length === 0) { - throw new Error('No component found'); + throw minErr('nocomp','No \'{0}\' component found', componentName); } if (candidateDirectives.length > 1) { - throw new Error('Too many components found'); + throw minErr('multicomp','Found {0} directive candidates for component \'{1}\'', candidateDirectives.length, componentName); } // get the info of the component var directiveInfo = candidateDirectives[0]; diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index 281b0763abea..60c6651715ef 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -2028,7 +2028,7 @@ describe('ngMock', function() { inject(function($componentController, $rootScope) { expect(function() { $componentController('test', { $scope: {} }); - }).toThrow('No component found'); + }).toThrowMinErr('$componentController','nocomp', "No 'test' component found"); }); }); @@ -2054,7 +2054,7 @@ describe('ngMock', function() { expect(function() { var $scope = {}; $componentController('test', { $scope: $scope, a: 'A', b: 'B' }, { x: 'X', y: 'Y' }); - }).toThrow('Too many components found'); + }).toThrowMinErr('$componentController','multicomp',"Found 2 directive candidates for component 'test'"); }); }); });