2

Sometimes, often when refactoring a Controller - or also a Factory -, I end up passing my Controller tests but when my app is up and running it keeps crashing at some point because I forgot to add/update dependency injection.

With this I mean, let's say I have the following Controller where I used to have an oldDependency but due to refactoring I'm using a newDependency instead. I update MyCtrl.$inject with the new changes but I forget to update the dependencies passed to the MyCtrl function:

angular
    .module('my-module')
    .controller('MyCtrl', MyCtrl);

MyCtrl.$inject = [
    'firstDependency',
    'secondDependency',
    'newDependency' // this has been only updated here
];

function MyCtrl(firstDependency, secondDependency, oldDependency) {
    var vm = this;

    // My Controller's code
    // etc...

    function someFunc(x) {
       // here I use newDependency
       return newDependency.doSomething(x);
    }
}   

So what happens then? I go and update MyCtrl tests, where I actually remember to update the dependency object passed to $controller():

// MyCtrl testing code
var MyCtrl = $controller('VolunteerResignCtrl', {
    firstDependency: firstDependency,
    secondDependency: secondDependency,
    newDependency: newDependency
});

Because of this, all MyCtrl tests keep passing, so I think that nothing broke. But it actually did.

Could anyone tell me if this can be tested somehow and avoid my app failing for this reason in the future?

charliebrownie
  • 5,777
  • 10
  • 35
  • 55
  • But that should work in your application, the string you inject doesn't have to match what the variable name is in your function, otherwise, things like minification wouldn't work. – George Jun 02 '17 at 12:56
  • You are right! But most of the time I might also update the Controller's code, where I use `newDependency`. Any idea for these cases? – charliebrownie Jun 02 '17 at 12:59
  • @George I have just updated it! – charliebrownie Jun 02 '17 at 13:03
  • Yeah that's it! I understand what is happening, that's why I want to know if there's any way to avoid these kind of behaviors. Thank you @George! ;-) – charliebrownie Jun 02 '17 at 13:13
  • Please, explain your case further. In your code `newDependency` variable isn't defined and will obviously throw on `newDependency.doSomething(x)`. What do we miss here? – Estus Flask Jun 02 '17 at 13:30
  • Yeah I'm unable to [reproduce this](http://jsfiddle.net/gunryv5h/2/), are you sure your tests are actually hitting the function that's changed? – George Jun 02 '17 at 13:33
  • @George yes, they are! – charliebrownie Jun 02 '17 at 13:36
  • @estus of course it will throw when the app is running. But test will pass as there, in `$controller()` the `newDependency` has been defined correctly. – charliebrownie Jun 02 '17 at 13:38
  • @charliebrownie but I've used your code and it's throwing the ReferenceError when called via `$controller` as you can see [here](http://jsfiddle.net/gunryv5h/3/), what version on angular are you using? fyi the easiest way of fixing this issue is having a good IDE as then it will point out to you `newDependency` has not been defined. – George Jun 02 '17 at 13:41
  • someFunc should be covered by tests and thus should fail them, there's no way how tests can pass this way. If it isn't covered, this is the primary problem, not DI. In the code you've posted DI is done right in fact. The controller is injected with newDependency as expected. This conforms pretty closely to what George said. – Estus Flask Jun 02 '17 at 13:43
  • 1
    I think I might know why. In your test cases, how are you defining the `newDependency` function because you might be exposing it to the global scope. Try wrapping your tests in an [IIFE](https://stackoverflow.com/questions/8228281/what-is-the-function-construct-in-javascript). As you can see with [this example](http://jsfiddle.net/gunryv5h/4/) with the function `newDependency` defined in a _higher_ scope it's able to resolve it. – George Jun 02 '17 at 13:53
  • @George I always use IIFE! – charliebrownie Jun 02 '17 at 15:51

2 Answers2

1

This seems like an issue with JS scoping rather than Angulars dependency injection. Try wrapping your test cases and the functions that you use for them in an IIFE as it they're not, JS will go up the scope until it can find a variable called newDependency which is probably why your tests are running but it's crashing in your app.

As you can see from the examples below

Without an IIFE

var myApp = angular.module('myApp', []);

myApp.factory('firstDependency', function() {
  return {}
}());
myApp.factory('secondDependency', function() {
  return {}
}());
myApp.factory('newDependency', function() {
  return {}
}())

myApp.controller('MyCtrl', MyCtrl);
myApp.controller('TestCtrl', TestCtrl);

MyCtrl.$inject = [
  'firstDependency',
  'secondDependency',
  'newDependency'
];

function MyCtrl(firstDependency, secondDependency, oldDependency) {
  var vm = this;
  vm.someFunc = function(x) {
    // Will work fine as newDependency is defined in a higher scope :c
    return newDependency.doSomething(x);
  }
}

var firstDependency = function() {
  return {}
}();
var secondDependency = function() {
  return {}
}();
var newDependency = function() {
  return {
    doSomething: function(x) {
      return x;
    }
  }
}()

function TestCtrl($scope, $controller) {
  var test = $controller('MyCtrl', {
    firstDependency: firstDependency,
    secondDependency: secondDependency,
    newDependency: newDependency
  });

  $scope.someFunc = test.someFunc("you're inside the new dependency");
}
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.23/angular.min.js"></script>
<div ng-app="myApp">
  <div ng-controller="TestCtrl">
    {{someFunc}}!
  </div>
</div>

With an IIFE

var myApp = angular.module('myApp', []);

myApp.factory('firstDependency', function() {
  return {}
}());
myApp.factory('secondDependency', function() {
  return {}
}());
myApp.factory('newDependency', function() {
  return {}
}())

myApp.controller('MyCtrl', MyCtrl);


MyCtrl.$inject = [
  'firstDependency',
  'secondDependency',
  'newDependency'
];

function MyCtrl(firstDependency, secondDependency, oldDependency) {
  var vm = this;
  vm.someFunc = function(x) {
    //Will throw an error, but this time it's a good error because that's what you want!
    return newDependency.doSomething(x);
  }
}

(function(myApp) {


  var firstDependency = function() {
    return {}
  }();
  var secondDependency = function() {
    return {}
  }();
  var newDependency = function() {
    return {
      doSomething: function(x) {
        return x;
      }
    }
  }()

  myApp.controller('TestCtrl', TestCtrl);

  function TestCtrl($scope, $controller) {
    var test = $controller('MyCtrl', {
      firstDependency: firstDependency,
      secondDependency: secondDependency,
      newDependency: newDependency
    });

    $scope.someFunc = test.someFunc("you're inside the new dependency");
  }

})(myApp)
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.23/angular.min.js"></script>
<div ng-app="myApp">
  <div ng-controller="TestCtrl">
    {{someFunc}}!
  </div>
</div>
George
  • 6,630
  • 2
  • 29
  • 36
0

Name of variables are not important from js-point of view. Following functions are SAME:

function f(SomeService) {
  SomeService.test();
}

function f(NewService) {
  NewService.test();
}

function f(a) {
  a.test();
}

Thats the reason why you need $inject - cause you pass string their. After you change name in $inject - you may change it in function, but you also may not change it. Doesnt matter at all.

Petr Averyanov
  • 9,327
  • 3
  • 20
  • 38