johnpapa / angular-styleguide

Angular Style Guide: A starting point for Angular development teams to provide consistency through good practices.
http://johnpapa.net
MIT License
23.88k stars 4.16k forks source link

Directive controllerAs and scope #536

Open facultymatt opened 8 years ago

facultymatt commented 8 years ago

When using multiple directives that use the controllerAs: 'vm' syntax and do not have scope, they will conflict and vm will get overwritten. For example, the following directives will not work as expected:

angular
    .module('app')
    .directive('myExampleOne', myExampleOne);

function myExampleOne() {
    var directive = {
        templateUrl: 'app/feature/example.directiveOne.html',
        controllerAs: 'vm',
        bindToController: true 
    };
    return directive;
}

angular
    .module('app')
    .directive('myExampleTwo', myExampleTwo);

function myExampleTwo() {
    var directive = {
        templateUrl: 'app/feature/example.directiveTwo.html',
        controllerAs: 'vm',
        bindToController: true 
    };
    return directive;
}

A few questions here:

  1. Does it make sense to always use scope? scope: {} or even scope: true will fix this issue. In my experience this is good practice to avoid scope conflicts even sans controllerAs.
  2. Should we not use bindToController in these examples?

    Related style

https://github.com/johnpapa/angular-styleguide#style-y075

lunks commented 8 years ago

I'd say that it is a good practice to always create a new scope for directives.

jansepke commented 8 years ago

using only isolated scopes, gives you more control over the dataflow of your application. and yes since angular 1.4 you should always use bindToController with isolated scopes and controllerAs.

lunks commented 8 years ago

Any changes on angular 1.4 that makes this different than 1.3, @Jandalf?

jansepke commented 8 years ago

@lunks yes bindToController supports now an object value to follow the angular 2 binding style

e.g.: scope: {}, bindToController {param1: '='}

see #543

karlhorky commented 8 years ago

Are isolated scopes not bad for composability (more directives on one element)? Since Angular 1.2 there is an error Multiple Directive Resource Contention when using multiple directives with isolated scopes. For instance:

<input my-directive my-second-directive ng-model="title">
angular.module('app', []).directive('myDirective', function () {
  return {
    restrict: 'A',
    scope: {}
  };
}).directive('mySecondDirective', function () {
  return {
    restrict: 'A',
    scope: {}
  };
});

Demo on Plunker

Resulting error: https://docs.angularjs.org/error/$compile/multidir?p0=myDirective&p1=mySecondDirective&p2=new%2Fisolatedundefinedcope&p3=undefined

karlhorky commented 8 years ago

I've opened #608 for an answer to this.

karlhorky commented 8 years ago

For this reason I would like an alternative answer to this question though - without using isolated scope. One thing I can imagine is to rename controllerAs based on the directive name.

angular
    .module('app')
    .directive('myExampleOne', myExampleOne);

function myExampleOne() {
    var directive = {
        templateUrl: 'app/feature/example.directiveOne.html',
        controllerAs: 'myExampleOneVm',
        bindToController: true 
    };
    return directive;
}

angular
    .module('app')
    .directive('myExampleTwo', myExampleTwo);

function myExampleTwo() {
    var directive = {
        templateUrl: 'app/feature/example.directiveTwo.html',
        controllerAs: 'myExampleTwoVm',
        bindToController: true 
    };
    return directive;
}

This would still not solve nested directives of the same type though...