Extract class controller declaration from component declaration

[aaronte]

Hey, currently the x.component.js is declared as following where the controller is declared inline with the component declaration:

/* ----- todo/todo-form/todo-form.component.js ----- */
import templateUrl from './todo-form.html';

export const TodoFormComponent = {
    bindings: {
        todo: '<',
        onAddTodo: '&'
    },
    templateUrl,
    controller: class TodoFormComponent {
        constructor(EventEmitter) {
            'ngInject';
            this.EventEmitter = EventEmitter;
        }

        $onChanges(changes) {
            if (changes.todo) {
                this.todo = Object.assign({}, this.todo);
            }
        }

        onSubmit() {
            if (!this.todo.title) {
                return;
            }
            // with EventEmitter wrapper
            this.onAddTodo(
                this.EventEmitter({
                    todo: this.todo
                })
            );
            // without EventEmitter wrapper
            this.onAddTodo({
                $event: {
                    todo: this.todo
                }
            });
        }
    }
};

I was wondering if there's a reason why you chose to do it inline instead of declaring it first before using it like the following:

/* ----- todo/todo-form/todo-form.component.js ----- */
import templateUrl from './todo-form.html';

class TodoFormComponent {
    constructor(EventEmitter) {
        'ngInject';
        this.EventEmitter = EventEmitter;
    }

    $onChanges(changes) {
        if (changes.todo) {
            this.todo = Object.assign({}, this.todo);
        }
    }

    onSubmit() {
        if (!this.todo.title) {
            return;
        }
        // with EventEmitter wrapper
        this.onAddTodo(
            this.EventEmitter({
                todo: this.todo
            })
        );
        // without EventEmitter wrapper
        this.onAddTodo({
            $event: {
                todo: this.todo
            }
        });
    }
}

export const TodoFormComponent = {
    bindings: {
        todo: '<',
        onAddTodo: '&'
    },
    templateUrl,
    controller: TodoFormComponent
};

I think this makes it more readable and lessens indention? Thoughts?

[ryepup]

I like this adjustment too, the only issue with what's written above is re-using the name TodoFormComponent to mean two different things:

• the controller for the component
• the component definition itself

This is also tripping up the eslint no-redeclare rule.

This issue has been open for awhile, have you had any other positive or negative experience using this pattern in your application?

[dima716]

I would suggest to extract controller in different file and then import it as
import controller from './todo-form.controller';.

You can see an example here

[aaronte]

@ryepup Woops. Made a typo. class and component variables should be different name. This is the pattern I have been using for my application so far and haven't had any problems:

import templateUrl from './todo-form.html';

class TodoForm {
    constructor(EventEmitter) {
        'ngInject';
        this.EventEmitter = EventEmitter;
    }
}

export const TodoFormComponent = {
    bindings: {
        todo: '<',
        onAddTodo: '&'
    },
    templateUrl,
    controller: TodoForm
};

[ryepup]

good to hear these divergences from the style guide are working out. We're looking to adopt this styleguide at work and trying to figure which bits are the MUSTs and which are the SHOULDs.

@aaronte it's not just a typo in this issue, the official docs (e.g. https://github.com/toddmotto/angularjs-styleguide/blame/master/README.md#L331)

export const TodoComponent = {
  templateUrl,
  controller: class TodoComponent {}
}

@dima716 that looks pretty good to me, although the guide recommends dropping the name "controller".

I think we'll probably try leaving the controller class in the same file as the component like you're suggesting, and pull it into another file if it gets too big. Although controllers aren't supposed to get big, so...