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

feat(ngRepeat): expose the iterating properties over the alias #9622

Closed
wants to merge 1 commit into from
Closed

feat(ngRepeat): expose the iterating properties over the alias #9622

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 11 additions & 8 deletions src/ng/directive/ngRepeat.js
View file
Original file line number Diff line number Diff line change
@@ -211,18 +211,21 @@
var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
var NG_REMOVED = '$$NG_REMOVED';
var ngRepeatMinErr = minErr('ngRepeat');
var nullAliasProperties = {};

var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength) {
var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength, aliasAs) {
var aliasRef = aliasAs ? {} : nullAliasProperties;
// TODO(perf): generate setters to shave off ~40ms or 1-1.5%
scope[valueIdentifier] = value;
if (keyIdentifier) scope[keyIdentifier] = key;
scope.$index = index;
scope.$first = (index === 0);
scope.$last = (index === (arrayLength - 1));
scope.$middle = !(scope.$first || scope.$last);
aliasRef.$index = scope.$index = index;
aliasRef.$first = scope.$first = (index === 0);
aliasRef.$last = scope.$last = (index === (arrayLength - 1));
aliasRef.$middle = scope.$middle = !(scope.$first || scope.$last);
// jshint bitwise: false
scope.$odd = !(scope.$even = (index&1) === 0);
aliasRef.$odd = scope.$odd = !(aliasRef.$even = scope.$even = (index&1) === 0);
// jshint bitwise: true
if (aliasAs) scope['$' + aliasAs] = aliasRef;
};

var getBlockStart = function(block) {
@@ -411,7 +414,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
$animate.move(getBlockNodes(block.clone), null, jqLite(previousNode));
}
previousNode = getBlockEnd(block);
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength);
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength, aliasAs);
} else {
// new item which we don't know about
$transclude(function ngRepeatTransclude(clone, scope) {
@@ -428,7 +431,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
// by a directive with templateUrl when its template arrives.
block.clone = clone;
nextBlockMap[block.id] = block;
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength);
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength, aliasAs);
});
}
}
37 changes: 37 additions & 0 deletions test/ng/directive/ngRepeatSpec.js
View file
Original file line number Diff line number Diff line change
@@ -492,6 +492,43 @@ describe('ngRepeat', function() {
return text.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
}
}));

it('should expose the index properties using `${{userSpecifiedAliasName}}`', inject(function() {
var expression = '{{item}}-{{$results.' + ['$index', '$first', '$middle', '$last', '$odd', '$even'].join('}}-{{$results.') + '}}:';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kinda torn on this --- I know I said it's better than the other thing, but I'm not sure... it sucks this way too =\

I think I need someone else to weigh in on which way would be better. I don't like potentially shadowing object keys, and I don't like the slight impact of crawling up the prototype chain. But maybe it's better than this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the original idea of reusing the same alias name, the collection is an array and that makes it somehow less problematic in regards to shadowing properties (still not great, but not really bad)
People that need to use this are a very small % of the uses of ng-repeat so I am not too worried about a small performance impact

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like it --- I think the extra directive option is better, tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean to extend the microsyntax or to use `${{aliasName}}?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neither. I think just using a new directive for setting up the alias is better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's too bad --- I suggested earlier that we could set up a dictionary of aliases on $scope (which would be touched by the dictionary) to keep them in sync with assign expressions --- then we'd use assign rather than = to set up the $index/etc vars, and do the hard work without touching dirty checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, maybe I misunderstood your idea on ng-alias, I thought that the idea was that if you have ng-alias='foo as bar', then if you modify foo, then bar would reflect that change and viceversa (of course, everything with $parse and expressions). Now, when on top of this you add one-time binding, this is when things get non-trivial. I am not saying impossible, just that need to be done with a lot of care

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one-time bindings would not make sense for an alias, because it would be completely unrelated to change detection (in my vision of it) --- so a one time binding would not matter, and that's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one-time binding is unrelated, but an implementation of ng-alias should be able to handle ng-alias='["foo"] as foo'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it shouldn't, ["foo"] is not a binding. We should be dealing with identifiers --- assignable expressions only

element = $compile(
'<div>' +
' <div ng-repeat="item in items as results">' + expression + '</div>' +
'</div>')(scope);

scope.items = ['a','b','c','d','e','f'];
scope.$digest();
expect(trim(element.text())).toEqual(
'a-0-true-false-false-false-true:' +
'b-1-false-true-false-true-false:' +
'c-2-false-true-false-false-true:' +
'd-3-false-true-false-true-false:' +
'e-4-false-true-false-false-true:' +
'f-5-false-false-true-true-false:');

scope.items.shift();
scope.$digest();
expect(trim(element.text())).toEqual(
'b-0-true-false-false-false-true:' +
'c-1-false-true-false-true-false:' +
'd-2-false-true-false-false-true:' +
'e-3-false-true-false-true-false:' +
'f-4-false-false-true-false-true:');
}));

it('should be possible to access the alias inside the loop', inject(function() {
element = $compile(
'<div>' +
' <div ng-repeat="item in items as results">{{results[$index]}}</div>' +
'</div>')(scope);
scope.items = ['a','b','c','d','e','f'];
scope.$digest();
expect(trim(element.text())).toEqual('abcdef');
}));
});