REQUEST FOR FEEDBACK: angular.component() - auto creating a controller

[shairez]

As discussed with @shahata and @petebacondarwin -

The problem I run into when writing lots of "component like" directives is the unnecessarily repeated configuration over and over again.

Registering controllers

It's redundant to have a separate controller declaration.

Registering controllers by name in a component based architecture is useful only for unit tests, where you'd want to use $controller to create an instance of the component's controller.

But registering it over and over again is redundant and can be done by the .component.

We should have one .comp.js file

If we use .component(), it should auto create a new controller, registered with the same component name (because it too must be unique).

So we should end up with one file where the .component() becomes a true "annotation like" thing, something like:

angular
 .module('myModule')
 .component('productList', {
   controller: ProductListController,
   templateUrl: 'product-list.comp.html'
  })

function ProductListController(){

}

$controller('productList') // <-- should be an instance of ProductListController

I'd love to hear the cons for this approach

What do you think?

[petebacondarwin]

So on an abstract level, what you are saying is that the controller provided to a component registration should be made available (primarily for testing purposes?) via some Angular API?

More concretely your suggestion is that the component() helper should also register the controller via the controller() helper mechanism so that it can be accessed like any other controller via the $controller service?

And further that we should use the name of the component as the name of the controller? For this particular suggestion, I would suggest that we postfix the name with something to help mitigate potential collisions with other controllers. Perhaps postfix Controller or even ComonentController on the component name.

[shahata]

In an app which uses es6 modules you would simply do:

//------ product-list.js ------
angular
 .module('myModule')
 .component('productList', {
   controller: ProductListController,
   templateUrl: 'product-list.comp.html'
  });

export function ProductListController() {}

//------ product-list.spec.js ------
import {ProductListController} from 'product-list';

$controller(ProductListController);

I like it that we can give a similar mechanism to people on es5.
+1 for Controller postfix.

[gkalpak]

Tl;dr: Sounds reasonable to me.

Generally, it's a good practice (imo) to register a directive controller by name (i.e. not as a constructor function directly), for testing purposes and decoupling. But since the .component() helper is just a...helper and one fo its purposes is to reduce boilerplate, it sounds reasonable to support/promote a good practice (registering a controller separately) without extra boilerplate overhead.

So, it's a 👍 from me on the idea.

Implementation-wise, I would:

• Use PascalCase (instead of the camelCased component's name).
• Suffix with Controller. (I actually prefer Ctrl, but Controller it a more generally accepted best practice, I think.)
• I would provide a way to opt-out (just in case). E.g. having registerController: false on the CDO, would skip that step.

[lgalfaso]

This sounds reasonable, but would it be possible to know the specific use case that this solves? BTW, I agree with the comment from @petebacondarwin that the controller name should have some suffix, even when I think that Controller is not a good option. Keep in mind that controllers are not registered as services, so there is no need to add a suffix to avoid name collisions

[shahata]

I'm just wondering if this is not too magical though. We could just recommend people to also register their controller with module.controller if they need to unit test it and then it is very clear to the developer what is actually happening.

Example:

//------ product-list.js ------
angular
 .module('myModule')
 .controller('ProductListController', ProductListController) // <-- register also as controller
 .component('productList', {
   controller: ProductListController,
   templateUrl: 'product-list.comp.html'
  });

function ProductListController() {}

//------ product-list.spec.js ------
$controller('ProductListController');

[shahata]

@lgalfaso the use case is that people sometimes want to write unit tests for their controller without actually going through compiling the component.

[petebacondarwin]

@lgalfaso - there could be name collisions with other controllers that have been registered explicitly via the controller() helper.

[petebacondarwin]

@shahata wrote:

We could just recommend people to also register their controller with module.controller if they need to unit test it and then it is very clear to the developer what is actually happening.

When would a developer not want to unit test their component controller? :-)

[petebacondarwin]

I like the opt-out idea. Although, perhaps we could be more clever and provide a way of overriding the name of the controller....

Explicit controller registration name

angular
 .module('myModule')
 .component('productList', {
   controller: ProductListController,
   controllerName: 'MyProductListController',
   templateUrl: 'product-list.comp.html'
  })

function ProductListController(){ ... }

// ...

var controller = $controller('MyProductListController');

Opt-out Controller registration

angular
 .module('myModule')
 .component('productList', {
   controller: ProductListController,
   controllerName: null,
   templateUrl: 'product-list.comp.html'
  })

function ProductListController(){ ... }

[shahata]

@petebacondarwin from my experience it is often better to test the component as a whole and not access the controller directly. A lot of times I'm thinking of a controller as private methods of the component. Any logic that needs to be unit tested inside controller I often move to services or domain objects.

[Narretz]

I like the opt-out option. Maybe with a different name though ( bike-shedding, I know), because controllerName is to similar to controllerAs. Maybe registerController, which defaults to true but can also be a string or false.

[shahata]

I actually convinced myself against this. I prefer people will explicitly register the controller themselves.

[lgalfaso]

Sorry for stressing this, but I do not find it clear that registering the controller is helpful for testing. The reason is that when a controller is used within a directive, properties are bound to the controller before the controllers constructor is called (and this is something that the public API does not expose). I am sure that there are other use cases for exposing the controller, just would like to be sure that the reason is not to be able to tell developers that they can test a directives controller outside of a directive.

[petebacondarwin]

@lgalfaso - actually there is a version of $controller in ngMock that allows you to add in the bindings for testing. See https://docs.angularjs.org/api/ngMock/service/$controller

[shairez]

+1 for the Controller suffix
+1 for the PascalCase
+1 for the optout - controllerRegistryName might be long but descriptive
+1 for testing controllers of directives (yeah shahar, I disagree :) )

About testing controllers

@shahata I think what @petebacondarwin shared about ngMock (exposing the bindings) makes testing the controller a no brainer, because it saves you from compiling the directive in each test setup.

Testing controllers is much easier (IMO) than testing directives, that's why I think it would lower the entry bar for new developers. (At least that's my experience from teaching developers how to test).

About the magic

I don't think it's too much magic, because the developer is still the one who creates the controller.

I do think it's a change though that should be communicated well.

And as a general philosophy - let's reward the testers!

Angular as a framework, makes life much easier for unit testing, hence - it has dependency injection built in.

I think we should continue this method of promoting unit testing, and provide this "auto registering", I don't see a cost there but maybe I'm wrong.

I predict that as soon as people will start componentize their apps more and more, they will encounter this annoying boilerplate setup, especially if they test their code or TDD their apps. I say let's not punish the testers, but reward them instead.

So in summary

I don't see adding this feature for testing purposes (even if the only use case) as a bad thing.

What do you guys think?

[drpicox]

-1

Explicit better that implicit.

The proposal is to make available a controller that it is not registered separately just in purpose to do testing. But, doing that we are allowing programmers to use controllers without the directive in any place (I'm afraid of lot of bad code emerging from juniors).

If you need the controller of any directive there are many mechanism to do so:

• for example as controllerAs: '$ctrl', is as simple to create the directive, access the $scope and get the controller
• if we have not controllerAs we can use element.controller

In both cases you can have a helper to save some writing time.

But doing that testing times can be a little bit slower (instance the whole directive), so
I have an alternative proposal:

• a helper function that looks into the directives definition structures, looks for the directive which controller we want to test, and instances that, some like:

/* given ... */
app.component('myFoo', {
   controller: FooController,
   bindings: { someBindings... },
   ...
});
...

/* we may test with something like: */
fooControllerInstance = $componentController('myFoo', locals, { myBindings... });

Good points:

• no polluting controllers space
• do not allow to use them in ngController directives
• do not allow to use them in other creative solutions
• do not increase extra bytes the angular.min.js (it can be defined in angular-mocks)

[shairez]

@drpicox OK, sounds good

I'm cool with everything that will shortcut my tests and get me the controller instance I need.

$componentController() also works for me as it achieves the same goal.

[petebacondarwin]

I have put together a PR with a $componentController service as suggested. I quite like the idea for the reasons given by @drpicox

[orizens]

hi.
If using es6/es2015, a few days ago I suggested the following defintion for component:

<script src="https://gist.github.com/orizens/22dbab8aacb6c3a173b3.js"></script>

https://gist.github.com/orizens/22dbab8aacb6c3a173b3
the inspiration for it came from angular2 component definition and from personal experience. I've found it more comfortable to have all the component's/directive definition in one file other than splitting it to several files.
Moreover, simliar to @shahata approach, the component controller (defined either as a class or a function), is accessible via importing the component object definition, and can be tested as expected.

[shairez]

@orizens Yes, I do the same with TypeScript and Angular 1 for testing controllers.
Also for using one file, as written in the description :)

In ES6 we don't have this issue, so I suggested this addition to only cater ES5 projects.

And thanks for the PR @petebacondarwin, looks awesome!