fix(ngRepeat): fix trackBy function being invoked with incorrect scope

Also fixes a leak of that scope across all further instances of the repeated element. Fixes #16776 Closes #16777
angular · Dec 6, 2018 · d4d1031 · d4d1031
1 parent 7981140
commit d4d1031
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 20 deletions.
diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js
@@ -454,6 +454,13 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
     return block.clone[block.clone.length - 1];
   };
 
+  var trackByIdArrayFn = function($scope, key, value) {
+    return hashKey(value);
+  };
+
+  var trackByIdObjFn = function($scope, key) {
+    return key;
+  };
 
   return {
     restrict: 'A',
@@ -493,32 +500,23 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
           aliasAs);
       }
 
-      var trackByExpGetter, trackByIdExpFn, trackByIdArrayFn, trackByIdObjFn;
-      var hashFnLocals = {$id: hashKey};
+      var trackByIdExpFn;
 
       if (trackByExp) {
-        trackByExpGetter = $parse(trackByExp);
-      } else {
-        trackByIdArrayFn = function(key, value) {
-          return hashKey(value);
-        };
-        trackByIdObjFn = function(key) {
-          return key;
+        var hashFnLocals = {$id: hashKey};
+        var trackByExpGetter = $parse(trackByExp);
+
+        trackByIdExpFn = function($scope, key, value, index) {
+          // assign key, value, and $index to the locals so that they can be used in hash functions
+          if (keyIdentifier) hashFnLocals[keyIdentifier] = key;
+          hashFnLocals[valueIdentifier] = value;
+          hashFnLocals.$index = index;
+          return trackByExpGetter($scope, hashFnLocals);
         };
       }
 
       return function ngRepeatLink($scope, $element, $attr, ctrl, $transclude) {
 
-        if (trackByExpGetter) {
-          trackByIdExpFn = function(key, value, index) {
-            // assign key, value, and $index to the locals so that they can be used in hash functions
-            if (keyIdentifier) hashFnLocals[keyIdentifier] = key;
-            hashFnLocals[valueIdentifier] = value;
-            hashFnLocals.$index = index;
-            return trackByExpGetter($scope, hashFnLocals);
-          };
-        }
-
         // Store a list of elements from previous run. This is a hash where key is the item from the
         // iterator, and the value is objects with following properties.
         //   - scope: bound scope
@@ -572,7 +570,7 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
           for (index = 0; index < collectionLength; index++) {
             key = (collection === collectionKeys) ? index : collectionKeys[index];
             value = collection[key];
-            trackById = trackByIdFn(key, value, index);
+            trackById = trackByIdFn($scope, key, value, index);
             if (lastBlockMap[trackById]) {
               // found previously seen block
               block = lastBlockMap[trackById];

diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js
@@ -400,6 +400,43 @@ describe('ngRepeat', function() {
       expect(element.find('input')[1].checked).toBe(true);
       expect(element.find('input')[2].checked).toBe(true);
     }));
+
+    it('should invoke track by with correct locals', function() {
+      scope.trackBy = jasmine.createSpy().and.callFake(function(k, v) {
+        return [k, v].join('');
+      });
+
+      element = $compile(
+        '<ul>' +
+          '<li ng-repeat="(k, v) in [1, 2] track by trackBy(k, v)"></li>' +
+        '</ul>')(scope);
+      scope.$digest();
+
+      expect(scope.trackBy).toHaveBeenCalledTimes(2);
+      expect(scope.trackBy.calls.argsFor(0)).toEqual([0, 1]);
+      expect(scope.trackBy.calls.argsFor(1)).toEqual([1, 2]);
+    });
+
+    // https://github.com/angular/angular.js/issues/16776
+    it('should invoke nested track by with correct locals', function() {
+      scope.trackBy = jasmine.createSpy().and.callFake(function(k1, v1, k2, v2) {
+        return [k1, v1, k2, v2].join('');
+      });
+
+      element = $compile(
+        '<ul>' +
+          '<li ng-repeat="(k1, v1) in [1, 2]">' +
+            '<div ng-repeat="(k2, v2) in [3, 4] track by trackBy(k1, v1, k2, v2)"></div>' +
+          '</li>' +
+        '</ul>')(scope);
+      scope.$digest();
+
+      expect(scope.trackBy).toHaveBeenCalledTimes(4);
+      expect(scope.trackBy.calls.argsFor(0)).toEqual([0, 1, 0, 3]);
+      expect(scope.trackBy.calls.argsFor(1)).toEqual([0, 1, 1, 4]);
+      expect(scope.trackBy.calls.argsFor(2)).toEqual([1, 2, 0, 3]);
+      expect(scope.trackBy.calls.argsFor(3)).toEqual([1, 2, 1, 4]);
+    });
   });
 
   describe('alias as', function() {