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

Commit 13d113c

Browse files
committed
perf(ngRepeat): use no-proto objects for blockMaps
1 parent 215d954 commit 13d113c

File tree

3 files changed

+33
-26
lines changed

3 files changed

+33
-26
lines changed

src/Angular.js

+16
Original file line numberDiff line numberDiff line change
@@ -1571,3 +1571,19 @@ function getBlockNodes(nodes) {
15711571

15721572
return jqLite(blockNodes);
15731573
}
1574+
1575+
1576+
/**
1577+
* Creates a new object without a prototype. This object is useful for lookup without having to
1578+
* guard against prototypically inherited properties via hasOwnProperty.
1579+
*
1580+
* Related micro-benchmarks:
1581+
* - http://jsperf.com/object-create2
1582+
* - http://jsperf.com/proto-map-lookup/2
1583+
* - http://jsperf.com/for-in-vs-object-keys2
1584+
*
1585+
* @returns {Object}
1586+
*/
1587+
function createMap() {
1588+
return Object.create(null);
1589+
}

src/ng/directive/ngRepeat.js

+17-18
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,10 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
297297
// - scope: bound scope
298298
// - element: previous element.
299299
// - index: position
300-
var lastBlockMap = {};
300+
//
301+
// We are using no-proto object so that we don't need to guard against inherited props via
302+
// hasOwnProperty.
303+
var lastBlockMap = createMap();
301304

302305
//watch props
303306
$scope.$watchCollection(rhs, function ngRepeatAction(collection) {
@@ -306,7 +309,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
306309
nextNode,
307310
// Same as lastBlockMap but it has the current state. It will become the
308311
// lastBlockMap on the next iteration.
309-
nextBlockMap = {},
312+
nextBlockMap = createMap(),
310313
arrayLength,
311314
key, value, // key/value of iteration
312315
trackById,
@@ -343,39 +346,35 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
343346
key = (collection === collectionKeys) ? index : collectionKeys[index];
344347
value = collection[key];
345348
trackById = trackByIdFn(key, value, index);
346-
assertNotHasOwnProperty(trackById, '`track by` id');
347-
if (lastBlockMap.hasOwnProperty(trackById)) {
349+
if (lastBlockMap[trackById]) {
350+
// found previously seen block
348351
block = lastBlockMap[trackById];
349352
delete lastBlockMap[trackById];
350353
nextBlockMap[trackById] = block;
351354
nextBlockOrder[index] = block;
352-
} else if (nextBlockMap.hasOwnProperty(trackById)) {
353-
// restore lastBlockMap
355+
} else if (nextBlockMap[trackById]) {
356+
// id collision detected. restore lastBlockMap and throw an error
354357
forEach(nextBlockOrder, function (block) {
355358
if (block && block.scope) lastBlockMap[block.id] = block;
356359
});
357-
// This is a duplicate and we need to throw an error
358360
throw ngRepeatMinErr('dupes', "Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}",
359361
expression, trackById);
360362
} else {
361363
// new never before seen block
362364
nextBlockOrder[index] = {id: trackById, scope: undefined, clone: undefined};
363-
nextBlockMap[trackById] = false;
365+
nextBlockMap[trackById] = true;
364366
}
365367
}
366368

367369
// remove existing items
368370
for (var blockKey in lastBlockMap) {
369-
// lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn
370-
if (lastBlockMap.hasOwnProperty(blockKey)) {
371-
block = lastBlockMap[blockKey];
372-
elementsToRemove = getBlockNodes(block.clone);
373-
$animate.leave(elementsToRemove);
374-
forEach(elementsToRemove, function (element) {
375-
element[NG_REMOVED] = true;
376-
});
377-
block.scope.$destroy();
378-
}
371+
block = lastBlockMap[blockKey];
372+
elementsToRemove = getBlockNodes(block.clone);
373+
$animate.leave(elementsToRemove);
374+
forEach(elementsToRemove, function (element) {
375+
element[NG_REMOVED] = true;
376+
});
377+
block.scope.$destroy();
379378
}
380379

381380
// we are not using forEach for perf reasons (trying to avoid #call)

test/ng/directive/ngRepeatSpec.js

-8
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,6 @@ describe('ngRepeat', function() {
174174
});
175175

176176

177-
it("should throw an exception if 'track by' evaluates to 'hasOwnProperty'", function() {
178-
scope.items = {age:20};
179-
$compile('<div ng-repeat="(key, value) in items track by \'hasOwnProperty\'"></div>')(scope);
180-
scope.$digest();
181-
expect($exceptionHandler.errors.shift().message).toMatch(/ng:badname/);
182-
});
183-
184-
185177
it('should track using build in $id function', function() {
186178
element = $compile(
187179
'<ul>' +

0 commit comments

Comments
 (0)