New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Directive 'vm' conflict #890

Closed
brendanscarano opened this Issue Nov 20, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@brendanscarano
Copy link

brendanscarano commented Nov 20, 2015

I'm working on a single page application in Meteor/Angular where I have two directives in my index.html on my global scope. I am using controllerAs: 'vm' for both directives and it seems that they are overwriting one another.

(function() {
  'use strict';
  function dateTime() {

    function dateTimeCtrl() {
        this.foo = 'testing date time component';
    }

    return {
      restrict: 'EA',
      controller: dateTimeCtrl,
      controllerAs: 'vm',
      template: '<div>{{vm.foo}}</div>'
    };
  }

  angular
    .module('DateTimeDirective', [])
    .directive('dateTime', dateTime);
})();
(function() {
  'use strict';
  function weatherForecast() {

    function weatherForecastCtrl() {
        this.bar = 'testing weather forecast component';
    }

    return {
      restrict: 'EA',
      controller: weatherForecastCtrl,
      controllerAs: 'vm',
      template: '<div>{{vm.bar}}</div>'
    };
  }

  angular
    .module('WeatherForecastDirective', [])
    .directive('weatherForecast', weatherForecast);
})();

I am placing both directives within my index.html

<head>
  <base href="/">
</head>
<body>
  <div ng-app="morningApp">

    <date-time></date-time>

    <weather-forecast></weather-forecast>

  </div>
</body>

When I run meteor and open my localhost I expect to see:
testing date time component
testing weather forecast component

However I am only seeing:
testing weather forecast component

When I inspect the 'vm' scope in my dev tools I am only seeing bar attached to 'vm', and not foo.

If I change the controllerAs to anything else in either directive the output works fine.

@brendanscarano brendanscarano changed the title Directive scope conflict Directive 'vm' conflict Nov 20, 2015

@MilosStanic

This comment has been minimized.

Copy link
Contributor

MilosStanic commented Nov 21, 2015

You shouldn't name controller aliases the same for both controllers, or any two controllers for that matter. As you properly concluded, now you have one single vm variable in the global scope, and whichever is declared last overwrites the vm variable.

You may have been confused by following instructions on page http://www.angular-meteor.com/api/getReactively

Where it gives example of naming the controller as vm.
However, the practice as per John Papa's guide is to alias this of the controller internally as vm and in routers, or other controllerAs declarations, use some more meaningful names. In your case:

 (function() {
  'use strict';
  function dateTime() {

    function dateTimeCtrl() {
        var vm = this; // or var self = this;
        vm.foo = 'testing date time compnent';

        vm.fooFn = function (){
            // ....
            vm.foo = 'you have a typo - it\'s compOnent';
        };
    }

    return {
      restrict: 'EA',
      controller: dateTimeCtrl,
      controllerAs: 'dtc',
      template: '<div>{{dtc.foo}}</div>'
    };
  }

  angular
    .module('DateTimeDirective', [])
    .directive('dateTime', dateTime);
})();

and

(function() {
  'use strict';
  function weatherForecast() {

    function weatherForecastCtrl() {
        var vm = this;
        vm.bar = 'testing weather forecast component';
    }

    return {
      restrict: 'EA',
      controller: weatherForecastCtrl,
      controllerAs: 'wfc',
      template: '<div>{{wfc.bar}}</div>'
    };
  }

  angular
    .module('WeatherForecastDirective', [])
    .directive('weatherForecast', weatherForecast);
})();

I hope that's clear now.

@brendanscarano

This comment has been minimized.

Copy link
Author

brendanscarano commented Nov 22, 2015

This makes sense - Thank you for the help. I'm going to refactor my controllers taking this into consideration.

@mxab

This comment has been minimized.

Copy link
Contributor

mxab commented Nov 22, 2015

I assume you don't want to access the global controller from inside your directive, therefore you should isolate the scope of your directive so you can also both name 'vm'.

To isolate use

  return {
      restrict: 'EA',
      controller: weatherForecastCtrl,
      controllerAs: 'vm',
      template: '<div>{{vm.bar}}</div>',
     scope : {}

    };

if you don't put the scope attribute there it will have access to outer scope which usually is more dangerous than helpful

@MilosStanic

This comment has been minimized.

Copy link
Contributor

MilosStanic commented Nov 22, 2015

@mxab true, I totally overlooked that.

@mxab

This comment has been minimized.

Copy link
Contributor

mxab commented Nov 24, 2015

@brendanscarano can you close this issue? I think it is really unrelated to the meteor angular project

@brendanscarano

This comment has been minimized.

Copy link
Author

brendanscarano commented Nov 24, 2015

@mxab sure, I'll close it! Thank you guys for the help

@mxab

This comment has been minimized.

Copy link
Contributor

mxab commented Nov 24, 2015

Don't you see a close button at the end of this issue?

@Urigo

This comment has been minimized.

Copy link
Owner

Urigo commented Dec 9, 2015

by the way, the updated tutorial is written in controllerAs syntax for helping people use best practices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment