Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

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

Merged
merged 2 commits into from
Dec 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions src/ng/directive/ngRepeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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};
jbedard marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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];
Expand All @@ -594,6 +592,12 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
}
}

// Clear the value property from the hashFnLocals object to prevent a reference to the last value
// being leaked into the ngRepeatCompile function scope
if (hashFnLocals) {
hashFnLocals[valueIdentifier] = undefined;
}

// remove leftover items
for (var blockKey in lastBlockMap) {
block = lastBlockMap[blockKey];
Expand Down
37 changes: 37 additions & 0 deletions test/ng/directive/ngRepeatSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});
jbedard marked this conversation as resolved.
Show resolved Hide resolved

// 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() {
Expand Down