AngularJS best practices: Be careful when using ng-repeat’s $index

“A customer reported they deleted an item and the wrong item got deleted!”

Sounds like a pretty serious bug. This is what we got one time at work. Attempting to locate it was quite difficult, because naturally the customer had no idea what they did to reproduce the issue.

Turns out the bug was caused by using $index in an ng-repeat. Let’s take a look at how this happens, and a super simple way to avoid this type of bug entirely, and also a few lessons we can learn from this.

A simple list with an action

Let’s look at an example of a perfectly valid ng-repeat, and its controller.

<ul ng-controller="ListCtrl">
  <li ng-repeat="item in items">
    {{item.name}}
    <button ng-click="remove($index)">remove</button>
  </li>
</ul>

app.controller('ListCtrl', ['$scope', function($scope) {
  //items come from somewhere, from where doesn't matter for this example
  $scope.items = getItems();
 
  $scope.remove = function(index) {
    var item = $scope.items[index];
    removeItem(item);
  };
}]);

Looks OK, right? Nothing particularly special about this code.

Adding a filter

Now, let’s do a small change: Let’s add a filter to the list. This is a reasonably common thing to do if you have a long list, for example to allow the user to search the list.

For sake of this example, assume searchFilter allows us to filter the list by some search query.

<ul ng-controller="ListCtrl">
  <li ng-repeat="item in items | searchFilter">
    {{item.name}}
    <button ng-click="remove($index)">remove</button>
  </li>
</ul>

The controller code stays the same. Still looks good, right?

There’s actually a bug in there now. Can you find it? Would you have thought about it if I hadn’t mentioned it?

Using $index should be avoided

The bug is in the controller:

$scope.remove = function(index) {
  var item = $scope.items[index];
  removeItem(item);
};

As we use the index here, we will run into problems as soon as we have filtered the list in a way which causes the indexes to not match the original list.

Thankfully there’s a really simple way to avoid this: Instead of using $index, prefer to pass the actual objects around.

<ul ng-controller="ListCtrl">
  <li ng-repeat="item in items | searchFilter">
    {{item.name}}
    <button ng-click="remove(item)">remove</button>
  </li>
</ul>

$scope.remove = function(item) {
  removeItem(item);
};

Notice how I changed remove($index) into remove(item), and changed the $scope.remove function to operate directly on the object instead.

This simple change avoids the bug issue entirely.

To better illustrate the problem and the solution, you can use this interactive example.

What can we learn from this?

The first lesson is of course that we should be careful when using $index, as it can easily cause bugs when used in certain ways.

The second lesson is that by recognizing patterns like this, you can establish better ways of doing things that can avoid certain types of bugs entirely. I’m definitely recommending everyone to avoid $index now, and as a result, their code will have less bugs from just this simple change of thinking.

The third lesson is tests won’t always help you. Even if you have automated tests covering something like this, it’s quite easy for them to miss this, because the bugs caused by this depend so much on the specific inputs given. The bug won’t always show itself, even when using filtering.

The fourth lesson is don’t break abstraction – This one is very easy to miss. Technically $index is a “template variable” created by ng-repeat. It’s only accurate and has meaning inside the repeat block. When we pass the value out, it loses its context and it’s no longer valid. In order for it to be valid outside the repeat, we would also have to filter the list in our controller, which would require some code duplication which should be avoided. Thankfully the pattern introduced in this article can be used to avoid this.